Skip to content

Update the default retry strategy to standard from legacy #6294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/feature-AWSSDKforJavav2-8c493dd.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "feature",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Update the default retry strategy to standard based on [Modernizing the default retry strategy](https://aws.amazon.com/blogs/developer/updating-aws-sdk-defaults-aws-sts-service-endpoint-and-retry-strategy/)"
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static Resolver resolver() {
* Allows customizing the variables used during determination of a {@link RetryMode}. Created via {@link #resolver()}.
*/
public static class Resolver {
private static final RetryMode SDK_DEFAULT_RETRY_MODE = LEGACY;
private static final RetryMode SDK_DEFAULT_RETRY_MODE = STANDARD;

private Supplier<ProfileFile> profileFile;
private String profileName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ public void testRetryIoExceptionFromExecute() throws Exception {
Assert.assertSame(ioException, e.getCause());
}

// Verify that we called execute 4 times.
verify(sdkHttpClient, times(4)).prepareRequest(any());
// Verify that we called execute 3 times.
verify(sdkHttpClient, times(3)).prepareRequest(any());
}

@Test
Expand All @@ -119,8 +119,8 @@ public void testRetryIoExceptionFromHandler() throws Exception {
Assert.assertSame(exception, e.getCause());
}

// Verify that we called execute 4 times.
verify(mockHandler, times(4)).handle(any(), any());
// Verify that we called execute 3 times.
verify(mockHandler, times(3)).handle(any(), any());
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ public void closesAllCreatedInputStreamsFromProvider() {
} catch (SdkServiceException ignored) {
}

// The test client uses the default retry policy so there should be 4
// The test client uses the default retry policy so there should be 3
// total attempts and an equal number created streams
assertThat(provider.getCreatedStreams().size()).isEqualTo(4);
assertThat(provider.getCreatedStreams().size()).isEqualTo(3);
for (CloseTrackingInputStream is : provider.getCreatedStreams()) {
assertThat(is.isClosed()).isTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public class RetryModeTest {
public static Collection<Object> data() {
return Arrays.asList(new Object[] {
// Test defaults
new TestData(null, null, null, null, RetryMode.LEGACY),
new TestData(null, null, "PropertyNotSet", null, RetryMode.LEGACY),
new TestData(null, null, null, null, RetryMode.STANDARD),
new TestData(null, null, "PropertyNotSet", null, RetryMode.STANDARD),

// Test resolution
new TestData("legacy", null, null, null, RetryMode.LEGACY),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public class RetryPolicyMaxRetriesTest {
public static Collection<Object> data() {
return Arrays.asList(new Object[] {
// Test defaults
new TestData(null, null, null, null, null, 3),
new TestData(null, null, null, null, "PropertyNotSet", 3),
new TestData(null, null, null, null, null, 2),
new TestData(null, null, null, null, "PropertyNotSet", 2),

// Test precedence
new TestData("9", "2", "standard", "standard", "PropertySetToStandard", 8),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void nonRetryPolicy_shouldUseNullCondition() {
@Test
public void nonRetryMode_shouldUseDefaultRetryMode() {
RetryPolicy policy = RetryPolicy.builder().build();
assertThat(policy.retryMode().toString()).isEqualTo("LEGACY");
assertThat(policy.retryMode().toString()).isEqualTo("STANDARD");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public class RetryStrategyMaxRetriesTest {
public static Collection<Object> data() {
return Arrays.asList(new Object[] {
// Test defaults
new TestData(null, null, null, null, null, 4),
new TestData(null, null, null, null, "PropertyNotSet", 4),
new TestData(null, null, null, null, null, 3),
new TestData(null, null, null, null, "PropertyNotSet", 3),

// Test precedence
new TestData("9", "2", "standard", "standard",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ void resolve_retryModeNotSetWithEnvNorSupplier_resolvesFromSdkDefault() {
RetryStrategy retryStrategy = DynamoDbRetryPolicy.resolveRetryStrategy(sdkClientConfiguration);
RetryMode retryMode = SdkDefaultRetryStrategy.retryMode(retryStrategy);

assertThat(retryMode).isEqualTo(RetryMode.LEGACY);
assertThat(retryMode).isEqualTo(RetryMode.STANDARD);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void handlerThrowsRetryableException_RetriedUpToLimit() throws Exception
});
assertThatThrownBy(() -> s3.getObject(getObjectRequest(), handler))
.isInstanceOf(SdkClientException.class);
assertThat(handler.currentCallCount()).isEqualTo(4);
assertThat(handler.currentCallCount()).isEqualTo(3);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void cleanup() {

private static Stream<Arguments> inputValues() {
return Stream.of(
Arguments.of("Default values", null, Arrays.asList("D", "N", "P", "T")),
Arguments.of("Default values", null, Arrays.asList("E", "N", "P", "T")),
Arguments.of("Account ID preferred mode ", AccountIdEndpointMode.PREFERRED, Arrays.asList("P", "T")),
Arguments.of("Account ID disabled mode ", AccountIdEndpointMode.DISABLED, Arrays.asList("Q", "T")),
Arguments.of("Account ID required mode ", AccountIdEndpointMode.REQUIRED, Arrays.asList("R", "T"))
Expand All @@ -98,6 +98,7 @@ void validate_metricsString_forDifferentConfigValues(String description,
assertThatThrownBy(() -> clientBuilder.build().operationWithNoInputOrOutput(r -> {}).join()).hasMessageContaining("stop");

String userAgent = assertAndGetUserAgentString();
System.out.println("userAgent "+userAgent);
expectedMetrics.forEach(expectedMetric -> assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedMetric)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public void exceptionMessage_ioException_includesMultipleAttempts() {
SdkClientException exception = assertThrows(SdkClientException.class,
() -> callAllTypes(client));

assertThat(exception.getMessage()).contains("SDK Attempt Count: 4");
wireMock.verify(4, postRequestedFor(anyUrl()));
assertThat(exception.getMessage()).contains("SDK Attempt Count: 3");
wireMock.verify(3, postRequestedFor(anyUrl()));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ public void request_settingRetryModeInOverrideConfigurationConsumerRunTwice() {

// Configuring the client using an unrelated plugin should not remember the previous settings.
assertThrows(ProtocolRestJsonException.class, () -> callAllTypes(client, Collections.singletonList(unrelatedPlugin)));
// Four retries, the LEGACY retry strategy is back in.
verifyRequestCount(3 + 4);
// 3 retries, the STANDARD retry strategy is back in.
verifyRequestCount(3 + 3);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void syncClient_serviceClientConfiguration_withoutOverrideConfiguration_s
assertThat(overrideConfig.apiCallAttemptTimeout()).isNotPresent();
assertThat(overrideConfig.apiCallTimeout()).isNotPresent();
assertThat(overrideConfig.retryPolicy()).isNotPresent();
assertThat(overrideConfig.retryStrategy().get().maxAttempts()).isEqualTo(4);
assertThat(overrideConfig.retryStrategy().get().maxAttempts()).isEqualTo(3);
assertThat(overrideConfig.defaultProfileFile()).hasValue(ProfileFile.defaultProfileFile());
assertThat(overrideConfig.metricPublishers()).isEmpty();
}
Expand Down Expand Up @@ -196,7 +196,7 @@ public void asyncClient_serviceClientConfiguration_withoutOverrideConfiguration_
assertThat(overrideConfig.apiCallAttemptTimeout()).isNotPresent();
assertThat(overrideConfig.apiCallTimeout()).isNotPresent();
assertThat(overrideConfig.retryPolicy()).isNotPresent();
assertThat(overrideConfig.retryStrategy().get().maxAttempts()).isEqualTo(4);
assertThat(overrideConfig.retryStrategy().get().maxAttempts()).isEqualTo(3);
assertThat(overrideConfig.defaultProfileFile()).hasValue(ProfileFile.defaultProfileFile());
assertThat(overrideConfig.metricPublishers()).isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void syncClient_serviceClientConfiguration_withoutOverrideConfiguration_shouldRe
assertThat(overrideConfiguration.apiCallAttemptTimeout()).isNotPresent();
assertThat(overrideConfiguration.apiCallTimeout()).isNotPresent();
assertThat(overrideConfiguration.retryPolicy()).isNotPresent();
assertThat(overrideConfiguration.retryStrategy().get().maxAttempts()).isEqualTo(4);
assertThat(overrideConfiguration.retryStrategy().get().maxAttempts()).isEqualTo(3);
assertThat(overrideConfiguration.defaultProfileFile()).hasValue(ProfileFile.defaultProfileFile());
assertThat(overrideConfiguration.metricPublishers()).isEmpty();
}
Expand Down Expand Up @@ -196,7 +196,7 @@ void asyncClient_serviceClientConfiguration_withoutOverrideConfiguration_shouldR
assertThat(result.apiCallAttemptTimeout()).isNotPresent();
assertThat(result.apiCallTimeout()).isNotPresent();
assertThat(result.retryPolicy()).isNotPresent();
assertThat(result.retryStrategy().get().maxAttempts()).isEqualTo(4);
assertThat(result.retryStrategy().get().maxAttempts()).isEqualTo(3);
assertThat(result.defaultProfileFile()).hasValue(ProfileFile.defaultProfileFile());
assertThat(result.metricPublishers()).isEmpty();
}
Expand Down
Loading