diff --git a/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java index 3b402856e80..b30b11d36f7 100644 --- a/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java @@ -99,6 +99,7 @@ public class RetryAnnotationBeanPostProcessor extends AbstractBeanFactoryAwareAd Arrays.asList(retryable.includes()), Arrays.asList(retryable.excludes()), instantiatePredicate(retryable.predicate()), parseLong(retryable.maxRetries(), retryable.maxRetriesString()), + parseDuration(retryable.timeout(), retryable.timeoutString(), timeUnit), parseDuration(retryable.delay(), retryable.delayString(), timeUnit), parseDuration(retryable.jitter(), retryable.jitterString(), timeUnit), parseDouble(retryable.multiplier(), retryable.multiplierString()), diff --git a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java index b9851b25852..6e556ecbafe 100644 --- a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java +++ b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java @@ -122,6 +122,39 @@ public @interface Retryable { */ String maxRetriesString() default ""; + /** + * The maximum amount of elapsed time allowed for the initial invocation and + * any subsequent retry attempts, including delays. + *

The default is {@code 0}, which signals that no timeout should be applied. + *

The time unit is milliseconds by default but can be overridden via + * {@link #timeUnit}. + *

Must be greater than or equal to zero. + * @since 7.0.2 + */ + long timeout() default 0; + + /** + * The timeout, as a duration String. + *

A non-empty value specified here overrides the {@link #timeout()} attribute. + *

The duration String can be in several formats: + *

+ * @return the timeout as a String value — for example, a placeholder, a + * {@link org.springframework.format.annotation.DurationFormat.Style#ISO8601 java.time.Duration} compliant value, + * or a {@link org.springframework.format.annotation.DurationFormat.Style#SIMPLE simple format} compliant value + * @since 7.0.2 + * @see #timeout() + */ + String timeoutString() default ""; + /** * The base delay after the initial invocation. If a multiplier is specified, * this serves as the initial delay to multiply from. diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java b/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java index 67964fb366e..4f8f1793302 100644 --- a/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java +++ b/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java @@ -17,6 +17,7 @@ package org.springframework.resilience.retry; import java.lang.reflect.Method; +import java.time.Duration; import java.util.concurrent.Future; import org.aopalliance.intercept.MethodInterceptor; @@ -94,6 +95,7 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { .excludes(spec.excludes()) .predicate(spec.predicate().forMethod(method)) .maxRetries(spec.maxRetries()) + .timeout(spec.timeout()) .delay(spec.delay()) .jitter(spec.jitter()) .multiplier(spec.multiplier()) @@ -142,8 +144,20 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { .multiplier(spec.multiplier()) .maxBackoff(spec.maxDelay()) .filter(spec.combinedPredicate().forMethod(method)); - publisher = (adapter.isMultiValue() ? Flux.from(publisher).retryWhen(retry) : - Mono.from(publisher).retryWhen(retry)); + + Duration timeout = spec.timeout(); + boolean timeoutIsPositive = (!timeout.isNegative() && !timeout.isZero()); + if (adapter.isMultiValue()) { + publisher = (timeoutIsPositive ? + Flux.from(publisher).retryWhen(retry).timeout(timeout) : + Flux.from(publisher).retryWhen(retry)); + } + else { + publisher = (timeoutIsPositive ? + Mono.from(publisher).retryWhen(retry).timeout(timeout) : + Mono.from(publisher).retryWhen(retry)); + } + return adapter.fromPublisher(publisher); } diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java index f07d777f4db..59992d0e473 100644 --- a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java +++ b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java @@ -28,11 +28,14 @@ import org.springframework.util.ExceptionTypeFilter; * on {@link org.springframework.resilience.annotation.Retryable}. * * @author Juergen Hoeller + * @author Sam Brannen * @since 7.0 * @param includes applicable exception types to attempt a retry for * @param excludes non-applicable exception types to avoid a retry for * @param predicate a predicate for filtering exceptions from applicable methods * @param maxRetries the maximum number of retry attempts + * @param timeout the maximum amount of elapsed time allowed for the initial + * invocation and any subsequent retry attempts, including delays * @param delay the base delay after the initial invocation * @param jitter a jitter value for the next retry attempt * @param multiplier a multiplier for a delay for the next retry attempt @@ -46,20 +49,40 @@ public record MethodRetrySpec( Collection> excludes, MethodRetryPredicate predicate, long maxRetries, + Duration timeout, Duration delay, Duration jitter, double multiplier, Duration maxDelay) { + /** + * Construct a new {@code MethodRetryPredicate} with the supplied arguments. + */ public MethodRetrySpec(MethodRetryPredicate predicate, long maxRetries, Duration delay) { this(predicate, maxRetries, delay, Duration.ZERO, 1.0, Duration.ofMillis(Long.MAX_VALUE)); } + /** + * Construct a new {@code MethodRetryPredicate} with the supplied arguments. + */ public MethodRetrySpec(MethodRetryPredicate predicate, long maxRetries, Duration delay, Duration jitter, double multiplier, Duration maxDelay) { - this(Collections.emptyList(), Collections.emptyList(), predicate, maxRetries, delay, - jitter, multiplier, maxDelay); + this(Collections.emptyList(), Collections.emptyList(), predicate, maxRetries, Duration.ZERO, + delay, jitter, multiplier, maxDelay); + } + + /** + * Construct a new {@code MethodRetryPredicate} with the supplied arguments. + * @deprecated as of Spring Framework 7.0.2, in favor of + * {@link #MethodRetrySpec(Collection, Collection, MethodRetryPredicate, long, Duration, Duration, Duration, double, Duration)} + */ + @Deprecated(since = "7.0.2", forRemoval = true) + public MethodRetrySpec(Collection> includes, + Collection> excludes, MethodRetryPredicate predicate, + long maxRetries, Duration delay, Duration jitter, double multiplier, Duration maxDelay) { + + this(includes, excludes, predicate, maxRetries, Duration.ZERO, delay, jitter, multiplier, maxDelay); } diff --git a/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java b/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java index b0970e97a5b..22c0f5552e7 100644 --- a/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java @@ -21,9 +21,11 @@ import java.nio.charset.MalformedInputException; import java.nio.file.AccessDeniedException; import java.nio.file.FileSystemException; import java.time.Duration; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import org.assertj.core.api.ThrowingConsumer; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import reactor.core.Exceptions; import reactor.core.publisher.Flux; @@ -316,6 +318,83 @@ class ReactiveRetryInterceptorTests { } + @Nested + class TimeoutTests { + + private final AnnotatedMethodBean proxy = getProxiedAnnotatedMethodBean(); + private final AnnotatedMethodBean target = (AnnotatedMethodBean) AopProxyUtils.getSingletonTarget(proxy); + + @Test + void timeoutNotExceededAfterInitialSuccess() { + String result = proxy.retryOperationWithTimeoutNotExceededAfterInitialSuccess().block(); + assertThat(result).isEqualTo("success"); + // 1 initial attempt + 0 retries + assertThat(target.counter).hasValue(1); + } + + @Test + void timeoutNotExceededAndRetriesExhausted() { + assertThatIllegalStateException() + .isThrownBy(() -> proxy.retryOperationWithTimeoutNotExceededAndRetriesExhausted().block()) + .satisfies(isRetryExhaustedException()) + .havingCause() + .isInstanceOf(IOException.class) + .withMessage("4"); + // 1 initial attempt + 3 retries + assertThat(target.counter).hasValue(4); + } + + @Test + void timeoutExceededAfterInitialFailure() { + assertThatRuntimeException() + .isThrownBy(() -> proxy.retryOperationWithTimeoutExceededAfterInitialFailure().block()) + .satisfies(isReactiveException()) + .havingCause() + .isInstanceOf(TimeoutException.class) + .withMessageContaining("within 5ms"); + // 1 initial attempt + 0 retries + assertThat(target.counter).hasValue(1); + } + + @Test + void timeoutExceededAfterFirstDelayButBeforeFirstRetry() { + assertThatRuntimeException() + .isThrownBy(() -> proxy.retryOperationWithTimeoutExceededAfterFirstDelayButBeforeFirstRetry().block()) + .satisfies(isReactiveException()) + .havingCause() + .isInstanceOf(TimeoutException.class) + .withMessageContaining("within 5ms"); + // 1 initial attempt + 0 retries + assertThat(target.counter).hasValue(1); + } + + @Test + void timeoutExceededAfterFirstRetry() { + assertThatRuntimeException() + .isThrownBy(() -> proxy.retryOperationWithTimeoutExceededAfterFirstRetry().block()) + .satisfies(isReactiveException()) + .havingCause() + .isInstanceOf(TimeoutException.class) + .withMessageContaining("within 5ms"); + // 1 initial attempt + 1 retry + assertThat(target.counter).hasValue(2); + } + + @Test + void timeoutExceededAfterSecondRetry() { + assertThatRuntimeException() + .isThrownBy(() -> proxy.retryOperationWithTimeoutExceededAfterSecondRetry().block()) + .satisfies(isReactiveException()) + .havingCause() + .isInstanceOf(TimeoutException.class) + .withMessageContaining("within 5ms"); + // 1 initial attempt + 2 retries + assertThat(target.counter).hasValue(3); + } + + } + + private static ThrowingConsumer isReactiveException() { return ex -> assertThat(ex.getClass().getName()).isEqualTo("reactor.core.Exceptions$ReactiveException"); } @@ -368,6 +447,61 @@ class ReactiveRetryInterceptorTests { throw new IOException(counter.toString()); }); } + + @Retryable(timeout = 555, delay = 10) + public Mono retryOperationWithTimeoutNotExceededAfterInitialSuccess() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + return "success"; + }); + } + + @Retryable(timeout = 555, delay = 10) + public Mono retryOperationWithTimeoutNotExceededAndRetriesExhausted() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + throw new IOException(counter.toString()); + }); + } + + @Retryable(timeout = 5, delay = 10) + public Mono retryOperationWithTimeoutExceededAfterInitialFailure() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + Thread.sleep(10); + throw new IOException(counter.toString()); + }); + } + + @Retryable(timeout = 5, delay = 10) + public Mono retryOperationWithTimeoutExceededAfterFirstDelayButBeforeFirstRetry() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + throw new IOException(counter.toString()); + }); + } + + @Retryable(timeout = 5, delay = 0) + public Mono retryOperationWithTimeoutExceededAfterFirstRetry() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + if (counter.get() == 2) { + Thread.sleep(10); + } + throw new IOException(counter.toString()); + }); + } + + @Retryable(timeout = 5, delay = 0) + public Mono retryOperationWithTimeoutExceededAfterSecondRetry() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + if (counter.get() == 3) { + Thread.sleep(10); + } + throw new IOException(counter.toString()); + }); + } } diff --git a/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java b/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java index e7b9c6956c0..8c6ae537e50 100644 --- a/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java @@ -27,6 +27,7 @@ import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.aopalliance.intercept.MethodInterceptor; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.aop.config.AopConfigUtils; @@ -312,6 +313,69 @@ class RetryInterceptorTests { } + @Nested + class TimeoutTests { + + private final DefaultListableBeanFactory bf = createBeanFactoryFor(AnnotatedMethodBean.class); + private final AnnotatedMethodBean proxy = bf.getBean(AnnotatedMethodBean.class); + private final AnnotatedMethodBean target = (AnnotatedMethodBean) AopProxyUtils.getSingletonTarget(proxy); + + @Test + void timeoutNotExceededAfterInitialSuccess() { + String result = proxy.retryOperationWithTimeoutNotExceededAfterInitialSuccess(); + assertThat(result).isEqualTo("success"); + // 1 initial attempt + 0 retries + assertThat(target.counter).isEqualTo(1); + } + + @Test + void timeoutNotExceededAndRetriesExhausted() { + assertThatIOException() + .isThrownBy(proxy::retryOperationWithTimeoutNotExceededAndRetriesExhausted) + .withMessage("4"); + // 1 initial attempt + 3 retries + assertThat(target.counter).isEqualTo(4); + } + + @Test + void timeoutExceededAfterInitialFailure() { + assertThatIOException() + .isThrownBy(proxy::retryOperationWithTimeoutExceededAfterInitialFailure) + .withMessage("1"); + // 1 initial attempt + 0 retries + assertThat(target.counter).isEqualTo(1); + } + + @Test + void timeoutExceededAfterFirstDelayButBeforeFirstRetry() { + assertThatIOException() + .isThrownBy(proxy::retryOperationWithTimeoutExceededAfterFirstDelayButBeforeFirstRetry) + .withMessage("1"); + // 1 initial attempt + 0 retries + assertThat(target.counter).isEqualTo(1); + } + + @Test + void timeoutExceededAfterFirstRetry() { + assertThatIOException() + .isThrownBy(proxy::retryOperationWithTimeoutExceededAfterFirstRetry) + .withMessage("2"); + // 1 initial attempt + 1 retry + assertThat(target.counter).isEqualTo(2); + } + + @Test + void timeoutExceededAfterSecondRetry() { + assertThatIOException() + .isThrownBy(proxy::retryOperationWithTimeoutExceededAfterSecondRetry) + .withMessage("3"); + // 1 initial attempt + 2 retries + assertThat(target.counter).isEqualTo(3); + } + + } + + private static DefaultListableBeanFactory createBeanFactoryFor(Class beanClass) { DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); bf.registerBeanDefinition("bean", new RootBeanDefinition(beanClass)); @@ -349,6 +413,49 @@ class RetryInterceptorTests { counter++; throw new IOException(Integer.toString(counter)); } + + @Retryable(timeout = 555, delay = 10) + public String retryOperationWithTimeoutNotExceededAfterInitialSuccess() { + counter++; + return "success"; + } + + @Retryable(timeout = 555, delay = 10) + public void retryOperationWithTimeoutNotExceededAndRetriesExhausted() throws Exception { + counter++; + throw new IOException(Integer.toString(counter)); + } + + @Retryable(timeout = 5, delay = 10) + public void retryOperationWithTimeoutExceededAfterInitialFailure() throws Exception { + counter++; + Thread.sleep(10); + throw new IOException(Integer.toString(counter)); + } + + @Retryable(timeout = 5, delay = 10) + public void retryOperationWithTimeoutExceededAfterFirstDelayButBeforeFirstRetry() throws IOException { + counter++; + throw new IOException(Integer.toString(counter)); + } + + @Retryable(timeout = 5, delay = 0) + public void retryOperationWithTimeoutExceededAfterFirstRetry() throws Exception { + counter++; + if (counter == 2) { + Thread.sleep(10); + } + throw new IOException(Integer.toString(counter)); + } + + @Retryable(timeout = 5, delay = 0) + public void retryOperationWithTimeoutExceededAfterSecondRetry() throws Exception { + counter++; + if (counter == 3) { + Thread.sleep(10); + } + throw new IOException(Integer.toString(counter)); + } } diff --git a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java index 703bce18e7a..50a1c049aa1 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java @@ -16,6 +16,7 @@ package org.springframework.core.retry; +import java.time.Duration; import java.util.Set; import java.util.StringJoiner; import java.util.function.Predicate; @@ -42,16 +43,19 @@ class DefaultRetryPolicy implements RetryPolicy { private final @Nullable Predicate predicate; + private final Duration timeout; + private final BackOff backOff; DefaultRetryPolicy(Set> includes, Set> excludes, - @Nullable Predicate predicate, BackOff backOff) { + @Nullable Predicate predicate, Duration timeout, BackOff backOff) { this.includes = includes; this.excludes = excludes; this.exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes); this.predicate = predicate; + this.timeout = timeout; this.backOff = backOff; } @@ -62,6 +66,11 @@ class DefaultRetryPolicy implements RetryPolicy { (this.predicate == null || this.predicate.test(throwable))); } + @Override + public Duration getTimeout() { + return this.timeout; + } + @Override public BackOff getBackOff() { return this.backOff; diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java b/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java index 7bfab43f979..f237751f63a 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryListener.java @@ -87,4 +87,20 @@ public interface RetryListener { default void onRetryPolicyInterruption(RetryPolicy retryPolicy, Retryable retryable, RetryException exception) { } + /** + * Called if the configured {@linkplain RetryPolicy#getTimeout() timeout} for + * a {@link RetryPolicy} is exceeded. + * @param retryPolicy the {@code RetryPolicy} + * @param retryable the {@link Retryable} operation + * @param exception the resulting {@link RetryException}, with the last + * exception thrown by the {@code Retryable} operation as the cause and any + * exceptions from previous attempts as suppressed exceptions + * @since 7.0.2 + * @see RetryException#getCause() + * @see RetryException#getSuppressed() + * @see RetryException#getRetryCount() + */ + default void onRetryPolicyTimeout(RetryPolicy retryPolicy, Retryable retryable, RetryException exception) { + } + } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java index ebc9278b4db..5c295436031 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java @@ -55,6 +55,21 @@ public interface RetryPolicy { */ boolean shouldRetry(Throwable throwable); + /** + * Get the timeout to use for this retry policy. + *

The returned {@link Duration} represents the maximum amount of elapsed + * time allowed for the initial invocation and any subsequent retry attempts, + * including delays. + *

Defaults to {@link Duration#ZERO} which signals that no timeout should + * be applied. + * @return the timeout to apply + * @since 7.0.2 + * @see Builder#timeout(Duration) + */ + default Duration getTimeout() { + return Duration.ZERO; + } + /** * Get the {@link BackOff} strategy to use for this retry policy. *

Defaults to a fixed backoff of {@value Builder#DEFAULT_DELAY} milliseconds @@ -158,6 +173,8 @@ public interface RetryPolicy { private @Nullable Long maxRetries; + private Duration timeout = Duration.ZERO; + private @Nullable Duration delay; private @Nullable Duration jitter; @@ -214,6 +231,24 @@ public interface RetryPolicy { return this; } + /** + * Specify a timeout for the maximum amount of elapsed time allowed for + * the initial invocation and any subsequent retry attempts, including + * delays. + *

The default is {@link Duration#ZERO}, which signals that no timeout + * should be applied. + *

The supplied value will override any previously configured value. + * @param timeout the timeout, typically in milliseconds or seconds; + * must be greater than or equal to zero + * @return this {@code Builder} instance for chained method invocations + * @since 7.0.2 + */ + public Builder timeout(Duration timeout) { + assertIsNotNegative("timeout", timeout); + this.timeout = timeout; + return this; + } + /** * Specify the base delay after the initial invocation. *

If a {@linkplain #multiplier(double) multiplier} is specified, this @@ -441,7 +476,7 @@ public interface RetryPolicy { } backOff = exponentialBackOff; } - return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, backOff); + return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, this.timeout, backOff); } } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java index 1870a7632e6..95b024e8370 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java @@ -136,6 +136,7 @@ public class RetryTemplate implements RetryOperations { @Override public R execute(Retryable retryable) throws RetryException { String retryableName = retryable.getName(); + long startTime = System.currentTimeMillis(); // Initial attempt try { logger.debug(() -> "Preparing to execute retryable operation '%s'".formatted(retryableName)); @@ -153,12 +154,15 @@ public class RetryTemplate implements RetryOperations { exceptions.add(initialException); Throwable lastException = initialException; + long timeout = this.retryPolicy.getTimeout().toMillis(); while (this.retryPolicy.shouldRetry(lastException)) { + checkIfTimeoutExceeded(timeout, startTime, 0, retryable, exceptions); try { long sleepTime = backOffExecution.nextBackOff(); if (sleepTime == BackOffExecution.STOP) { break; } + checkIfTimeoutExceeded(timeout, startTime, sleepTime, retryable, exceptions); logger.debug(() -> "Backing off for %dms after retryable operation '%s'" .formatted(sleepTime, retryableName)); Thread.sleep(sleepTime); @@ -201,6 +205,24 @@ public class RetryTemplate implements RetryOperations { } } + private void checkIfTimeoutExceeded(long timeout, long startTime, long sleepTime, Retryable retryable, + Deque exceptions) throws RetryException { + + if (timeout != 0) { + // If sleepTime > 0, we are predicting what the effective elapsed time + // would be if we were to sleep for sleepTime milliseconds. + long elapsedTime = System.currentTimeMillis() + sleepTime - startTime; + if (elapsedTime >= timeout) { + RetryException retryException = new RetryException( + "Retry policy for operation '%s' exceeded timeout (%d ms); aborting execution" + .formatted(retryable.getName(), timeout), exceptions.removeLast()); + exceptions.forEach(retryException::addSuppressed); + this.retryListener.onRetryPolicyTimeout(this.retryPolicy, retryable, retryException); + throw retryException; + } + } + } + private static class RetryInterruptedException extends RetryException { diff --git a/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java b/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java index cdf02bf33f3..5d3c7a62665 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java +++ b/spring-core/src/main/java/org/springframework/core/retry/support/CompositeRetryListener.java @@ -95,4 +95,9 @@ public class CompositeRetryListener implements RetryListener { this.listeners.forEach(listener -> listener.onRetryPolicyInterruption(retryPolicy, retryable, exception)); } + @Override + public void onRetryPolicyTimeout(RetryPolicy retryPolicy, Retryable retryable, RetryException exception) { + this.listeners.forEach(listener -> listener.onRetryPolicyTimeout(retryPolicy, retryable, exception)); + } + } diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java index 1aeaa303c43..4d6ffb637c8 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java @@ -131,6 +131,22 @@ class RetryPolicyTests { assertToString(policy, 1000, 0, 1.0, Long.MAX_VALUE, 5); } + @Test + void timeoutPreconditions() { + assertThatIllegalArgumentException() + .isThrownBy(() -> RetryPolicy.builder().timeout(Duration.ofMillis(-1))) + .withMessage("Invalid timeout (-1ms): must be greater than or equal to zero."); + } + + @Test + void timeout() { + Duration timeout = Duration.ofMillis(42); + + var policy = RetryPolicy.builder().timeout(timeout).build(); + + assertThat(policy.getTimeout()).isSameAs(timeout); + } + @Test void delayPreconditions() { assertThatIllegalArgumentException() diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java index 0ed720d77e4..dd1190afc04 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java @@ -26,6 +26,7 @@ import java.util.function.Consumer; import org.assertj.core.api.ThrowingConsumer; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments.ArgumentSet; @@ -407,6 +408,185 @@ class RetryTemplateTests { } + @Nested + class TimeoutTests { + + @Test + void retryWithImmediateSuccessAndTimeoutExceeded() throws Exception { + RetryPolicy retryPolicy = RetryPolicy.builder().timeout(Duration.ofMillis(5)).build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + AtomicInteger invocationCount = new AtomicInteger(); + Retryable retryable = () -> { + invocationCount.incrementAndGet(); + Thread.sleep(10); + return "always succeeds"; + }; + + assertThat(invocationCount).hasValue(0); + assertThat(retryTemplate.execute(retryable)).isEqualTo("always succeeds"); + assertThat(invocationCount).hasValue(1); + + // RetryListener interactions: + verifyNoInteractions(retryListener); + } + + @Test + void retryWithInitialFailureAndZeroRetriesRetryPolicyAndTimeoutExceeded() { + RetryPolicy retryPolicy = RetryPolicy.builder() + .timeout(Duration.ofMillis(5)) + .predicate(throwable -> false) // Zero retries + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + Exception exception = new RuntimeException("Boom!"); + Retryable retryable = () -> { + Thread.sleep(10); + throw exception; + }; + + assertThatExceptionOfType(RetryException.class) + .isThrownBy(() -> retryTemplate.execute(retryable)) + .withMessageMatching("Retry policy for operation '.+?' exhausted; aborting execution") + .withCause(exception) + .satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty()) + .satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero()) + .satisfies(throwable -> inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, throwable)); + + verifyNoMoreInteractions(retryListener); + } + + @Test + void retryWithTimeoutExceededAfterInitialFailure() throws Exception { + RetryPolicy retryPolicy = RetryPolicy.builder() + .timeout(Duration.ofMillis(5)) + .delay(Duration.ZERO) + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + AtomicInteger invocationCount = new AtomicInteger(); + Retryable retryable = () -> { + Thread.sleep(10); + throw new CustomException("Boom " + invocationCount.incrementAndGet()); + }; + + assertThat(invocationCount).hasValue(0); + assertThatExceptionOfType(RetryException.class) + .isThrownBy(() -> retryTemplate.execute(retryable)) + .withMessageMatching("Retry policy for operation '.+?' exceeded timeout \\(5 ms\\); aborting execution") + .withCause(new CustomException("Boom 1")) + .satisfies(throwable -> inOrder.verify(retryListener).onRetryPolicyTimeout( + eq(retryPolicy), eq(retryable), eq(throwable))); + assertThat(invocationCount).hasValue(1); + + verifyNoMoreInteractions(retryListener); + } + + @Test + void retryWithTimeoutExceededAfterFirstDelayButBeforeFirstRetry() throws Exception { + RetryPolicy retryPolicy = RetryPolicy.builder() + .timeout(Duration.ofMillis(5)) + .delay(Duration.ofMillis(10)) // Delay > Timeout + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + AtomicInteger invocationCount = new AtomicInteger(); + Retryable retryable = () -> { + throw new CustomException("Boom " + invocationCount.incrementAndGet()); + }; + + assertThat(invocationCount).hasValue(0); + assertThatExceptionOfType(RetryException.class) + .isThrownBy(() -> retryTemplate.execute(retryable)) + .withMessageMatching("Retry policy for operation '.+?' exceeded timeout \\(5 ms\\); aborting execution") + .withCause(new CustomException("Boom 1")) + .satisfies(throwable -> inOrder.verify(retryListener).onRetryPolicyTimeout( + eq(retryPolicy), eq(retryable), eq(throwable))); + assertThat(invocationCount).hasValue(1); + + verifyNoMoreInteractions(retryListener); + } + + @Test + void retryWithTimeoutExceededAfterFirstRetry() throws Exception { + RetryPolicy retryPolicy = RetryPolicy.builder() + .timeout(Duration.ofMillis(5)) + .delay(Duration.ZERO) + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + AtomicInteger invocationCount = new AtomicInteger(); + Retryable retryable = () -> { + int currentInvocation = invocationCount.incrementAndGet(); + if (currentInvocation == 2) { + Thread.sleep(10); + } + throw new CustomException("Boom " + currentInvocation); + }; + + assertThat(invocationCount).hasValue(0); + assertThatExceptionOfType(RetryException.class) + .isThrownBy(() -> retryTemplate.execute(retryable)) + .withMessageMatching("Retry policy for operation '.+?' exceeded timeout \\(5 ms\\); aborting execution") + .withCause(new CustomException("Boom 2")) + .satisfies(throwable -> { + inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable); + inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, new CustomException("Boom 2")); + + inOrder.verify(retryListener).onRetryPolicyTimeout( + eq(retryPolicy), eq(retryable), eq(throwable)); + }); + assertThat(invocationCount).hasValue(2); + + verifyNoMoreInteractions(retryListener); + } + + @Test + void retryWithTimeoutExceededAfterSecondRetry() throws Exception { + RetryPolicy retryPolicy = RetryPolicy.builder() + .timeout(Duration.ofMillis(5)) + .delay(Duration.ZERO) + .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(retryListener); + + AtomicInteger invocationCount = new AtomicInteger(); + Retryable retryable = () -> { + int currentInvocation = invocationCount.incrementAndGet(); + if (currentInvocation == 3) { + Thread.sleep(10); + } + throw new CustomException("Boom " + currentInvocation); + }; + + assertThat(invocationCount).hasValue(0); + assertThatExceptionOfType(RetryException.class) + .isThrownBy(() -> retryTemplate.execute(retryable)) + .withMessageMatching("Retry policy for operation '.+?' exceeded timeout \\(5 ms\\); aborting execution") + .withCause(new CustomException("Boom 3")) + .satisfies(throwable -> { + var counter = new AtomicInteger(1); + repeat(2, () -> { + inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable); + inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, + new CustomException("Boom " + counter.incrementAndGet())); + }); + inOrder.verify(retryListener).onRetryPolicyTimeout( + eq(retryPolicy), eq(retryable), eq(throwable)); + }); + assertThat(invocationCount).hasValue(3); + + verifyNoMoreInteractions(retryListener); + } + + } + + private static void repeat(int times, Runnable runnable) { for (int i = 0; i < times; i++) { runnable.run(); diff --git a/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java b/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java index 3ceb0e9bb60..e7fa3741a8a 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/support/CompositeRetryListenerTests.java @@ -16,6 +16,7 @@ package org.springframework.core.retry.support; +import java.time.Duration; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -102,4 +103,15 @@ class CompositeRetryListenerTests { verify(listener3).onRetryPolicyInterruption(retryPolicy, retryable, exception); } + @Test + void onRetryPolicyTimeout() { + Duration elapsedTime = Duration.ofMillis(100); + RetryException exception = new RetryException("", new Exception()); + compositeRetryListener.onRetryPolicyTimeout(retryPolicy, retryable, exception); + + verify(listener1).onRetryPolicyTimeout(retryPolicy, retryable, exception); + verify(listener2).onRetryPolicyTimeout(retryPolicy, retryable, exception); + verify(listener3).onRetryPolicyTimeout(retryPolicy, retryable, exception); + } + }