Skip to content

Commit 5a611ad

Browse files
dagnirdagnir
committed
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.
1 parent 4de251e commit 5a611ad

3 files changed

Lines changed: 125 additions & 18 deletions

File tree

‎core/retries/src/main/java/software/amazon/awssdk/retries/internal/BaseRetryStrategy.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,13 @@ static Duration maxOf(Duration left, Duration right) {
380380
return right;
381381
}
382382

383+
static Duration minOf(Duration left, Duration right) {
384+
if (left.compareTo(right) <= 0) {
385+
return left;
386+
}
387+
return right;
388+
}
389+
383390
static DefaultRetryToken asDefaultRetryToken(RetryToken token) {
384391
return Validate.isInstanceOf(DefaultRetryToken.class, token,
385392
"RetryToken is of unexpected class (%s), "

‎core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultStandardRetryStrategy.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package software.amazon.awssdk.retries.internal;
1717

1818
import java.time.Duration;
19+
import java.util.Optional;
1920
import java.util.function.Predicate;
2021
import software.amazon.awssdk.annotations.SdkInternalApi;
2122
import software.amazon.awssdk.retries.StandardRetryStrategy;
@@ -28,6 +29,7 @@
2829
public final class DefaultStandardRetryStrategy
2930
extends BaseRetryStrategy implements StandardRetryStrategy {
3031
private static final Logger LOG = Logger.loggerFor(DefaultStandardRetryStrategy.class);
32+
private static final Duration FIVE_SECONDS = Duration.ofSeconds(5);
3133
private final boolean retries2026Enabled;
3234

3335
DefaultStandardRetryStrategy(Builder builder) {
@@ -56,6 +58,38 @@ protected Duration computeAcquireFailureBackoff(RefreshRetryTokenRequest request
5658
return computeBackoff(request, attemptIncremented);
5759
}
5860

61+
@Override
62+
protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) {
63+
if (!retries2026Enabled) {
64+
return super.computeBackoff(request, token);
65+
}
66+
67+
Duration strategyBackoff;
68+
if (treatAsThrottling.test(request.failure())) {
69+
strategyBackoff = throttlingBackoffStrategy.computeDelay(token.attempt());
70+
} else {
71+
strategyBackoff = backoffStrategy.computeDelay(token.attempt());
72+
}
73+
74+
Optional<Duration> optionalSuggested = request.suggestedDelay();
75+
76+
if (!optionalSuggested.isPresent()) {
77+
return strategyBackoff;
78+
}
79+
80+
// the suggested delay needs to be at least what the strategy computed, OR
81+
// not greater than 5s more than what the strat computed
82+
Duration minBackoff = strategyBackoff;
83+
Duration maxBackoff = strategyBackoff.plus(FIVE_SECONDS);
84+
85+
Duration backoff = optionalSuggested.get();
86+
87+
backoff = maxOf(minBackoff, backoff);
88+
backoff = minOf(maxBackoff, backoff);
89+
90+
return backoff;
91+
}
92+
5993
public static class Builder extends BaseRetryStrategy.Builder implements StandardRetryStrategy.Builder {
6094
private boolean retries2026Enabled;
6195

‎core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java‎

Lines changed: 84 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,17 @@ void verifyScenario(Scenario scenario) {
104104
case RETRY_REQUEST: {
105105
ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode,
106106
response.throttling);
107-
RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder()
108-
.failure(scenarioTestException)
109-
.isLongPolling(given.isLongPolling)
110-
.token(token.get())
111-
.build();
112-
RefreshRetryTokenResponse refreshResponse = strategy.refreshRetryToken(refreshRequest);
107+
RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder();
108+
109+
if (response.xAmzRetryAfter != null) {
110+
refreshRequest.suggestedDelay(response.xAmzRetryAfter);
111+
}
112+
113+
refreshRequest.failure(scenarioTestException)
114+
.isLongPolling(given.isLongPolling)
115+
.token(token.get())
116+
.build();
117+
RefreshRetryTokenResponse refreshResponse = strategy.refreshRetryToken(refreshRequest.build());
113118
DefaultRetryToken refreshedToken = (DefaultRetryToken) refreshResponse.token();
114119
token.set(refreshedToken);
115120

@@ -120,14 +125,18 @@ void verifyScenario(Scenario scenario) {
120125
case RETRY_QUOTA_EXCEEDED: {
121126
ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode,
122127
response.throttling);
123-
RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder()
124-
.failure(scenarioTestException)
125-
.isLongPolling(given.isLongPolling)
126-
.token(token.get())
127-
.build();
128+
RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder();
129+
130+
if (response.xAmzRetryAfter != null) {
131+
refreshRequest.suggestedDelay(response.xAmzRetryAfter);
132+
}
128133

134+
refreshRequest.failure(scenarioTestException)
135+
.isLongPolling(given.isLongPolling)
136+
.token(token.get())
137+
.build();
129138

130-
assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest))
139+
assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest.build()))
131140
.isInstanceOf(TokenAcquisitionFailedException.class)
132141
.matches(e -> {
133142
TokenAcquisitionFailedException acquireException = (TokenAcquisitionFailedException) e;
@@ -149,12 +158,18 @@ void verifyScenario(Scenario scenario) {
149158
case MAX_ATTEMPTS_EXCEEDED: {
150159
ScenarioTestException scenarioTestException = new ScenarioTestException(response.statusCode,
151160
response.throttling);
152-
RefreshRetryTokenRequest refreshRequest = RefreshRetryTokenRequest.builder()
153-
.failure(scenarioTestException)
154-
.isLongPolling(given.isLongPolling)
155-
.token(token.get())
156-
.build();
157-
assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest))
161+
RefreshRetryTokenRequest.Builder refreshRequest = RefreshRetryTokenRequest.builder();
162+
163+
if (response.xAmzRetryAfter != null) {
164+
refreshRequest.suggestedDelay(response.xAmzRetryAfter);
165+
}
166+
167+
refreshRequest.failure(scenarioTestException)
168+
.isLongPolling(given.isLongPolling)
169+
.token(token.get())
170+
.build();
171+
172+
assertThatThrownBy(() -> strategy.refreshRetryToken(refreshRequest.build()))
158173
.isInstanceOf(TokenAcquisitionFailedException.class)
159174
.matches(e -> {
160175
TokenAcquisitionFailedException acquireException = (TokenAcquisitionFailedException) e;
@@ -673,7 +688,52 @@ private static Stream<Scenario> retriesV21Tests() {
673688
.expected(e ->
674689
e.outcome(Outcome.MAX_ATTEMPTS_EXCEEDED)
675690
.delay(Duration.ZERO)
691+
.retryQuota(486))),
692+
693+
aScenario("Honor x-amz-retry-after Header")
694+
.newRetries2026(true)
695+
.addResponse(r ->
696+
r.statusCode(500)
697+
.xAmzRetryAfter(Duration.ofMillis(1500))
698+
.expected(e ->
699+
e.outcome(Outcome.RETRY_REQUEST)
700+
.delay(Duration.ofMillis(1500))
676701
.retryQuota(486)))
702+
.addResponse(r ->
703+
r.statusCode(200)
704+
.expected(e ->
705+
e.outcome(Outcome.SUCCESS)
706+
.retryQuota(500))),
707+
708+
aScenario("x-amz-retry-after minimum is exponential backoff duration")
709+
.newRetries2026(true)
710+
.addResponse(r ->
711+
r.statusCode(500)
712+
.xAmzRetryAfter(Duration.ofMillis(0))
713+
.expected(e ->
714+
e.outcome(Outcome.RETRY_REQUEST)
715+
.delay(Duration.ofMillis(50))
716+
.retryQuota(486)))
717+
.addResponse(r ->
718+
r.statusCode(200)
719+
.expected(e ->
720+
e.outcome(Outcome.SUCCESS)
721+
.retryQuota(500))),
722+
723+
aScenario("x-amz-retry-after maximum is 5+exponential backoff duration")
724+
.newRetries2026(true)
725+
.addResponse(r ->
726+
r.statusCode(500)
727+
.xAmzRetryAfter(Duration.ofMillis(10000))
728+
.expected(e ->
729+
e.outcome(Outcome.RETRY_REQUEST)
730+
.delay(Duration.ofMillis(5050))
731+
.retryQuota(486)))
732+
.addResponse(r ->
733+
r.statusCode(200)
734+
.expected(e ->
735+
e.outcome(Outcome.SUCCESS)
736+
.retryQuota(500)))
677737
);
678738
}
679739

@@ -721,6 +781,7 @@ public Given maxBackoff(Duration maxBackoff) {
721781
private static class Response {
722782
private int statusCode;
723783
private boolean throttling;
784+
private Duration xAmzRetryAfter;
724785
private Expected expected;
725786

726787
public Response statusCode(int statusCode) {
@@ -733,6 +794,11 @@ public Response isThrottling(boolean throttling) {
733794
return this;
734795
}
735796

797+
public Response xAmzRetryAfter(Duration xAmzRetryAfter) {
798+
this.xAmzRetryAfter = xAmzRetryAfter;
799+
return this;
800+
}
801+
736802
public Response expected(Consumer<Expected> acceptor) {
737803
this.expected = new Expected();
738804
acceptor.accept(this.expected);

0 commit comments

Comments
 (0)