Skip to content

Commit 43fcfef

Browse files
sugmanuesugmanue
authored
Change uses of RetryPolicy to RetryStrategy (#4125)
1 parent 5b116d2 commit 43fcfef

File tree

42 files changed

+263
-117
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+263
-117
lines changed

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ public class CustomizationConfig {
130130
*/
131131
private String customRetryPolicy;
132132

133+
/**
134+
* Custom Retry strategy
135+
*/
136+
private String customRetryStrategy;
137+
133138
private boolean skipSyncClientGeneration;
134139

135140
/**
@@ -416,10 +421,18 @@ public String getCustomRetryPolicy() {
416421
return customRetryPolicy;
417422
}
418423

424+
public String getCustomRetryStrategy() {
425+
return customRetryStrategy;
426+
}
427+
419428
public void setCustomRetryPolicy(String customRetryPolicy) {
420429
this.customRetryPolicy = customRetryPolicy;
421430
}
422431

432+
public void setCustomRetryStrategy(String customRetryStrategy) {
433+
this.customRetryStrategy = customRetryStrategy;
434+
}
435+
423436
public boolean isSkipSyncClientGeneration() {
424437
return skipSyncClientGeneration;
425438
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/IntermediateModel.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,10 @@ public String getCustomRetryPolicy() {
205205
return customizationConfig.getCustomRetryPolicy();
206206
}
207207

208+
public String getCustomRetryStrategy() {
209+
return customizationConfig.getCustomRetryStrategy();
210+
}
211+
208212
public String getSdkModeledExceptionBaseFqcn() {
209213
return String.format("%s.%s",
210214
metadata.getFullModelPackageName(),

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,12 @@ private MethodSpec finalizeServiceConfigurationMethod() {
342342
PoetUtils.classNameFromFqcn(model.getCustomizationConfig().getCustomRetryPolicy()));
343343
}
344344

345+
if (StringUtils.isNotBlank(model.getCustomizationConfig().getCustomRetryStrategy())) {
346+
builder.addCode(".option($1T.RETRY_STRATEGY, $2T.resolveRetryStrategy(config))",
347+
SdkClientOption.class,
348+
PoetUtils.classNameFromFqcn(model.getCustomizationConfig().getCustomRetryStrategy()));
349+
}
350+
345351
if (StringUtils.isNotBlank(clientConfigClassName)) {
346352
builder.addCode(".option($T.SERVICE_CONFIGURATION, finalServiceConfig)", SdkClientOption.class);
347353
}

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/test-client-builder-class.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.List;
55
import software.amazon.MyServiceHttpConfig;
66
import software.amazon.MyServiceRetryPolicy;
7+
import software.amazon.MyServiceRetryStrategy;
78
import software.amazon.awssdk.annotations.Generated;
89
import software.amazon.awssdk.annotations.SdkInternalApi;
910
import software.amazon.awssdk.auth.signer.Aws4Signer;
@@ -126,6 +127,7 @@ protected final SdkClientConfiguration finalizeServiceConfiguration(SdkClientCon
126127
.option(AwsClientOption.FIPS_ENDPOINT_ENABLED, finalServiceConfig.fipsModeEnabled())
127128
.option(SdkClientOption.EXECUTION_INTERCEPTORS, interceptors)
128129
.option(SdkClientOption.RETRY_POLICY, MyServiceRetryPolicy.resolveRetryPolicy(config))
130+
.option(SdkClientOption.RETRY_STRATEGY, MyServiceRetryStrategy.resolveRetryStrategy(config))
129131
.option(SdkClientOption.SERVICE_CONFIGURATION, finalServiceConfig).build();
130132
}
131133

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/json/customization.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"hasFipsProperty": true
1111
},
1212
"customRetryPolicy": "software.amazon.MyServiceRetryPolicy",
13+
"customRetryStrategy": "software.amazon.MyServiceRetryStrategy",
1314
"verifiedSimpleMethods" : ["paginatedOperationWithResultKey"],
1415
"blacklistedSimpleMethods" : [
1516
"eventStreamOperation"

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/rest-json/customization.config

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"hasAccelerateModeEnabledProperty":true
1515
},
1616
"customRetryPolicy": "software.amazon.MyServiceRetryPolicy",
17+
"customRetryStrategy": "software.amazon.MyServiceRetryStrategy",
1718
"verifiedSimpleMethods" : ["paginatedOperationWithResultKey"],
1819
"blacklistedSimpleMethods" : [
1920
"eventStreamOperation"

‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -372,28 +372,8 @@ private RetryPolicy resolveAwsRetryPolicy(SdkClientConfiguration config) {
372372
}
373373
}
374374

375-
RetryMode retryMode = RetryMode.resolver()
376-
.profileFile(config.option(SdkClientOption.PROFILE_FILE_SUPPLIER))
377-
.profileName(config.option(SdkClientOption.PROFILE_NAME))
378-
.defaultRetryMode(config.option(SdkClientOption.DEFAULT_RETRY_MODE))
379-
.resolve();
380-
return AwsRetryPolicy.forRetryMode(retryMode);
381-
// TODO: fixme This will be changed like this to pick the configured retry strategy
382-
// if no retry policy is configured.
383-
/*
384-
RetryPolicy policy = config.option(SdkClientOption.RETRY_POLICY);
385-
386-
if (policy != null) {
387-
if (policy.additionalRetryConditionsAllowed()) {
388-
return AwsRetryPolicy.addRetryConditions(policy);
389-
} else {
390-
return policy;
391-
}
392-
}
393-
394375
// If we don't have a configured retry policy we will use the configured retry strategy instead.
395376
return null;
396-
*/
397377
}
398378

399379
private RetryStrategy<?, ?> resolveAwsRetryStrategy(SdkClientConfiguration config) {

‎core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultsModeTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static software.amazon.awssdk.awscore.client.config.AwsClientOption.DEFAULTS_MODE;
2424
import static software.amazon.awssdk.core.client.config.SdkClientOption.DEFAULT_RETRY_MODE;
2525
import static software.amazon.awssdk.core.client.config.SdkClientOption.RETRY_POLICY;
26+
import static software.amazon.awssdk.core.client.config.SdkClientOption.RETRY_STRATEGY;
2627
import static software.amazon.awssdk.regions.ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT;
2728

2829
import java.time.Duration;
@@ -36,6 +37,7 @@
3637
import software.amazon.awssdk.awscore.internal.defaultsmode.DefaultsModeConfiguration;
3738
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
3839
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
40+
import software.amazon.awssdk.core.internal.retry.SdkDefaultRetryStrategy;
3941
import software.amazon.awssdk.core.retry.RetryMode;
4042
import software.amazon.awssdk.http.SdkHttpClient;
4143
import software.amazon.awssdk.http.SdkHttpConfigurationOption;
@@ -75,7 +77,7 @@ public void defaultClient_shouldUseLegacyModeWithExistingDefaults() {
7577
.build();
7678

7779
assertThat(client.clientConfiguration.option(DEFAULTS_MODE)).isEqualTo(DefaultsMode.LEGACY);
78-
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(RetryMode.defaultRetryMode());
80+
assertThat(SdkDefaultRetryStrategy.retryMode(client.clientConfiguration.option(RETRY_STRATEGY))).isEqualTo(RetryMode.defaultRetryMode());
7981
assertThat(client.clientConfiguration.option(DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT)).isNull();
8082
}
8183

@@ -97,7 +99,7 @@ public void nonLegacyDefaultsMode_shouldApplySdkDefaultsAndHttpDefaults() {
9799

98100
AttributeMap attributes = DefaultsModeConfiguration.defaultConfig(targetMode);
99101

100-
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(attributes.get(DEFAULT_RETRY_MODE));
102+
assertThat(SdkDefaultRetryStrategy.retryMode(client.clientConfiguration.option(RETRY_STRATEGY))).isEqualTo(attributes.get(DEFAULT_RETRY_MODE));
101103
assertThat(client.clientConfiguration.option(DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT)).isEqualTo("regional");
102104
}
103105

@@ -119,7 +121,7 @@ public void nonLegacyDefaultsModeAsyncClient_shouldApplySdkDefaultsAndHttpDefaul
119121

120122
AttributeMap attributes = DefaultsModeConfiguration.defaultConfig(targetMode);
121123

122-
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(attributes.get(DEFAULT_RETRY_MODE));
124+
assertThat(SdkDefaultRetryStrategy.retryMode(client.clientConfiguration.option(RETRY_STRATEGY))).isEqualTo(attributes.get(DEFAULT_RETRY_MODE));
123125
}
124126

125127
@Test

‎core/retries-api/src/main/java/software/amazon/awssdk/retries/api/RetryStrategy.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ public interface RetryStrategy<
8383
*/
8484
RecordSuccessResponse recordSuccess(RecordSuccessRequest request);
8585

86+
/**
87+
* Returns the maximum numbers attempts that this retry policy will allow.
88+
*
89+
* @return the maximum numbers attempts that this retry policy will allow.
90+
*/
91+
int maxAttempts();
92+
8693
/**
8794
* Create a new {@link Builder} with the current configuration.
8895
*

‎core/retries-api/src/test/java/software/amazon/awssdk/retries/api/RetryStrategyBuilderTest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,16 @@ public RecordSuccessResponse recordSuccess(RecordSuccessRequest request) {
176176
return null;
177177
}
178178

179+
@Override
180+
public int maxAttempts() {
181+
return 0;
182+
}
183+
179184
@Override
180185
public BuilderToTestDefaults toBuilder() {
181186
return null;
182187
}
188+
183189
}
184190

185191
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ public final RecordSuccessResponse recordSuccess(RecordSuccessRequest request) {
138138
return RecordSuccessResponse.create(refreshedToken);
139139
}
140140

141+
@Override
142+
public int maxAttempts() {
143+
return maxAttempts;
144+
}
145+
141146
@Override
142147
public abstract B toBuilder();
143148

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -358,22 +358,7 @@ private String resolveClientUserAgent(SdkClientConfiguration config, String retr
358358
}
359359

360360
private RetryPolicy resolveRetryPolicy(SdkClientConfiguration config) {
361-
RetryPolicy policy = config.option(SdkClientOption.RETRY_POLICY);
362-
if (policy != null) {
363-
return policy;
364-
}
365-
366-
RetryMode retryMode = RetryMode.resolver()
367-
.profileFile(config.option(SdkClientOption.PROFILE_FILE_SUPPLIER))
368-
.profileName(config.option(SdkClientOption.PROFILE_NAME))
369-
.defaultRetryMode(config.option(SdkClientOption.DEFAULT_RETRY_MODE))
370-
.resolve();
371-
return RetryPolicy.forRetryMode(retryMode);
372-
// TODO: fixme This will be changed like this to pick the configured retry strategy
373-
// if no retry policy is configured.
374-
/*
375361
return config.option(SdkClientOption.RETRY_POLICY);
376-
*/
377362
}
378363

