From a297dd020f991af91a997e287edb091e285aba00 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 14:27:55 +0100 Subject: [PATCH 1/9] PI-1478 changes to retry exceptions --- libs/commons/build.gradle.kts | 1 + .../gov/justice/digital/hmpps/retry/Retry.kt | 21 ++++++++++---- .../justice/digital/hmpps/retry/RetryTest.kt | 28 +++++++++++++++++++ .../hmpps/listener/AwsNotificationListener.kt | 8 +++++- 4 files changed, 52 insertions(+), 6 deletions(-) diff --git a/libs/commons/build.gradle.kts b/libs/commons/build.gradle.kts index ef67056a62..0b8b278d19 100644 --- a/libs/commons/build.gradle.kts +++ b/libs/commons/build.gradle.kts @@ -7,6 +7,7 @@ dependencies { implementation("com.fasterxml.jackson.module:jackson-module-kotlin") implementation(libs.bundles.telemetry) implementation(libs.flipt) + implementation(libs.openfeign) testImplementation("org.springframework.boot:spring-boot-starter-data-ldap") testImplementation("org.springframework.boot:spring-boot-starter-test") diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt index cf38551e1c..03e74eb34f 100644 --- a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt +++ b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt @@ -1,13 +1,24 @@ package uk.gov.justice.digital.hmpps.retry -fun retry(maxRetries: Int, code: () -> T): T { - var throwable: Throwable? = null - (1..maxRetries).forEach { _ -> +import kotlin.reflect.KClass + +fun retry(maxRetries: Int, exceptions: List> = listOf(Exception::class), code: () -> T): T { + var throwable: Throwable? + (1..maxRetries).forEach { count -> try { return code() } catch (e: Throwable) { - throwable = e + val matchedException = exceptions.firstOrNull { it.isInstance(e) } + throwable = if (matchedException != null && count < maxRetries) { + null + } else { + e + } + + if (throwable != null) { + throw throwable!! + } } } - throw throwable!! + throw RuntimeException("unknown error") } diff --git a/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt b/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt index ed0455696c..68b7da7701 100644 --- a/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt +++ b/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt @@ -2,9 +2,12 @@ package uk.gov.justice.digital.hmpps.retry import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo +import org.json.JSONException import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows +import org.springframework.dao.OptimisticLockingFailureException import java.rmi.UnexpectedException +import java.sql.SQLException import java.util.concurrent.atomic.AtomicInteger internal class RetryTest { @@ -29,4 +32,29 @@ internal class RetryTest { } assertThat(result, equalTo(1)) } + + @Test + fun `when optimistic lock exception thrown retry until max retries`() { + val counter = AtomicInteger(0) + assertThrows { + retry(3, listOf(OptimisticLockingFailureException::class, JSONException::class)) { + counter.incrementAndGet() + throw OptimisticLockingFailureException("OLE") + } + } + assertThat(counter.get(), equalTo(3)) + } + + @Test + fun `when SQL exception thrown no retries`() { + val counter = AtomicInteger(0) + assertThrows { + retry(3, listOf(OptimisticLockingFailureException::class)) { + counter.incrementAndGet() + throw SQLException("SQLE") + } + } + assertThat(counter.get(), equalTo(1)) + } } + diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index d1e739cd3b..d0640b98f9 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt @@ -12,6 +12,7 @@ import org.springframework.context.annotation.Conditional import org.springframework.stereotype.Component import uk.gov.justice.digital.hmpps.config.AwsCondition import uk.gov.justice.digital.hmpps.messaging.NotificationHandler +import uk.gov.justice.digital.hmpps.retry.retry import java.util.concurrent.CompletionException @Component @@ -25,7 +26,12 @@ class AwsNotificationListener( @WithSpan(kind = SpanKind.CONSUMER) fun receive(message: String) { try { - handler.handle(message) + retry(3,listOf(ObjectOptimisticLockingFailureException::class, + ConstraintViolationException::class, + RetryableException::class, + CannotCreateTransactionException::class, + FeignException.NotFound::class, + CannotGetJdbcConnectionException::class,)) { handler.handle(message) } } catch (e: Throwable) { Sentry.captureException(unwrapSqsExceptions(e)) throw e From 433579c102b278391a84fed3377e159bc8a78d04 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 14:46:04 +0100 Subject: [PATCH 2/9] PI-1478 changes to retry exceptions --- libs/commons/build.gradle.kts | 1 - libs/messaging/build.gradle.kts | 2 ++ .../hmpps/listener/AwsNotificationListener.kt | 22 ++++++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/libs/commons/build.gradle.kts b/libs/commons/build.gradle.kts index 0b8b278d19..ef67056a62 100644 --- a/libs/commons/build.gradle.kts +++ b/libs/commons/build.gradle.kts @@ -7,7 +7,6 @@ dependencies { implementation("com.fasterxml.jackson.module:jackson-module-kotlin") implementation(libs.bundles.telemetry) implementation(libs.flipt) - implementation(libs.openfeign) testImplementation("org.springframework.boot:spring-boot-starter-data-ldap") testImplementation("org.springframework.boot:spring-boot-starter-test") diff --git a/libs/messaging/build.gradle.kts b/libs/messaging/build.gradle.kts index 5ec0ce6bb3..12f97f197e 100644 --- a/libs/messaging/build.gradle.kts +++ b/libs/messaging/build.gradle.kts @@ -5,6 +5,8 @@ dependencies { implementation("org.springframework.boot:spring-boot-starter-web") implementation("com.fasterxml.jackson.module:jackson-module-kotlin") implementation(libs.bundles.telemetry) + compileOnly(libs.openfeign) + compileOnly("org.springframework.boot:spring-boot-starter-data-jpa") api(libs.bundles.aws.messaging) diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index d0640b98f9..0c08b153ce 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt @@ -1,5 +1,7 @@ package uk.gov.justice.digital.hmpps.listener +import feign.FeignException +import feign.RetryableException import io.awspring.cloud.sqs.annotation.SqsListener import io.awspring.cloud.sqs.listener.AsyncAdapterBlockingExecutionFailedException import io.awspring.cloud.sqs.listener.ListenerExecutionFailedException @@ -7,9 +9,13 @@ import io.opentelemetry.api.trace.SpanKind import io.opentelemetry.instrumentation.annotations.WithSpan import io.sentry.Sentry import io.sentry.spring.jakarta.tracing.SentryTransaction +import org.hibernate.exception.ConstraintViolationException import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression import org.springframework.context.annotation.Conditional +import org.springframework.jdbc.CannotGetJdbcConnectionException +import org.springframework.orm.ObjectOptimisticLockingFailureException import org.springframework.stereotype.Component +import org.springframework.transaction.CannotCreateTransactionException import uk.gov.justice.digital.hmpps.config.AwsCondition import uk.gov.justice.digital.hmpps.messaging.NotificationHandler import uk.gov.justice.digital.hmpps.retry.retry @@ -26,12 +32,16 @@ class AwsNotificationListener( @WithSpan(kind = SpanKind.CONSUMER) fun receive(message: String) { try { - retry(3,listOf(ObjectOptimisticLockingFailureException::class, - ConstraintViolationException::class, - RetryableException::class, - CannotCreateTransactionException::class, - FeignException.NotFound::class, - CannotGetJdbcConnectionException::class,)) { handler.handle(message) } + retry( + 3, listOf( + ObjectOptimisticLockingFailureException::class, + ConstraintViolationException::class, + RetryableException::class, + CannotCreateTransactionException::class, + FeignException.NotFound::class, + CannotGetJdbcConnectionException::class, + ) + ) { handler.handle(message) } } catch (e: Throwable) { Sentry.captureException(unwrapSqsExceptions(e)) throw e From 97dc2519f4284bdc49ec407410e5e3c1e5cba1b3 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 14:49:45 +0100 Subject: [PATCH 3/9] PI-1478 changes to retry exceptions --- .../kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt | 1 - .../digital/hmpps/listener/AwsNotificationListener.kt | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt b/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt index 68b7da7701..f5e86979ac 100644 --- a/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt +++ b/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/retry/RetryTest.kt @@ -57,4 +57,3 @@ internal class RetryTest { assertThat(counter.get(), equalTo(1)) } } - diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index 0c08b153ce..f28868ea01 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt @@ -33,13 +33,14 @@ class AwsNotificationListener( fun receive(message: String) { try { retry( - 3, listOf( + 3, + listOf( ObjectOptimisticLockingFailureException::class, ConstraintViolationException::class, RetryableException::class, CannotCreateTransactionException::class, FeignException.NotFound::class, - CannotGetJdbcConnectionException::class, + CannotGetJdbcConnectionException::class ) ) { handler.handle(message) } } catch (e: Throwable) { From 51fd4628568d8453e3e66946e46171d5d01f3611 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 14:58:21 +0100 Subject: [PATCH 4/9] PI-1478 changes to retry exceptions --- libs/messaging/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/messaging/build.gradle.kts b/libs/messaging/build.gradle.kts index 12f97f197e..9743d0a5db 100644 --- a/libs/messaging/build.gradle.kts +++ b/libs/messaging/build.gradle.kts @@ -5,8 +5,8 @@ dependencies { implementation("org.springframework.boot:spring-boot-starter-web") implementation("com.fasterxml.jackson.module:jackson-module-kotlin") implementation(libs.bundles.telemetry) - compileOnly(libs.openfeign) - compileOnly("org.springframework.boot:spring-boot-starter-data-jpa") + implementation(libs.openfeign) + implementation("org.springframework.boot:spring-boot-starter-data-jpa") api(libs.bundles.aws.messaging) From 2afd90ea9aaaeedb478ca8776948dec1ee72fb24 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 15:17:55 +0100 Subject: [PATCH 5/9] PI-1478 changes to retry exceptions --- libs/messaging/build.gradle.kts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/messaging/build.gradle.kts b/libs/messaging/build.gradle.kts index 9743d0a5db..a92cd0a765 100644 --- a/libs/messaging/build.gradle.kts +++ b/libs/messaging/build.gradle.kts @@ -5,13 +5,15 @@ dependencies { implementation("org.springframework.boot:spring-boot-starter-web") implementation("com.fasterxml.jackson.module:jackson-module-kotlin") implementation(libs.bundles.telemetry) - implementation(libs.openfeign) - implementation("org.springframework.boot:spring-boot-starter-data-jpa") + compileOnly(libs.openfeign) + compileOnly("org.springframework.boot:spring-boot-starter-data-jpa") api(libs.bundles.aws.messaging) testImplementation("org.springframework.boot:spring-boot-starter-test") testImplementation(libs.bundles.mockito) + testImplementation(libs.openfeign) + testImplementation("org.springframework.boot:spring-boot-starter-data-jpa") } configure { From 81ab95a894d6a913a6b2b6e15c70de538d22f7a8 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 15:29:29 +0100 Subject: [PATCH 6/9] PI-1478 changes to retry exceptions --- .../digital/hmpps/listener/AwsNotificationListener.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index f28868ea01..bc932082c5 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt @@ -9,7 +9,6 @@ import io.opentelemetry.api.trace.SpanKind import io.opentelemetry.instrumentation.annotations.WithSpan import io.sentry.Sentry import io.sentry.spring.jakarta.tracing.SentryTransaction -import org.hibernate.exception.ConstraintViolationException import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression import org.springframework.context.annotation.Conditional import org.springframework.jdbc.CannotGetJdbcConnectionException @@ -35,11 +34,10 @@ class AwsNotificationListener( retry( 3, listOf( - ObjectOptimisticLockingFailureException::class, - ConstraintViolationException::class, + FeignException.NotFound::class, RetryableException::class, + ObjectOptimisticLockingFailureException::class, CannotCreateTransactionException::class, - FeignException.NotFound::class, CannotGetJdbcConnectionException::class ) ) { handler.handle(message) } From aebb305b806856987326daad35132450dcd9c65d Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 16:38:23 +0100 Subject: [PATCH 7/9] PI-1478 changes to retry exceptions --- .../uk/gov/justice/digital/hmpps/retry/Retry.kt | 14 +++++++++++--- .../hmpps/listener/AwsNotificationListener.kt | 4 ++-- .../digital/hmpps/config/feign/FeignConfig.kt | 4 ++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt index 03e74eb34f..b2dab5ec19 100644 --- a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt +++ b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt @@ -1,8 +1,15 @@ package uk.gov.justice.digital.hmpps.retry +import java.time.Duration +import java.util.concurrent.TimeUnit import kotlin.reflect.KClass -fun retry(maxRetries: Int, exceptions: List> = listOf(Exception::class), code: () -> T): T { +fun retry( + maxRetries: Int, + delay: Duration = Duration.ofMillis(100), + exceptions: List> = listOf(Exception::class), + code: () -> T +): T { var throwable: Throwable? (1..maxRetries).forEach { count -> try { @@ -14,8 +21,9 @@ fun retry(maxRetries: Int, exceptions: List> = listOf( } else { e } - - if (throwable != null) { + if (throwable == null) { + TimeUnit.MILLISECONDS.sleep(delay.toMillis() * count * count) + }else{ throw throwable!! } } diff --git a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt index bc932082c5..65db7b458f 100644 --- a/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt +++ b/libs/messaging/src/main/kotlin/uk/gov/justice/digital/hmpps/listener/AwsNotificationListener.kt @@ -1,7 +1,6 @@ package uk.gov.justice.digital.hmpps.listener import feign.FeignException -import feign.RetryableException import io.awspring.cloud.sqs.annotation.SqsListener import io.awspring.cloud.sqs.listener.AsyncAdapterBlockingExecutionFailedException import io.awspring.cloud.sqs.listener.ListenerExecutionFailedException @@ -11,6 +10,7 @@ import io.sentry.Sentry import io.sentry.spring.jakarta.tracing.SentryTransaction import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression import org.springframework.context.annotation.Conditional +import org.springframework.dao.CannotAcquireLockException import org.springframework.jdbc.CannotGetJdbcConnectionException import org.springframework.orm.ObjectOptimisticLockingFailureException import org.springframework.stereotype.Component @@ -35,7 +35,7 @@ class AwsNotificationListener( 3, listOf( FeignException.NotFound::class, - RetryableException::class, + CannotAcquireLockException::class, ObjectOptimisticLockingFailureException::class, CannotCreateTransactionException::class, CannotGetJdbcConnectionException::class diff --git a/libs/oauth-client/src/main/kotlin/uk/gov/justice/digital/hmpps/config/feign/FeignConfig.kt b/libs/oauth-client/src/main/kotlin/uk/gov/justice/digital/hmpps/config/feign/FeignConfig.kt index 640cc2548c..62ea960dd1 100644 --- a/libs/oauth-client/src/main/kotlin/uk/gov/justice/digital/hmpps/config/feign/FeignConfig.kt +++ b/libs/oauth-client/src/main/kotlin/uk/gov/justice/digital/hmpps/config/feign/FeignConfig.kt @@ -1,6 +1,7 @@ package uk.gov.justice.digital.hmpps.config.feign import feign.RequestInterceptor +import feign.Retryer import org.springframework.context.annotation.Bean import org.springframework.http.HttpHeaders import org.springframework.security.authentication.AnonymousAuthenticationToken @@ -16,6 +17,9 @@ abstract class FeignConfig( abstract fun registrationId(): String + @Bean + open fun retryer() = Retryer.Default() + @Bean open fun requestInterceptor() = RequestInterceptor { template -> template.header(HttpHeaders.AUTHORIZATION, "Bearer ${getAccessToken()}") From 97b4765edf33da0aaf5802f9abc1683b994a315f Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 16:39:04 +0100 Subject: [PATCH 8/9] PI-1478 changes to retry exceptions --- .../src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt index b2dab5ec19..863d3992ac 100644 --- a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt +++ b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt @@ -6,8 +6,8 @@ import kotlin.reflect.KClass fun retry( maxRetries: Int, - delay: Duration = Duration.ofMillis(100), exceptions: List> = listOf(Exception::class), + delay: Duration = Duration.ofMillis(100), code: () -> T ): T { var throwable: Throwable? From 3a8f9a99c578a611f2e95afbcb49a86ac14641f0 Mon Sep 17 00:00:00 2001 From: stevomcallister Date: Thu, 21 Sep 2023 16:41:42 +0100 Subject: [PATCH 9/9] PI-1478 changes to retry exceptions --- .../src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt index 863d3992ac..c3b9f565c2 100644 --- a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt +++ b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/retry/Retry.kt @@ -23,7 +23,7 @@ fun retry( } if (throwable == null) { TimeUnit.MILLISECONDS.sleep(delay.toMillis() * count * count) - }else{ + } else { throw throwable!! } }