spring/spring-framework
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-framework.git synced 2026-09-25 08:19:01 +00:00
Code Issues Projects Releases Wiki Activity

Remove maxDuration/maxElapsedTime support from RetryPolicy

Browse Source 
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
This commit is contained in:
Sam Brannen
2025-07-02 10:18:36 +02:00
parent b256babad5
commit 5cae769c2d
2 changed files with 20 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spring-core/src/test/java/org/springframework/core/retry/RetryPolicyTests.java
+15 -61
View File
@@ -87,29 +87,6 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
.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 @@ class RetryPolicyTests {
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 @@ class RetryPolicyTests {
maxElapsedTime=%d, \
maxAttempts=%d\
]]""",
filters, initialInterval, jitter, multiplier, maxInterval, maxElapsedTime, maxAttempts);
filters, initialInterval, jitter, multiplier, maxInterval, Long.MAX_VALUE, maxAttempts);
}
@SafeVarargs