From 5a611adc4ecc00ae8d69b052ddebfcc23a427595 Mon Sep 17 00:00:00 2001 From: Dongie Agnir Date: Tue, 28 Apr 2026 10:06:11 -0700 Subject: [PATCH] Retries 2.1 std suggested delay computation For standard 2.1, there is additional treatment of the service suggested backoff so it's at least as long as the strategy computed value, and not longer than 5s more than the strategy computed value. --- .../retries/internal/BaseRetryStrategy.java | 7 ++ .../DefaultStandardRetryStrategy.java | 34 ++++++ .../internal/StandardRetryStrategyTest.java | 102 ++++++++++++++---- 3 files changed, 125 insertions(+), 18 deletions(-) diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java index 15656ab96e5..dfda06093ab 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java @@ -380,6 +380,13 @@ static Duration maxOf(Duration left, Duration right) { return right; } + static Duration minOf(Duration left, Duration right) { + if (left.compareTo(right) <= 0) { + return left; + } + return right; + } + static DefaultRetryToken asDefaultRetryToken(RetryToken token) { return Validate.isInstanceOf(DefaultRetryToken.class, token, "RetryToken is of unexpected class (%s), " diff --git a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java index 878cdcd380c..92b24421b57 100644 --- a/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java +++ b/core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.retries.internal; import java.time.Duration; +import java.util.Optional; import java.util.function.Predicate; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.retries.StandardRetryStrategy; @@ -28,6 +29,7 @@ public final class DefaultStandardRetryStrategy extends BaseRetryStrategy implements StandardRetryStrategy { private static final Logger LOG = Logger.loggerFor(DefaultStandardRetryStrategy.class); + private static final Duration FIVE_SECONDS = Duration.ofSeconds(5); private final boolean retries2026Enabled; DefaultStandardRetryStrategy(Builder builder) { @@ -56,6 +58,38 @@ protected Duration computeAcquireFailureBackoff(RefreshRetryTokenRequest request return computeBackoff(request, attemptIncremented); } + @Override + protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) { + if (!retries2026Enabled) { + return super.computeBackoff(request, token); + } + + Duration strategyBackoff; + if (treatAsThrottling.test(request.failure())) { + strategyBackoff = throttlingBackoffStrategy.computeDelay(token.attempt()); + } else { + strategyBackoff = backoffStrategy.computeDelay(token.attempt()); + } + + Optional optionalSuggested = request.suggestedDelay(); + + if (!optionalSuggested.isPresent()) { + return strategyBackoff; + } + + // the suggested delay needs to be at least what the strategy computed, OR + // not greater than 5s more than what the strat computed + Duration minBackoff = strategyBackoff; + Duration maxBackoff = strategyBackoff.plus(FIVE_SECONDS); + + Duration backoff = optionalSuggested.get(); + + backoff = maxOf(minBackoff, backoff); + backoff = minOf(maxBackoff, backoff); + + return backoff; + } + public static class Builder extends BaseRetryStrategy.Builder implements StandardRetryStrategy.Builder { private boolean retries2026Enabled; diff --git a/core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java b/core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java index c182a0ed637..59497420701 100644 --- a/core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java +++ b/core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java @@ -104,12 +104,17 @@ void verifyScenario(Scenario scenario) { case RETRY_REQUEST: { ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode, response.throttling); - RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder() - .failure(scenarioTestException) - .isLongPolling(given.isLongPolling) - .token(token.get()) - .build(); - RefreshRetryTokenResponse refreshResponse = strategy.refreshRetryToken(refreshRequest); + RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder(); + + if (response.xAmzRetryAfter != null) { + refreshRequest.suggestedDelay(response.xAmzRetryAfter); + } + + refreshRequest.failure(scenarioTestException) + .isLongPolling(given.isLongPolling) + .token(token.get()) + .build(); + RefreshRetryTokenResponse refreshResponse = strategy.refreshRetryToken(refreshRequest.build()); DefaultRetryToken refreshedToken = (DefaultRetryToken) refreshResponse.token(); token.set(refreshedToken); @@ -120,14 +125,18 @@ void verifyScenario(Scenario scenario) { case RETRY_QUOTA_EXCEEDED: { ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode, response.throttling); - RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder() - .failure(scenarioTestException) - .isLongPolling(given.isLongPolling) - .token(token.get()) - .build(); + RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder(); + + if (response.xAmzRetryAfter != null) { + refreshRequest.suggestedDelay(response.xAmzRetryAfter); + } + refreshRequest.failure(scenarioTestException) + .isLongPolling(given.isLongPolling) + .token(token.get()) + .build(); - assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest)) + assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest.build())) .isInstanceOf(TokenAcquisitionFailedException.class) .matches(e -> { TokenAcquisitionFailedException acquireException = (TokenAcquisitionFailedException) e; @@ -149,12 +158,18 @@ void verifyScenario(Scenario scenario) { case MAX_ATTEMPTS_EXCEEDED: { ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode, response.throttling); - RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder() - .failure(scenarioTestException) - .isLongPolling(given.isLongPolling) - .token(token.get()) - .build(); - assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest)) + RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder(); + + if (response.xAmzRetryAfter != null) { + refreshRequest.suggestedDelay(response.xAmzRetryAfter); + } + + refreshRequest.failure(scenarioTestException) + .isLongPolling(given.isLongPolling) + .token(token.get()) + .build(); + + assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest.build())) .isInstanceOf(TokenAcquisitionFailedException.class) .matches(e -> { TokenAcquisitionFailedException acquireException = (TokenAcquisitionFailedException) e; @@ -673,7 +688,52 @@ private static Stream retriesV21Tests() { .expected(e -> e.outcome(Outcome.MAX_ATTEMPTS_EXCEEDED) .delay(Duration.ZERO) + .retryQuota(486))), + + aScenario("Honor x-amz-retry-after Header") + .newRetries2026(true) + .addResponse(r -> + r.statusCode(500) + .xAmzRetryAfter(Duration.ofMillis(1500)) + .expected(e -> + e.outcome(Outcome.RETRY_REQUEST) + .delay(Duration.ofMillis(1500)) .retryQuota(486))) + .addResponse(r -> + r.statusCode(200) + .expected(e -> + e.outcome(Outcome.SUCCESS) + .retryQuota(500))), + + aScenario("x-amz-retry-after minimum is exponential backoff duration") + .newRetries2026(true) + .addResponse(r -> + r.statusCode(500) + .xAmzRetryAfter(Duration.ofMillis(0)) + .expected(e -> + e.outcome(Outcome.RETRY_REQUEST) + .delay(Duration.ofMillis(50)) + .retryQuota(486))) + .addResponse(r -> + r.statusCode(200) + .expected(e -> + e.outcome(Outcome.SUCCESS) + .retryQuota(500))), + + aScenario("x-amz-retry-after maximum is 5+exponential backoff duration") + .newRetries2026(true) + .addResponse(r -> + r.statusCode(500) + .xAmzRetryAfter(Duration.ofMillis(10000)) + .expected(e -> + e.outcome(Outcome.RETRY_REQUEST) + .delay(Duration.ofMillis(5050)) + .retryQuota(486))) + .addResponse(r -> + r.statusCode(200) + .expected(e -> + e.outcome(Outcome.SUCCESS) + .retryQuota(500))) ); } @@ -721,6 +781,7 @@ public Given maxBackoff(Duration maxBackoff) { private static class Response { private int statusCode; private boolean throttling; + private Duration xAmzRetryAfter; private Expected expected; public Response statusCode(int statusCode) { @@ -733,6 +794,11 @@ public Response isThrottling(boolean throttling) { return this; } + public Response xAmzRetryAfter(Duration xAmzRetryAfter) { + this.xAmzRetryAfter = xAmzRetryAfter; + return this; + } + public Response expected(Consumer acceptor) { this.expected = new Expected(); acceptor.accept(this.expected);