Skip to content

Commit 89c6d6f

Browse files
Bennett-LynchBennett-Lynch
authored
Use "full jitter" & updated base delay for STANDARD retry mode defaults (#2648)
* Use "full jitter" & updated base delay for STANDARD retry mode defaults ## Motivation and Context The STANDARD retry mode has standardized behavior across the SDKs. This includes using "full jitter" backoff (i.e., 0-100% of the computed delay) for both normal and throttling errors. The standard behavior also uses a base delay of 1 second. ## Modifications * Update BackoffStrategy's `defaultStrategy` and `defaultThrottlingStrategy` methods to accept a `RetryMode`. The `RetryMode` will be used to resolve the default jitter strategy ("full" vs. "equal") as well as the base delay (the max backoff time does not vary between modes). * Define additional constants in SdkDefaultRetrySetting to support the update base delay values. (Note: This includes backwards-incompatible changes but the class is annotated @SdkInternalApi.) * Update RetryPolicy to use the new methods. * Add tests to RetryPolicyTest to ensure that it is resolving to the correct strategy and values. * Add new unit test class for FullJitterBackoffStrategy. This confirms that full jitter is consistent with the standard retry behavior. * Update FullJitterBackoffStrategy to add 1 ms to the random result. This both allows us to reach our maximum value (since the random parameter is exclusive) and ensures we always have a delay of at least 1 ms (which is consistent with BackoffStrategy.none()'s behavior).
1 parent 8eebc72 commit 89c6d6f

File tree

7 files changed

+273
-18
lines changed

7 files changed

+273
-18
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-84afdff.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Use \"full jitter\" & updated base delay for STANDARD retry mode defaults"
6+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/retry/SdkDefaultRetrySetting.java

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
@SdkInternalApi
3434
public final class SdkDefaultRetrySetting {
3535
public static final class Legacy {
36+
private static final int MAX_ATTEMPTS = 4;
37+
private static final Duration BASE_DELAY = Duration.ofMillis(100);
38+
private static final Duration THROTTLED_BASE_DELAY = Duration.ofMillis(500);
3639
private static final int THROTTLE_EXCEPTION_TOKEN_COST = 0;
3740
private static final int DEFAULT_EXCEPTION_TOKEN_COST = 5;
3841

@@ -44,6 +47,9 @@ public static final class Legacy {
4447
}
4548

4649
public static final class Standard {
50+
private static final int MAX_ATTEMPTS = 3;
51+
private static final Duration BASE_DELAY = Duration.ofSeconds(1);
52+
private static final Duration THROTTLED_BASE_DELAY = Duration.ofSeconds(1);
4753
private static final int THROTTLE_EXCEPTION_TOKEN_COST = 5;
4854
private static final int DEFAULT_EXCEPTION_TOKEN_COST = 5;
4955

@@ -56,13 +62,7 @@ public static final class Standard {
5662

5763
public static final int TOKEN_BUCKET_SIZE = 500;
5864

59-
public static final Duration BASE_DELAY = Duration.ofMillis(100);
60-
61-
public static final Duration THROTTLED_BASE_DELAY = Duration.ofMillis(500);
62-
63-
public static final Duration MAX_BACKOFF = Duration.ofMillis(20_000);
64-
65-
public static final Integer DEFAULT_MAX_RETRIES = 3;
65+
public static final Duration MAX_BACKOFF = Duration.ofSeconds(20);
6666

6767
public static final Set<Integer> RETRYABLE_STATUS_CODES;
6868
public static final Set<Class<? extends Exception>> RETRYABLE_EXCEPTIONS;
@@ -91,13 +91,13 @@ public static Integer maxAttempts(RetryMode retryMode) {
9191
if (maxAttempts == null) {
9292
switch (retryMode) {
9393
case LEGACY:
94-
maxAttempts = 4;
94+
maxAttempts = Legacy.MAX_ATTEMPTS;
9595
break;
9696
case STANDARD:
97-
maxAttempts = 3;
97+
maxAttempts = Standard.MAX_ATTEMPTS;
9898
break;
9999
default:
100-
throw new IllegalArgumentException("Unknown retry mode: " + retryMode);
100+
throw new IllegalStateException("Unsupported RetryMode: " + retryMode);
101101
}
102102
}
103103

@@ -117,4 +117,26 @@ public static TokenBucketExceptionCostFunction tokenCostFunction(RetryMode retry
117117
public static Integer defaultMaxAttempts() {
118118
return maxAttempts(RetryMode.defaultRetryMode());
119119
}
120+
121+
public static Duration baseDelay(RetryMode retryMode) {
122+
switch (retryMode) {
123+
case LEGACY:
124+
return Legacy.BASE_DELAY;
125+
case STANDARD:
126+
return Standard.BASE_DELAY;
127+
default:
128+
throw new IllegalStateException("Unsupported RetryMode: " + retryMode);
129+
}
130+
}
131+
132+
public static Duration throttledBaseDelay(RetryMode retryMode) {
133+
switch (retryMode) {
134+
case LEGACY:
135+
return Legacy.THROTTLED_BASE_DELAY;
136+
case STANDARD:
137+
return Standard.THROTTLED_BASE_DELAY;
138+
default:
139+
throw new IllegalStateException("Unsupported RetryMode: " + retryMode);
140+
}
141+
}
120142
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/RetryPolicy.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,8 @@ private BuilderImpl(RetryMode retryMode) {
329329
this.retryMode = retryMode;
330330
this.numRetries = SdkDefaultRetrySetting.maxAttempts(retryMode) - 1;
331331
this.additionalRetryConditionsAllowed = true;
332-
this.backoffStrategy = BackoffStrategy.defaultStrategy();
333-
this.throttlingBackoffStrategy = BackoffStrategy.defaultThrottlingStrategy();
332+
this.backoffStrategy = BackoffStrategy.defaultStrategy(retryMode);
333+
this.throttlingBackoffStrategy = BackoffStrategy.defaultThrottlingStrategy(retryMode);
334334
this.retryCondition = RetryCondition.defaultRetryCondition();
335335
this.retryCapacityCondition = TokenBucketRetryCondition.forRetryMode(retryMode);
336336
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.time.Duration;
1919
import software.amazon.awssdk.annotations.SdkPublicApi;
2020
import software.amazon.awssdk.core.internal.retry.SdkDefaultRetrySetting;
21+
import software.amazon.awssdk.core.retry.RetryMode;
2122
import software.amazon.awssdk.core.retry.RetryPolicyContext;
2223

2324
@SdkPublicApi
@@ -44,17 +45,35 @@ default int calculateExponentialDelay(int retriesAttempted, Duration baseDelay,
4445
}
4546

4647
static BackoffStrategy defaultStrategy() {
48+
return defaultStrategy(RetryMode.defaultRetryMode());
49+
}
50+
51+
static BackoffStrategy defaultStrategy(RetryMode retryMode) {
4752
return FullJitterBackoffStrategy.builder()
48-
.baseDelay(SdkDefaultRetrySetting.BASE_DELAY)
53+
.baseDelay(SdkDefaultRetrySetting.baseDelay(retryMode))
4954
.maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF)
5055
.build();
5156
}
5257

5358
static BackoffStrategy defaultThrottlingStrategy() {
54-
return EqualJitterBackoffStrategy.builder()
55-
.baseDelay(SdkDefaultRetrySetting.THROTTLED_BASE_DELAY)
56-
.maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF)
57-
.build();
59+
return defaultThrottlingStrategy(RetryMode.defaultRetryMode());
60+
}
61+
62+
static BackoffStrategy defaultThrottlingStrategy(RetryMode retryMode) {
63+
switch (retryMode) {
64+
case LEGACY:
65+
return EqualJitterBackoffStrategy.builder()
66+
.baseDelay(SdkDefaultRetrySetting.throttledBaseDelay(retryMode))
67+
.maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF)
68+
.build();
69+
case STANDARD:
70+
return FullJitterBackoffStrategy.builder()
71+
.baseDelay(SdkDefaultRetrySetting.throttledBaseDelay(retryMode))
72+
.maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF)
73+
.build();
74+
default:
75+
throw new IllegalStateException("Unsupported RetryMode: " + retryMode);
76+
}
5877
}
5978

6079
static BackoffStrategy none() {

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ private FullJitterBackoffStrategy(BuilderImpl builder) {
6161
@Override
6262
public Duration computeDelayBeforeNextRetry(RetryPolicyContext context) {
6363
int ceil = calculateExponentialDelay(context.retriesAttempted(), baseDelay, maxBackoffTime);
64-
return Duration.ofMillis(random.nextInt(ceil));
64+
// Minimum of 1 ms (consistent with BackoffStrategy.none()'s behavior)
65+
return Duration.ofMillis(random.nextInt(ceil) + 1L);
6566
}
6667

6768
@Override

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/RetryPolicyTest.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.mockito.Mockito.verify;
2121

22+
import java.time.Duration;
2223
import org.junit.Assert;
2324
import org.junit.Test;
2425
import org.junit.runner.RunWith;
2526
import org.mockito.Mock;
2627
import org.mockito.runners.MockitoJUnitRunner;
2728
import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
29+
import software.amazon.awssdk.core.retry.backoff.EqualJitterBackoffStrategy;
30+
import software.amazon.awssdk.core.retry.backoff.FullJitterBackoffStrategy;
2831
import software.amazon.awssdk.core.retry.conditions.RetryCondition;
2932

3033
@RunWith(MockitoJUnitRunner.class)
@@ -117,4 +120,36 @@ public void maxRetriesFromDefaultRetryModeIsCorrect() {
117120
Assert.fail();
118121
}
119122
}
123+
124+
@Test
125+
public void legacyRetryMode_shouldUseFullJitterAndEqualJitter() {
126+
RetryPolicy legacyRetryPolicy = RetryPolicy.forRetryMode(RetryMode.LEGACY);
127+
128+
assertThat(legacyRetryPolicy.backoffStrategy()).isInstanceOf(FullJitterBackoffStrategy.class);
129+
FullJitterBackoffStrategy backoffStrategy = (FullJitterBackoffStrategy) legacyRetryPolicy.backoffStrategy();
130+
assertThat(backoffStrategy.toBuilder().baseDelay()).isEqualTo(Duration.ofMillis(100));
131+
assertThat(backoffStrategy.toBuilder().maxBackoffTime()).isEqualTo(Duration.ofSeconds(20));
132+
133+
assertThat(legacyRetryPolicy.throttlingBackoffStrategy()).isInstanceOf(EqualJitterBackoffStrategy.class);
134+
EqualJitterBackoffStrategy throttlingBackoffStrategy =
135+
(EqualJitterBackoffStrategy) legacyRetryPolicy.throttlingBackoffStrategy();
136+
assertThat(throttlingBackoffStrategy.toBuilder().baseDelay()).isEqualTo(Duration.ofMillis(500));
137+
assertThat(throttlingBackoffStrategy.toBuilder().maxBackoffTime()).isEqualTo(Duration.ofSeconds(20));
138+
}
139+
140+
@Test
141+
public void standardRetryMode_shouldUseFullJitterOnly() {
142+
RetryPolicy standardRetryPolicy = RetryPolicy.forRetryMode(RetryMode.STANDARD);
143+
144+
assertThat(standardRetryPolicy.backoffStrategy()).isInstanceOf(FullJitterBackoffStrategy.class);
145+
FullJitterBackoffStrategy backoffStrategy = (FullJitterBackoffStrategy) standardRetryPolicy.backoffStrategy();
146+
assertThat(backoffStrategy.toBuilder().baseDelay()).isEqualTo(Duration.ofSeconds(1));
147+
assertThat(backoffStrategy.toBuilder().maxBackoffTime()).isEqualTo(Duration.ofSeconds(20));
148+
149+
assertThat(standardRetryPolicy.throttlingBackoffStrategy()).isInstanceOf(FullJitterBackoffStrategy.class);
150+
FullJitterBackoffStrategy throttlingBackoffStrategy =
151+
(FullJitterBackoffStrategy) standardRetryPolicy.throttlingBackoffStrategy();
152+
assertThat(throttlingBackoffStrategy.toBuilder().baseDelay()).isEqualTo(Duration.ofSeconds(1));
153+
assertThat(throttlingBackoffStrategy.toBuilder().maxBackoffTime()).isEqualTo(Duration.ofSeconds(20));
154+
}
120155
}

‎core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategyTest.java

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.core.retry.backoff;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.mockito.AdditionalAnswers.returnsFirstArg;
20+
import static org.mockito.Matchers.anyInt;
21+
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.when;
23+
24+
import java.time.Duration;
25+
import java.util.Arrays;
26+
import java.util.Collection;
27+
import java.util.Random;
28+
import org.junit.Before;
29+
import org.junit.Test;
30+
import org.junit.runner.RunWith;
31+
import org.junit.runners.Parameterized;
32+
import org.junit.runners.Parameterized.Parameter;
33+
import org.junit.runners.Parameterized.Parameters;
34+
import org.mockito.Mock;
35+
import org.mockito.stubbing.Answer;
36+
import software.amazon.awssdk.core.retry.RetryMode;
37+
import software.amazon.awssdk.core.retry.RetryPolicyContext;
38+
39+
@RunWith(Parameterized.class)
40+
public class FullJitterBackoffStrategyTest {
41+
42+
@Parameters
43+
public static Collection<TestCase> parameters() throws Exception {
44+
return Arrays.asList(
45+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
46+
.retriesAttempted(0)
47+
.expectedMaxDelay(Duration.ofSeconds(1))
48+
.expectedMedDelay(Duration.ofMillis(500))
49+
.expectedMinDelay(Duration.ofMillis(1)),
50+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
51+
.retriesAttempted(1)
52+
.expectedMaxDelay(Duration.ofSeconds(2))
53+
.expectedMedDelay(Duration.ofSeconds(1))
54+
.expectedMinDelay(Duration.ofMillis(1)),
55+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
56+
.retriesAttempted(2)
57+
.expectedMaxDelay(Duration.ofSeconds(4))
58+
.expectedMedDelay(Duration.ofSeconds(2))
59+
.expectedMinDelay(Duration.ofMillis(1)),
60+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
61+
.retriesAttempted(3)
62+
.expectedMaxDelay(Duration.ofSeconds(8))
63+
.expectedMedDelay(Duration.ofSeconds(4))
64+
.expectedMinDelay(Duration.ofMillis(1)),
65+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
66+
.retriesAttempted(4)
67+
.expectedMaxDelay(Duration.ofSeconds(16))
68+
.expectedMedDelay(Duration.ofSeconds(8))
69+
.expectedMinDelay(Duration.ofMillis(1)),
70+
new TestCase().backoffStrategy(BackoffStrategy.defaultStrategy(RetryMode.STANDARD))
71+
.retriesAttempted(5)
72+
.expectedMaxDelay(Duration.ofSeconds(20))
73+
.expectedMedDelay(Duration.ofSeconds(10))
74+
.expectedMinDelay(Duration.ofMillis(1))
75+
);
76+
}
77+
78+
@Parameter
79+
public TestCase testCase;
80+
81+
@Mock
82+
private Random mockRandom = mock(Random.class);
83+
84+
@Before
85+
public void setUp() throws Exception {
86+
testCase.backoffStrategy = injectMockRandom(testCase.backoffStrategy);
87+
}
88+
89+
@Test
90+
public void testMaxDelay() {
91+
mockMaxRandom();
92+
test(testCase.backoffStrategy, testCase.retriesAttempted, testCase.expectedMaxDelay);
93+
}
94+
95+
@Test
96+
public void testMedDelay() {
97+
mockMediumRandom();
98+
test(testCase.backoffStrategy, testCase.retriesAttempted, testCase.expectedMedDelay);
99+
}
100+
101+
@Test
102+
public void testMinDelay() {
103+
mockMinRandom();
104+
test(testCase.backoffStrategy, testCase.retriesAttempted, testCase.expectedMinDelay);
105+
}
106+
107+
private static void test(BackoffStrategy backoffStrategy, int retriesAttempted, Duration expectedDelay) {
108+
RetryPolicyContext context = RetryPolicyContext.builder()
109+
.retriesAttempted(retriesAttempted)
110+
.build();
111+
Duration computedDelay = backoffStrategy.computeDelayBeforeNextRetry(context);
112+
assertThat(computedDelay).isEqualTo(expectedDelay);
113+
}
114+
115+
private FullJitterBackoffStrategy injectMockRandom(BackoffStrategy strategy) {
116+
FullJitterBackoffStrategy.Builder builder = ((FullJitterBackoffStrategy) strategy).toBuilder();
117+
return new FullJitterBackoffStrategy(builder.baseDelay(), builder.maxBackoffTime(), mockRandom);
118+
}
119+
120+
private void mockMaxRandom() {
121+
when(mockRandom.nextInt(anyInt())).then((Answer<Integer>) invocationOnMock -> {
122+
Integer firstArg = (Integer) returnsFirstArg().answer(invocationOnMock);
123+
return firstArg - 1;
124+
});
125+
}
126+
127+
private void mockMinRandom() {
128+
when(mockRandom.nextInt(anyInt())).then((Answer<Integer>) invocationOnMock -> {
129+
return 0;
130+
});
131+
}
132+
133+
private void mockMediumRandom() {
134+
when(mockRandom.nextInt(anyInt())).then((Answer<Integer>) invocationOnMock -> {
135+
Integer firstArg = (Integer) returnsFirstArg().answer(invocationOnMock);
136+
return firstArg / 2 - 1;
137+
});
138+
}
139+
140+
private static class TestCase {
141+
private BackoffStrategy backoffStrategy;
142+
private int retriesAttempted;
143+
private Duration expectedMinDelay;
144+
private Duration expectedMedDelay;
145+
private Duration expectedMaxDelay;
146+
147+
public TestCase backoffStrategy(BackoffStrategy backoffStrategy) {
148+
this.backoffStrategy = backoffStrategy;
149+
return this;
150+
}
151+
152+
public TestCase retriesAttempted(int retriesAttempted) {
153+
this.retriesAttempted = retriesAttempted;
154+
return this;
155+
}
156+
157+
public TestCase expectedMinDelay(Duration expectedMinDelay) {
158+
this.expectedMinDelay = expectedMinDelay;
159+
return this;
160+
}
161+
162+
public TestCase expectedMedDelay(Duration expectedMedDelay) {
163+
this.expectedMedDelay = expectedMedDelay;
164+
return this;
165+
}
166+
167+
public TestCase expectedMaxDelay(Duration expectedMaxDelay) {
168+
this.expectedMaxDelay = expectedMaxDelay;
169+
return this;
170+
}
171+
}
172+
}

0 commit comments

Comments
 (0)