Remove maxDuration/maxElapsedTime support from RetryPolicy

When the core retry functionality was introduced, it had a built-in
MaxRetryDurationPolicy. In #35058, that was migrated to a
withMaxDuration() factory method, and in #35110 that was renamed to
withMaxElapsedTime() (with a corresponding maxElapsedTime() method on
the builder) in order to align with the maxElapsedTime feature of
ExponentialBackOff. The latter also changed the semantics of the
feature in the context of the RetryPolicy.

However, @⁠Retryable does not provide maxElapsedTime support.

In addition, the maxElapsedTime feature is a bit misleading, since it
does not actually track CPU time or wall-clock time but rather only the
sum of individual, accumulated back-off intervals/delays, which is
likely not very useful. Furthermore, the maxElapsedTime will never
apply to a zero-valued delay/interval.

In light of the above, this commit removes the maxElapsedTime support
from the built-in RetryPolicy.

Users can still implement a custom BackOff strategy if they find they
need some form of "max elapsed time" or "max duration".

See gh-34716
See gh-35058
See gh-34529
See gh-35110
Closes gh-35144

Author: sbrannen

spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java

@@ -35,8 +35,8 @@
  *
  * <p>Also provides factory methods and a fluent builder API for creating retry
  * policies with common configurations. See {@link #withDefaults()},
- * {@link #withMaxAttempts(long)}, {@link #withMaxElapsedTime(Duration)},
- * {@link #builder()}, and the configuration options in {@link Builder} for details.
+ * {@link #withMaxAttempts(long)}, {@link #builder()}, and the configuration
+ * options in {@link Builder} for details.
  *
  * @author Sam Brannen
  * @author Mahmoud Ben Hassine
@@ -91,18 +91,6 @@ static RetryPolicy withMaxAttempts(long maxAttempts) {
 		return builder().backOff(new FixedBackOff(Builder.DEFAULT_DELAY, maxAttempts)).build();
 	}
 
-	/**
-	 * Create a {@link RetryPolicy} configured with a maximum elapsed time.
-	 * <p>The returned policy uses a fixed backoff of {@value Builder#DEFAULT_DELAY}
-	 * milliseconds.
-	 * @param maxElapsedTime the maximum elapsed time; must be positive
-	 * @see Builder#maxElapsedTime(Duration)
-	 * @see FixedBackOff
-	 */
-	static RetryPolicy withMaxElapsedTime(Duration maxElapsedTime) {
-		return builder().maxElapsedTime(maxElapsedTime).build();
-	}
-
 	/**
 	 * Create a {@link Builder} to configure a {@link RetryPolicy} with common
 	 * configuration options.
@@ -152,8 +140,6 @@ final class Builder {
 
 		private @Nullable Duration maxDelay;
 
-		private @Nullable Duration maxElapsedTime;
-
 		private final Set<Class<? extends Throwable>> includes = new LinkedHashSet<>();
 
 		private final Set<Class<? extends Throwable>> excludes = new LinkedHashSet<>();
@@ -173,8 +159,7 @@ private Builder() {
 		 * strategy, you should not configure any of the following:
 		 * {@link #maxAttempts(long) maxAttempts}, {@link #delay(Duration) delay},
 		 * {@link #jitter(Duration) jitter}, {@link #multiplier(double) multiplier},
-		 * {@link #maxDelay(Duration) maxDelay}, or {@link #maxElapsedTime(Duration)
-		 * maxElapsedTime}.
+		 * or {@link #maxDelay(Duration) maxDelay}.
 		 * @param backOff the {@code BackOff} strategy
 		 * @return this {@code Builder} instance for chained method invocations
 		 */
@@ -291,21 +276,6 @@ public Builder maxDelay(Duration maxDelay) {
 			return this;
 		}
 
-		/**
-		 * Specify the maximum elapsed time.
-		 * <p>The default is unlimited.
-		 * <p>The supplied value will override any previously configured value.
-		 * <p>You should not specify this configuration option if you have
-		 * configured a custom {@link #backOff(BackOff) BackOff} strategy.
-		 * @param maxElapsedTime the maximum elapsed time; must be positive
-		 * @return this {@code Builder} instance for chained method invocations
-		 */
-		public Builder maxElapsedTime(Duration maxElapsedTime) {
-			assertIsPositive("maxElapsedTime", maxElapsedTime);
-			this.maxElapsedTime = maxElapsedTime;
-			return this;
-		}
-
 		/**
 		 * Specify the types of exceptions for which the {@link RetryPolicy}
 		 * should retry a failed operation.
@@ -415,10 +385,10 @@ public RetryPolicy build() {
 			BackOff backOff = this.backOff;
 			if (backOff != null) {
 				boolean misconfigured = (this.maxAttempts != 0) || (this.delay != null) || (this.jitter != null) ||
-						(this.multiplier != 0) || (this.maxDelay != null) || (this.maxElapsedTime != null);
+						(this.multiplier != 0) || (this.maxDelay != null);
 				Assert.state(!misconfigured, """
 						The following configuration options are not supported with a custom BackOff strategy: \
-						maxAttempts, delay, jitter, multiplier, maxDelay, or maxElapsedTime.""");
+						maxAttempts, delay, jitter, multiplier, or maxDelay.""");
 			}
 			else {
 				ExponentialBackOff exponentialBackOff = new ExponentialBackOff();
@@ -429,9 +399,6 @@ public RetryPolicy build() {
 				if (this.jitter != null) {
 					exponentialBackOff.setJitter(this.jitter.toMillis());
 				}
-				if (this.maxElapsedTime != null) {
-					exponentialBackOff.setMaxElapsedTime(this.maxElapsedTime.toMillis());
-				}
 				backOff = exponentialBackOff;
 			}
 			return new DefaultRetryPolicy(this.includes, this.excludes, this.predicate, backOff);

spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java

@@ -87,29 +87,6 @@ void withMaxAttempts() {
 						assertThat(backOff.getInterval()).isEqualTo(1000);
 					});
 		}
-
-		@Test
-		void withMaxElapsedTimePreconditions() {
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(0)))
-					.withMessage("Invalid duration (0ms): maxElapsedTime must be positive.");
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.withMaxElapsedTime(Duration.ofMillis(-1)))
-					.withMessage("Invalid duration (-1ms): maxElapsedTime must be positive.");
-		}
-
-		@Test
-		void withMaxElapsedTime() {
-			var policy = RetryPolicy.withMaxElapsedTime(Duration.ofMillis(42));
-
-			assertThat(policy.shouldRetry(new AssertionError())).isTrue();
-			assertThat(policy.shouldRetry(new IOException())).isTrue();
-
-			assertThat(policy.getBackOff())
-					.asInstanceOf(type(ExponentialBackOff.class))
-					.satisfies(hasDefaultMaxAttemptsAndDelay())
-					.extracting(ExponentialBackOff::getMaxElapsedTime).isEqualTo(42L);
-		}
 	}
 
 
@@ -122,7 +99,7 @@ void backOffPlusConflictingConfig() {
 					.isThrownBy(() -> RetryPolicy.builder().backOff(mock()).delay(Duration.ofMillis(10)).build())
 					.withMessage("""
 							The following configuration options are not supported with a custom BackOff strategy: \
-							maxAttempts, delay, jitter, multiplier, maxDelay, or maxElapsedTime.""");
+							maxAttempts, delay, jitter, multiplier, or maxDelay.""");
 		}
 
 		@Test
@@ -157,7 +134,7 @@ void maxAttempts() {
 						assertThat(backOff.getInitialInterval()).isEqualTo(1000);
 					});
 
-			assertToString(policy, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 5);
+			assertToString(policy, 1000, 0, 1.0, Long.MAX_VALUE, 5);
 		}
 
 		@Test
@@ -178,7 +155,7 @@ void delay() {
 						assertThat(backOff.getMaxAttempts()).isEqualTo(3);
 					});
 
-			assertToString(policy, 42, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, 42, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -197,7 +174,7 @@ void jitter() {
 					.satisfies(hasDefaultMaxAttemptsAndDelay())
 					.extracting(ExponentialBackOff::getJitter).isEqualTo(42L);
 
-			assertToString(policy, 1000, 42, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, 1000, 42, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -226,7 +203,7 @@ void multiplier() {
 					.satisfies(hasDefaultMaxAttemptsAndDelay())
 					.extracting(ExponentialBackOff::getMultiplier).isEqualTo(1.5);
 
-			assertToString(policy, 1000, 0, 1.5, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, 1000, 0, 1.5, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -248,29 +225,7 @@ void maxDelay() {
 					.satisfies(hasDefaultMaxAttemptsAndDelay())
 					.extracting(ExponentialBackOff::getMaxInterval).isEqualTo(42L);
 
-			assertToString(policy, 1000, 0, 1, 42, Long.MAX_VALUE, 3);
-		}
-
-		@Test
-		void maxElapsedTimePreconditions() {
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(0)))
-					.withMessage("Invalid duration (0ms): maxElapsedTime must be positive.");
-			assertThatIllegalArgumentException()
-					.isThrownBy(() -> RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(-1)))
-					.withMessage("Invalid duration (-1ms): maxElapsedTime must be positive.");
-		}
-
-		@Test
-		void maxElapsedTime() {
-			var policy = RetryPolicy.builder().maxElapsedTime(Duration.ofMillis(42)).build();
-
-			assertThat(policy.getBackOff())
-					.asInstanceOf(type(ExponentialBackOff.class))
-					.satisfies(hasDefaultMaxAttemptsAndDelay())
-					.extracting(ExponentialBackOff::getMaxElapsedTime).isEqualTo(42L);
-
-			assertToString(policy, 1000, 0, 1, Long.MAX_VALUE, 42, 3);
+			assertToString(policy, 1000, 0, 1.0, 42, 3);
 		}
 
 		@Test
@@ -294,7 +249,7 @@ void includes() {
 
 			String filters = "includes=" + names(FileNotFoundException.class, IllegalArgumentException.class,
 					NumberFormatException.class, AssertionError.class) + ", ";
-			assertToString(policy, filters, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, filters, 1000, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -311,7 +266,7 @@ void includesSubtypeMatching() {
 					.asInstanceOf(type(ExponentialBackOff.class))
 					.satisfies(hasDefaultMaxAttemptsAndDelay());
 
-			assertToString(policy, "includes=[java.io.IOException], ", 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, "includes=[java.io.IOException], ", 1000, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -335,7 +290,7 @@ void excludes() {
 
 			String filters = "excludes=" + names(FileNotFoundException.class, IllegalArgumentException.class,
 					NumberFormatException.class, AssertionError.class) + ", ";
-			assertToString(policy, filters, 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, filters, 1000, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -349,7 +304,7 @@ void excludesSubtypeMatching() {
 			assertThat(policy.shouldRetry(new Throwable())).isTrue();
 			assertThat(policy.shouldRetry(new AssertionError())).isTrue();
 
-			assertToString(policy, "excludes=[java.io.IOException], ", 1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, "excludes=[java.io.IOException], ", 1000, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -368,8 +323,7 @@ void predicate() {
 					.asInstanceOf(type(ExponentialBackOff.class))
 					.satisfies(hasDefaultMaxAttemptsAndDelay());
 
-			assertToString(policy, "predicate=NumberFormatExceptionMatcher, ",
-					1000, 0, 1, Long.MAX_VALUE, Long.MAX_VALUE, 3);
+			assertToString(policy, "predicate=NumberFormatExceptionMatcher, ", 1000, 0, 1.0, Long.MAX_VALUE, 3);
 		}
 
 		@Test
@@ -398,13 +352,13 @@ void predicatesCombined() {
 
 
 		private static void assertToString(RetryPolicy policy, long initialInterval, long jitter,
-				double multiplier, long maxInterval, long maxElapsedTime, int maxAttempts) {
+				double multiplier, long maxInterval, int maxAttempts) {
 
-			assertToString(policy, "", initialInterval, jitter, multiplier, maxInterval, maxElapsedTime, maxAttempts);
+			assertToString(policy, "", initialInterval, jitter, multiplier, maxInterval, maxAttempts);
 		}
 
 		private static void assertToString(RetryPolicy policy, String filters, long initialInterval, long jitter,
-				double multiplier, long maxInterval, long maxElapsedTime, int maxAttempts) {
+				double multiplier, long maxInterval, int maxAttempts) {
 
 			assertThat(policy).asString()
 				.isEqualTo("""
@@ -416,7 +370,7 @@ private static void assertToString(RetryPolicy policy, String filters, long init
 						maxElapsedTime=%d, \
 						maxAttempts=%d\
 						]]""",
-						filters, initialInterval, jitter, multiplier, maxInterval, maxElapsedTime, maxAttempts);
+						filters, initialInterval, jitter, multiplier, maxInterval, Long.MAX_VALUE, maxAttempts);
 		}
 
 		@SafeVarargs