From 99df00e70f7b28c865e62742d3cf11c46df2b053 Mon Sep 17 00:00:00 2001 From: Aadit Chugh Date: Mon, 15 Jul 2024 05:57:07 +0000 Subject: [PATCH] Resolve ADAPT1-1599 "Incorporate Retry Policy Logic for SchemaRegistry Client" --- .../hydra/avro/registry/SchemaRegistry.scala | 24 ++++++++++++++----- .../http/TopicDeletionEndpointSpec.scala | 4 ++-- .../programs/TopicDeletionProgramSpec.scala | 4 ++-- .../endpoints/BootstrapEndpointV2Spec.scala | 5 ++-- .../programs/CreateTopicProgramSpec.scala | 12 +++++----- 5 files changed, 31 insertions(+), 18 deletions(-) diff --git a/avro/src/main/scala/hydra/avro/registry/SchemaRegistry.scala b/avro/src/main/scala/hydra/avro/registry/SchemaRegistry.scala index cabb40673..788d4f61c 100644 --- a/avro/src/main/scala/hydra/avro/registry/SchemaRegistry.scala +++ b/avro/src/main/scala/hydra/avro/registry/SchemaRegistry.scala @@ -64,17 +64,21 @@ trait SchemaRegistry[F[_]] { * * @param subject - subject name for the schema found in SchemaRegistry including the suffix (-key | -value) * @param schema - avro Schema which is expected to be in Schema Registry + * @param useExponentialBackoffRetryPolicy - flag indicating whether to use exponential backoff retry policy + * (default: false, meaning constant delay retry policy is used) * @return SchemaVersion */ - def getVersion(subject: String, schema: Schema): F[SchemaVersion] + def getVersion(subject: String, schema: Schema, useExponentialBackoffRetryPolicy: Boolean = false): F[SchemaVersion] /** * Retrieves all SchemaVersion(s) for a given subject. * * @param subject - subject name for the schema found in SchemaRegistry including the suffix (-key | -value) + * @param useExponentialBackoffRetryPolicy - flag indicating whether to use exponential backoff retry policy + * (default: false, meaning constant delay retry policy is used) * @return List[SchemaVersion] or List.empty if Subject Not Found */ - def getAllVersions(subject: String): F[List[SchemaVersion]] + def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean = false): F[List[SchemaVersion]] /** * Retrieves all subjects found in the SchemaRegistry @@ -204,7 +208,10 @@ object SchemaRegistry { schemaRegistryClientRetries: Int, schemaRegistryClientRetriesDelay: FiniteDuration): SchemaRegistry[F] = new SchemaRegistry[F] { - val retryPolicy = limitRetries(schemaRegistryClientRetries) |+| exponentialBackoff[F](schemaRegistryClientRetriesDelay) + lazy val constantDelayRetryPolicy = limitRetries(schemaRegistryClientRetries) |+| constantDelay[F](schemaRegistryClientRetriesDelay) + + lazy val exponentialBackOffRetryPolicy = limitRetries(schemaRegistryClientRetries) |+| exponentialBackoff[F](schemaRegistryClientRetriesDelay) + private implicit class SchemaOps(sch: Schema) { def fields: List[Schema.Field] = fieldsEval("topLevel", box = false).value @@ -292,8 +299,10 @@ object SchemaRegistry { override def getVersion( subject: String, - schema: Schema - ): F[SchemaVersion] = + schema: Schema, + useExponentialBackoffRetryPolicy: Boolean = false + ): F[SchemaVersion] = { + val retryPolicy = if(useExponentialBackoffRetryPolicy) exponentialBackOffRetryPolicy else constantDelayRetryPolicy Sync[F].delay { schemaRegistryClient.getVersion(subject, schema) }.retryingOnSomeErrors( @@ -304,12 +313,15 @@ object SchemaRegistry { policy = retryPolicy, onError = onFailure("getVersion", subject) ) + } - override def getAllVersions(subject: String): F[List[SchemaId]] = + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean = false): F[List[SchemaId]] = { + val retryPolicy = if(useExponentialBackoffRetryPolicy) exponentialBackOffRetryPolicy else constantDelayRetryPolicy Sync[F].fromTry(Try(schemaRegistryClient.getAllVersions(subject))) .map(_.asScala.toList.map(_.toInt)).recover { case r: RestClientException if r.getErrorCode == 40401 => List.empty }.retryingOnAllErrors(retryPolicy, onFailure("getAllVersions", subject)) + } private def onFailure(resourceTried: String, subject: String): (Throwable, RetryDetails) => F[Unit] = (error, retryDetails) => diff --git a/ingest/src/test/scala/hydra/ingest/http/TopicDeletionEndpointSpec.scala b/ingest/src/test/scala/hydra/ingest/http/TopicDeletionEndpointSpec.scala index b6d29137a..248112e55 100644 --- a/ingest/src/test/scala/hydra/ingest/http/TopicDeletionEndpointSpec.scala +++ b/ingest/src/test/scala/hydra/ingest/http/TopicDeletionEndpointSpec.scala @@ -64,10 +64,10 @@ class TopicDeletionEndpointSpec extends Matchers with AnyWordSpecLike with Scala override def deleteSchemaOfVersion(subject: String,version: SchemaVersion): F[Unit] = underlying.deleteSchemaOfVersion(subject,version) - override def getVersion(subject: String,schema: Schema): F[SchemaVersion] = + override def getVersion(subject: String,schema: Schema, useExponentialBackoffRetryPolicy: Boolean): F[SchemaVersion] = underlying.getVersion(subject,schema) - override def getAllVersions(subject: String): F[List[SchemaId]] = + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): F[List[SchemaId]] = underlying.getAllVersions(subject) override def getAllSubjects: F[List[String]] = diff --git a/ingest/src/test/scala/hydra/ingest/programs/TopicDeletionProgramSpec.scala b/ingest/src/test/scala/hydra/ingest/programs/TopicDeletionProgramSpec.scala index ac8d4ab09..42ae6fc66 100644 --- a/ingest/src/test/scala/hydra/ingest/programs/TopicDeletionProgramSpec.scala +++ b/ingest/src/test/scala/hydra/ingest/programs/TopicDeletionProgramSpec.scala @@ -60,10 +60,10 @@ class TopicDeletionProgramSpec extends AnyFlatSpec with Matchers { override def deleteSchemaOfVersion(subject: String, version: SchemaVersion): F[Unit] = underlying.deleteSchemaOfVersion(subject, version) - override def getVersion(subject: String, schema: Schema): F[SchemaVersion] = + override def getVersion(subject: String, schema: Schema, useExponentialBackoffRetryPolicy: Boolean): F[SchemaVersion] = underlying.getVersion(subject, schema) - override def getAllVersions(subject: String): F[List[SchemaId]] = + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): F[List[SchemaId]] = underlying.getAllVersions(subject) override def getAllSubjects: F[List[String]] = diff --git a/ingestors/kafka/src/test/scala/hydra/kafka/endpoints/BootstrapEndpointV2Spec.scala b/ingestors/kafka/src/test/scala/hydra/kafka/endpoints/BootstrapEndpointV2Spec.scala index 1bd207a9f..b1db0ff27 100644 --- a/ingestors/kafka/src/test/scala/hydra/kafka/endpoints/BootstrapEndpointV2Spec.scala +++ b/ingestors/kafka/src/test/scala/hydra/kafka/endpoints/BootstrapEndpointV2Spec.scala @@ -268,9 +268,10 @@ final class BootstrapEndpointV2Spec ): IO[Unit] = err override def getVersion( subject: String, - schema: Schema + schema: Schema, + useExponentialBackoffRetryPolicy: Boolean ): IO[SchemaVersion] = err - override def getAllVersions(subject: String): IO[List[Int]] = err + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): IO[List[Int]] = err override def getAllSubjects: IO[List[String]] = err override def getSchemaRegistryClient: IO[SchemaRegistryClient] = err diff --git a/ingestors/kafka/src/test/scala/hydra/kafka/programs/CreateTopicProgramSpec.scala b/ingestors/kafka/src/test/scala/hydra/kafka/programs/CreateTopicProgramSpec.scala index ecff53481..5b5ee7b69 100644 --- a/ingestors/kafka/src/test/scala/hydra/kafka/programs/CreateTopicProgramSpec.scala +++ b/ingestors/kafka/src/test/scala/hydra/kafka/programs/CreateTopicProgramSpec.scala @@ -75,10 +75,10 @@ class CreateTopicProgramSpec extends AsyncFreeSpec with Matchers with IOSuite { override def deleteSchemaOfVersion(subject: String, version: SchemaVersion): IO[Unit] = ref.update(_.copy(deleteSchemaWasCalled = true)) - override def getVersion(subject: String, schema: Schema): IO[SchemaVersion] = + override def getVersion(subject: String, schema: Schema, useExponentialBackoffRetryPolicy: Boolean): IO[SchemaVersion] = ref.get.map(testState => testState.numSchemasRegistered + 1) - override def getAllVersions(subject: String): IO[List[Int]] = IO.pure(List()) + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): IO[List[Int]] = IO.pure(List()) override def getAllSubjects: IO[List[String]] = IO.pure(List()) override def getSchemaRegistryClient: IO[SchemaRegistryClient] = IO.raiseError(new Exception("Something horrible went wrong!")) @@ -104,8 +104,8 @@ class CreateTopicProgramSpec extends AsyncFreeSpec with Matchers with IOSuite { ref.get.flatMap(n => ref.set(n + 1) *> IO.raiseError(new Exception("Something horrible went wrong!"))) override def deleteSchemaOfVersion(subject: String, version: SchemaVersion): IO[Unit] = IO.unit - override def getVersion(subject: String, schema: Schema): IO[SchemaVersion] = IO.pure(1) - override def getAllVersions(subject: String): IO[List[Int]] = IO.pure(Nil) + override def getVersion(subject: String, schema: Schema, useExponentialBackoffRetryPolicy: Boolean): IO[SchemaVersion] = IO.pure(1) + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): IO[List[Int]] = IO.pure(Nil) override def getAllSubjects: IO[List[String]] = IO.pure(Nil) override def getSchemaRegistryClient: IO[SchemaRegistryClient] = IO.raiseError(new Exception("Something horrible went wrong!")) override def getLatestSchemaBySubject(subject: String): IO[Option[Schema]] = IO.pure(None) @@ -135,8 +135,8 @@ class CreateTopicProgramSpec extends AsyncFreeSpec with Matchers with IOSuite { override def deleteSchemaOfVersion(subject: String, version: SchemaVersion): IO[Unit] = ref.update(ts => ts.copy(schemas = ts.schemas - subject)) - override def getVersion(subject: String, schema: Schema): IO[SchemaVersion] = ref.get.map(_.schemas(subject)) - override def getAllVersions(subject: String): IO[List[Int]] = IO.pure(Nil) + override def getVersion(subject: String, schema: Schema, useExponentialBackoffRetryPolicy: Boolean): IO[SchemaVersion] = ref.get.map(_.schemas(subject)) + override def getAllVersions(subject: String, useExponentialBackoffRetryPolicy: Boolean): IO[List[Int]] = IO.pure(Nil) override def getAllSubjects: IO[List[String]] = IO.pure(Nil) override def getSchemaRegistryClient: IO[SchemaRegistryClient] = IO.raiseError(new Exception) override def getLatestSchemaBySubject(subject: String): IO[Option[Schema]] = IO.pure(None)