379364
private RetryStrategy<?, ?> resolveRetryStrategy(SdkClientConfiguration config) {

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/SdkClientOptionValidation.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ private static void validateClientOptions(SdkClientConfiguration c) {
4646

4747
require("overrideConfiguration.additionalHttpHeaders", c.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS));
4848
require("overrideConfiguration.executionInterceptors", c.option(SdkClientOption.EXECUTION_INTERCEPTORS));
49-
// TODO: fixme, this will be removed as retryPolicy will be optional
50-
require("overrideConfiguration.retryPolicy", c.option(SdkClientOption.RETRY_POLICY));
5149
require("overrideConfiguration.retryStrategy", c.option(SdkClientOption.RETRY_STRATEGY));
5250

5351
require("overrideConfiguration.advancedOption[USER_AGENT_PREFIX]",

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/RetryableStage2.java

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
import software.amazon.awssdk.core.internal.http.pipeline.RequestToResponsePipeline;
2929
import software.amazon.awssdk.core.internal.http.pipeline.stages.utils.RetryableStageHelper2;
3030
import software.amazon.awssdk.http.SdkHttpFullRequest;
31+
import software.amazon.awssdk.http.SdkHttpFullResponse;
3132

3233
/**
3334
* Wrapper around the pipeline for a single request to provide retry, clock-skew and request throttling functionality.
3435
*/
3536
@SdkInternalApi
3637
public final class RetryableStage2<OutputT> implements RequestToResponsePipeline<OutputT> {
38+
private static final String RETRY_AFTER_HEADER = "Retry-After";
3739
private final RequestPipeline<SdkHttpFullRequest, Response<OutputT>> requestPipeline;
3840
private final HttpClientDependencies dependencies;
3941

@@ -54,8 +56,13 @@ public Response<OutputT> execute(SdkHttpFullRequest request, RequestExecutionCon
5456
Response<OutputT> response = executeRequest(retryableStageHelper, context);
5557
retryableStageHelper.recordAttemptSucceeded();
5658
return response;
57-
} catch (SdkException | IOException e) {
58-
retryableStageHelper.setLastException(e);
59+
} catch (SdkExceptionWithRetryAfterHint | SdkException | IOException e) {
60+
Throwable throwable = e;
61+
if (e instanceof SdkExceptionWithRetryAfterHint) {
62+
SdkExceptionWithRetryAfterHint wrapper = (SdkExceptionWithRetryAfterHint) e;
63+
throwable = wrapper.cause();
64+
}
65+
retryableStageHelper.setLastException(throwable);
5966
Duration suggestedDelay = suggestedDelay(e);
6067
Optional<Duration> backoffDelay = retryableStageHelper.tryRefreshToken(suggestedDelay);
6168
if (backoffDelay.isPresent()) {
@@ -70,6 +77,10 @@ public Response<OutputT> execute(SdkHttpFullRequest request, RequestExecutionCon
7077
}
7178

7279
private Duration suggestedDelay(Exception e) {
80+
if (e instanceof SdkExceptionWithRetryAfterHint) {
81+
SdkExceptionWithRetryAfterHint except = (SdkExceptionWithRetryAfterHint) e;
82+
return Duration.ofSeconds(except.retryAfter());
83+
}
7384
return Duration.ZERO;
7485
}
7586

@@ -83,8 +94,52 @@ private Response<OutputT> executeRequest(RetryableStageHelper2 retryableStageHel
8394
retryableStageHelper.setLastResponse(response.httpResponse());
8495
if (!response.isSuccess()) {
8596
retryableStageHelper.adjustClockIfClockSkew(response);
86-
throw response.exception();
97+
throw responseException(response);
8798
}
8899
return response;
89100
}
101+
102+
private RuntimeException responseException(Response<OutputT> response) {
103+
Optional<Integer> optionalRetryAfter = retryAfter(response.httpResponse());
104+
if (optionalRetryAfter.isPresent()) {
105+
return new SdkExceptionWithRetryAfterHint(optionalRetryAfter.get(), response.exception());
106+
}
107+
return response.exception();
108+
}
109+
110+
private Optional<Integer> retryAfter(SdkHttpFullResponse response) {
111+
Optional<String> optionalRetryAfterHeader = response.firstMatchingHeader(RETRY_AFTER_HEADER);
112+
if (optionalRetryAfterHeader.isPresent()) {
113+
String retryAfterHeader = optionalRetryAfterHeader.get();
114+
try {
115+
return Optional.of(Integer.parseInt(retryAfterHeader));
116+
} catch (NumberFormatException e) {
117+
// Ignore and fallback to returning empty.
118+
}
119+
}
120+
return Optional.empty();
121+
}
122+
123+
// This probably should go directly into SdkException
124+
static class SdkExceptionWithRetryAfterHint extends RuntimeException {
125+
private final SdkException cause;
126+
private final int seconds;
127+
128+
SdkExceptionWithRetryAfterHint(int seconds, SdkException cause) {
129+
this.seconds = seconds;
130+
this.cause = cause;
131+
}
132+
133+
public int retryAfter() {
134+
return seconds;
135+
}
136+
137+
public SdkException cause() {
138+
return cause;
139+
}
140+
141+
public int seconds() {
142+
return seconds;
143+
}
144+
}
90145
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/RetryableStageHelper2.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,9 @@ public void logBackingOff(Duration backoffDelay) {
173173
* Retrieve the request to send to the service, including any detailed retry information headers.
174174
*/
175175
public SdkHttpFullRequest requestToSend() {
176-
// TODO: fixme, we don't longer have this information handy we need to change the interface to access it.
177-
int maxAllowedRetries = 3;
178176
return request.toBuilder()
179-
.putHeader(SDK_RETRY_INFO_HEADER, "attempt=" + attemptNumber + "; max=" + maxAllowedRetries)
177+
.putHeader(SDK_RETRY_INFO_HEADER, "attempt=" + attemptNumber
178+
+ "; max=" + retryStrategy().maxAttempts())
180179
.build();
181180
}
182181

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ public RecordSuccessResponse recordSuccess(RecordSuccessRequest request) {
7777
return RecordSuccessResponse.create(token);
7878
}
7979

80+
@Override
81+
public int maxAttempts() {
82+
return retryPolicy.numRetries() + 1;
83+
}
84+
8085
@Override
8186
public Builder toBuilder() {
8287
return new Builder(this);

0 commit comments

Comments
 (0)