Skip to content

Remove type params from RetryStrategy, but keep them in RetryStrategy… #5262

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

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,15 @@ private void configureRetryPolicy(SdkClientConfiguration.Builder config) {
}

private void configureRetryStrategy(SdkClientConfiguration.Builder config) {
RetryStrategy<?, ?> strategy = config.option(SdkClientOption.RETRY_STRATEGY);
RetryStrategy strategy = config.option(SdkClientOption.RETRY_STRATEGY);
if (strategy != null) {
config.option(SdkClientOption.RETRY_STRATEGY, AwsRetryStrategy.configureStrategy(strategy.toBuilder()).build());
return;
}
config.lazyOption(SdkClientOption.RETRY_STRATEGY, this::resolveAwsRetryStrategy);
}

private RetryStrategy<?, ?> resolveAwsRetryStrategy(LazyValueSource config) {
private RetryStrategy resolveAwsRetryStrategy(LazyValueSource config) {
RetryMode retryMode = RetryMode.resolver()
.profileFile(config.get(SdkClientOption.PROFILE_FILE_SUPPLIER))
.profileName(config.get(SdkClientOption.PROFILE_NAME))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private AwsRetryStrategy() {
*
* @return The default retry strategy.
*/
public static RetryStrategy<?, ?> defaultRetryStrategy() {
public static RetryStrategy defaultRetryStrategy() {
return forRetryMode(RetryMode.defaultRetryMode());
}

Expand All @@ -51,7 +51,7 @@ private AwsRetryStrategy() {
* @param mode The retry mode for which we want to create a retry strategy.
* @return A retry strategy for the given retry mode.
*/
public static RetryStrategy<?, ?> forRetryMode(RetryMode mode) {
public static RetryStrategy forRetryMode(RetryMode mode) {
switch (mode) {
case STANDARD:
return standardRetryStrategy();
Expand All @@ -72,19 +72,19 @@ private AwsRetryStrategy() {
* @param strategy The strategy to update
* @return The updated strategy.
*/
public static RetryStrategy<?, ?> addRetryConditions(RetryStrategy<?, ?> strategy) {
public static RetryStrategy addRetryConditions(RetryStrategy strategy) {
return strategy.toBuilder()
.retryOnException(AwsRetryStrategy::retryOnAwsRetryableErrors)
.build();
}

/**
* Returns a retry strategy that does not retry.
* Returns a retry strategy that do not retry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does*

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 I will sneak in a fix on the next PR.

*
* @return A retry strategy that does not retry.
* @return A retry strategy that do not retry.
*/
public static RetryStrategy<?, ?> none() {
return DefaultRetryStrategy.none();
public static RetryStrategy doNotRetry() {
return DefaultRetryStrategy.doNotRetry();
}

/**
Expand Down Expand Up @@ -152,11 +152,11 @@ private static boolean retryOnAwsRetryableErrors(Throwable ex) {
}

/**
* Returns a {@link RetryStrategy<?, ?>} that implements the legacy {@link RetryMode#ADAPTIVE} mode.
* Returns a {@link RetryStrategy} that implements the legacy {@link RetryMode#ADAPTIVE} mode.
*
* @return a {@link RetryStrategy<?, ?>} that implements the legacy {@link RetryMode#ADAPTIVE} mode.
* @return a {@link RetryStrategy} that implements the legacy {@link RetryMode#ADAPTIVE} mode.
*/
private static RetryStrategy<?, ?> legacyAdaptiveRetryStrategy() {
private static RetryStrategy legacyAdaptiveRetryStrategy() {
return RetryPolicyAdapter.builder()
.retryPolicy(AwsRetryPolicy.forRetryMode(RetryMode.ADAPTIVE))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import java.util.function.Predicate;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.annotations.ThreadSafe;
import software.amazon.awssdk.utils.builder.CopyableBuilder;
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;

/**
* A strategy used by an SDK to determine when something should be retried.
Expand All @@ -41,10 +39,7 @@
*/
@ThreadSafe
@SdkPublicApi
public interface RetryStrategy<
B extends CopyableBuilder<B, T> & RetryStrategy.Builder<B, T>,
T extends ToCopyableBuilder<B, T> & RetryStrategy<B, T>>
extends ToCopyableBuilder<B, T> {
public interface RetryStrategy {
/**
* Invoked before the first request attempt.
*
Expand Down Expand Up @@ -95,16 +90,14 @@ public interface RetryStrategy<
*
* <p>This is useful for modifying the strategy's behavior, like conditions or max retries.
*/
@Override
B toBuilder();
Builder<?, ?> toBuilder();

/**
* Builder to create immutable instances of {@link RetryStrategy}.
*/
interface Builder<
B extends Builder<B, T> & CopyableBuilder<B, T>,
T extends ToCopyableBuilder<B, T> & RetryStrategy<B, T>>
extends CopyableBuilder<B, T> {
B extends Builder<B, T>,
T extends RetryStrategy> {
/**
* Configure the strategy to retry when the provided predicate returns true, given a failure exception.
*/
Expand Down Expand Up @@ -214,7 +207,6 @@ default B retryOnRootCauseInstanceOf(Class<? extends Throwable> throwable) {
/**
* Build a new {@link RetryStrategy} with the current configuration on this builder.
*/
@Override
T build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public DummyRetryStrategy build() {
}
}

static class DummyRetryStrategy implements RetryStrategy<BuilderToTestDefaults, DummyRetryStrategy> {
static class DummyRetryStrategy implements RetryStrategy {

@Override
public AcquireInitialTokenResponse acquireInitialToken(AcquireInitialTokenRequest request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
*/
@SdkPublicApi
@ThreadSafe
public interface AdaptiveRetryStrategy extends RetryStrategy<AdaptiveRetryStrategy.Builder, AdaptiveRetryStrategy> {
public interface AdaptiveRetryStrategy extends RetryStrategy {

/**
* Create a new {@link AdaptiveRetryStrategy.Builder}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ private DefaultRetryStrategy() {
}

/**
* Creates a non-retrying strategy.
* Returns a retry strategy that do not retry.
*/
public static StandardRetryStrategy none() {
public static StandardRetryStrategy doNotRetry() {
return standardStrategyBuilder()
.maxAttempts(1)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
*/
@SdkPublicApi
@ThreadSafe
public interface LegacyRetryStrategy extends RetryStrategy<LegacyRetryStrategy.Builder, LegacyRetryStrategy> {
public interface LegacyRetryStrategy extends RetryStrategy {
/**
* Create a new {@link LegacyRetryStrategy.Builder}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
*/
@SdkPublicApi
@ThreadSafe
public interface StandardRetryStrategy extends RetryStrategy<StandardRetryStrategy.Builder, StandardRetryStrategy> {
public interface StandardRetryStrategy extends RetryStrategy {
/**
* Create a new {@link StandardRetryStrategy.Builder}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,13 @@
import software.amazon.awssdk.retries.internal.circuitbreaker.TokenBucketStore;
import software.amazon.awssdk.utils.Logger;
import software.amazon.awssdk.utils.Validate;
import software.amazon.awssdk.utils.builder.CopyableBuilder;
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;

/**
* Generic class that implements that common logic for all the retries
* strategies with extension points for specific strategies to tailor
* the behavior to its needs.
* Generic class that implements that common logic for all the retries strategies with extension points for specific strategies to
* tailor the behavior to its needs.
*/
@SdkInternalApi
public abstract class BaseRetryStrategy<
B extends CopyableBuilder<B, T> & RetryStrategy.Builder<B, T>,
T extends ToCopyableBuilder<B, T> & RetryStrategy<B, T>> implements RetryStrategy<B, T> {
public abstract class BaseRetryStrategy implements RetryStrategy {

protected final Logger log;
protected final List<Predicate<Throwable>> retryPredicates;
Expand All @@ -70,8 +65,7 @@ public abstract class BaseRetryStrategy<
}

/**
* This method implements the logic of {@link
* RetryStrategy#acquireInitialToken(AcquireInitialTokenRequest)}.
* This method implements the logic of {@link RetryStrategy#acquireInitialToken(AcquireInitialTokenRequest)}.
*
* @see RetryStrategy#acquireInitialToken(AcquireInitialTokenRequest)
*/
Expand All @@ -83,8 +77,7 @@ public final AcquireInitialTokenResponse acquireInitialToken(AcquireInitialToken
}

/**
* This method implements the logic of {@link
* RetryStrategy#refreshRetryToken(RefreshRetryTokenRequest)}.
* This method implements the logic of {@link RetryStrategy#refreshRetryToken(RefreshRetryTokenRequest)}.
*
* @see RetryStrategy#refreshRetryToken(RefreshRetryTokenRequest)
*/
Expand Down Expand Up @@ -115,8 +108,7 @@ public final RefreshRetryTokenResponse refreshRetryToken(RefreshRetryTokenReques
}

/**
* This method implements the logic of {@link
* RetryStrategy#recordSuccess(RecordSuccessRequest)}.
* This method implements the logic of {@link RetryStrategy#recordSuccess(RecordSuccessRequest)}.
*
* @see RetryStrategy#recordSuccess(RecordSuccessRequest)
*/
Expand All @@ -143,24 +135,18 @@ public int maxAttempts() {
return maxAttempts;
}

@Override
public abstract B toBuilder();

/**
* Computes the backoff before the first attempt, by default
* {@link Duration#ZERO}. Extending classes can override
* this method to compute different a different depending on their
* logic.
* Computes the backoff before the first attempt, by default {@link Duration#ZERO}. Extending classes can override this method
* to compute different a different depending on their logic.
*/
protected Duration computeInitialBackoff(AcquireInitialTokenRequest request) {
return Duration.ZERO;
}

/**
* Computes the backoff before a retry using the configured
* backoff strategy. Extending classes can override
* this method to compute different a different depending on their
* logic.
* Computes the backoff before a retry using the configured backoff strategy. Extending classes can override this method to
* compute different a different depending on their logic.
*/
protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) {
Duration backoff = backoffStrategy.computeDelay(token.attempt());
Expand All @@ -169,24 +155,21 @@ protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetry
}

/**
* Called inside {@link #recordSuccess} to allow extending classes
* to update their internal state after a successful request.
* Called inside {@link #recordSuccess} to allow extending classes to update their internal state after a successful request.
*/
protected void updateStateForSuccess(DefaultRetryToken token) {
}

/**
* Called inside {@link #refreshRetryToken} to allow extending
* classes to update their internal state before retrying a
* Called inside {@link #refreshRetryToken} to allow extending classes to update their internal state before retrying a
* request.
*/
protected void updateStateForRetry(RefreshRetryTokenRequest request) {
}

/**
* Returns the amount of tokens to withdraw from the token
* bucket. Extending classes can override this method to tailor
* this amount for the specific kind of failure.
* Returns the amount of tokens to withdraw from the token bucket. Extending classes can override this method to tailor this
* amount for the specific kind of failure.
*/
protected int exceptionCost(RefreshRetryTokenRequest request) {
if (circuitBreakerEnabled) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

@SdkInternalApi
public final class DefaultAdaptiveRetryStrategy
extends BaseRetryStrategy<AdaptiveRetryStrategy.Builder, AdaptiveRetryStrategy> implements AdaptiveRetryStrategy {
extends BaseRetryStrategy implements AdaptiveRetryStrategy {

private static final Logger LOG = Logger.loggerFor(DefaultAdaptiveRetryStrategy.class);
private final Predicate<Throwable> treatAsThrottling;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

@SdkInternalApi
public final class DefaultLegacyRetryStrategy
extends BaseRetryStrategy<LegacyRetryStrategy.Builder, LegacyRetryStrategy> implements LegacyRetryStrategy {
extends BaseRetryStrategy implements LegacyRetryStrategy {
private static final Logger LOG = Logger.loggerFor(LegacyRetryStrategy.class);
private final BackoffStrategy throttlingBackoffStrategy;
private final int throttlingExceptionCost;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

@SdkInternalApi
public final class DefaultStandardRetryStrategy
extends BaseRetryStrategy<StandardRetryStrategy.Builder, StandardRetryStrategy> implements StandardRetryStrategy {
extends BaseRetryStrategy implements StandardRetryStrategy {
private static final Logger LOG = Logger.loggerFor(DefaultStandardRetryStrategy.class);

DefaultStandardRetryStrategy(Builder builder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void circuitBreakerRemembersState(Function<String, TestCase> defaultTestCaseSupp

// The test case will throw twice and then succeed, so each run will withdraw 2 * TEST_EXCEPTION_COST and deposit back
// TEST_EXCEPTION_COST.
RetryStrategy<?, ?> strategy = testCase.builder.build();
RetryStrategy strategy = testCase.builder.build();
int total = TEST_MAX;
for (int idx = 0; idx < 9; idx++) {
String name = testCase.name + " round " + idx;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,11 @@ public TestCase expectLastRecordedDelay(Duration delay) {
}

public void run() {
RetryStrategy<?, ?> strategy = builder.build();
RetryStrategy strategy = builder.build();
runTestCase(this, strategy);
}

public static void runTestCase(TestCase testCase, RetryStrategy<?, ?> strategy) {
public static void runTestCase(TestCase testCase, RetryStrategy strategy) {
AcquireInitialTokenResponse res = strategy.acquireInitialToken(AcquireInitialTokenRequestImpl.create(testCase.scope));
RetryToken token = res.token();
testCase.succeeded = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class AmazonHttpClientSslHandshakeTimeoutTest extends UnresponsiveMockSer
@Test(timeout = 60 * 1000)
public void testSslHandshakeTimeout() {
AmazonSyncHttpClient httpClient = HttpTestUtils.testClientBuilder()
.retryStrategy(DefaultRetryStrategy.none())
.retryStrategy(DefaultRetryStrategy.doNotRetry())
.httpClient(ApacheHttpClient.builder()
.socketTimeout(CLIENT_SOCKET_TO)
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static void tearDown() {
public void leasing_a_new_connection_fails_with_connection_pool_timeout() {

AmazonSyncHttpClient httpClient = HttpTestUtils.testClientBuilder()
.retryStrategy(DefaultRetryStrategy.none())
.retryStrategy(DefaultRetryStrategy.doNotRetry())
.httpClient(ApacheHttpClient.builder()
.connectionTimeout(Duration.ofMillis(100))
.maxConnections(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ protected SdkClientConfiguration invokePlugins(SdkClientConfiguration config) {
return config;
}

private String resolveRetryMode(RetryPolicy retryPolicy, RetryStrategy<?, ?> retryStrategy) {
private String resolveRetryMode(RetryPolicy retryPolicy, RetryStrategy retryStrategy) {
if (retryPolicy != null) {
return retryPolicy.retryMode().toString();
}
Expand All @@ -394,7 +394,7 @@ private String resolveClientUserAgent(LazyValueSource config) {
retryMode);
}

private RetryStrategy<?, ?> resolveRetryStrategy(LazyValueSource config) {
private RetryStrategy resolveRetryStrategy(LazyValueSource config) {
RetryMode retryMode = RetryMode.resolver()
.profileFile(config.get(PROFILE_FILE_SUPPLIER))
.profileName(config.get(PROFILE_NAME))
Expand Down
Loading
Loading