Add throttling backoff to all retry strategies (#5348)

* Add throttling backoff to all retry strategies

* Update javadoc as per suggestion

Author: sugmanue

.changes/next-release/bugfix-AWSSDKforJavav2-65f766c.json

@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "", 
+    "type": "bugfix", 
+    "description": "The retry strategies implementation was not backwards compatible with the retry policies in regards of throttled exceptions, for these the retry policies had a different backoff strategy that is much more slower. This change retrofits the retry strategies to have also a different backoff strategy for throttling errors that has the same base and max delay values as the legacy retry policy."
+}

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

@@ -188,7 +188,7 @@ default B retryOnRootCauseInstanceOf(Class<? extends Throwable> throwable) {
         }
 
         /**
-         * Configure the maximum number of attempts used by this executor.
+         * Configure the maximum number of attempts used by the retry strategy.
          *
          * <p>The actual number of attempts made may be less, depending on the retry strategy implementation. For example, the
          * standard and adaptive retry modes both employ short-circuiting which reduces the maximum attempts during outages.
@@ -198,12 +198,26 @@ default B retryOnRootCauseInstanceOf(Class<? extends Throwable> throwable) {
         B maxAttempts(int maxAttempts);
 
         /**
-         * Configure the backoff strategy used by this executor.
+         * Configure the backoff strategy used by the retry strategy.
          *
          * <p>By default, this uses jittered exponential backoff.
          */
         B backoffStrategy(BackoffStrategy backoffStrategy);
 
+        /**
+         * Configure the backoff strategy used for throttling exceptions by this strategy.
+         *
+         * <p>By default, this uses jittered exponential backoff.
+         */
+        B throttlingBackoffStrategy(BackoffStrategy throttlingBackoffStrategy);
+
+        /**
+         * Configure a predicate that determines whether a retryable exception is a throttling exception. When this predicate
+         * returns true, the retry strategy will use the {@link #throttlingBackoffStrategy}. If it returns false, the
+         * {@link #backoffStrategy} will be used. This predicate will not be called for non-retryable exceptions.
+         */
+        B treatAsThrottling(Predicate<Throwable> treatAsThrottling);
+
         /**
          * Build a new {@link RetryStrategy} with the current configuration on this builder.
          */

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

@@ -158,6 +158,16 @@ public BuilderToTestDefaults backoffStrategy(BackoffStrategy backoffStrategy) {
             return this;
         }
 
+        @Override
+        public BuilderToTestDefaults throttlingBackoffStrategy(BackoffStrategy backoffStrategy) {
+            return this;
+        }
+
+        @Override
+        public BuilderToTestDefaults treatAsThrottling(Predicate<Throwable> treatAsThrottling) {
+            return this;
+        }
+
         @Override
         public DummyRetryStrategy build() {
             return null;

core/retries/src/it/java/software/amazon/awssdk/retries/internal/AdaptiveRetryStrategyResourceConstrainedTest.java

@@ -71,6 +71,7 @@ void seemsToBeCorrectAndThreadSafe() {
             .tokenBucketStore(TokenBucketStore.builder().tokenBucketMaxCapacity(10_000).build())
             // Just wait for the rate limiter delays.
             .backoffStrategy(BackoffStrategy.retryImmediately())
+            .throttlingBackoffStrategy(BackoffStrategy.retryImmediately())
             .rateLimiterTokenBucketStore(RateLimiterTokenBucketStore.builder().build())
             .retryOnExceptionInstanceOf(ThrottlingException.class)
             .treatAsThrottling(x -> x instanceof ThrottlingException)

core/retries/src/main/java/software/amazon/awssdk/retries/AdaptiveRetryStrategy.java

@@ -19,7 +19,6 @@
 import software.amazon.awssdk.annotations.SdkPublicApi;
 import software.amazon.awssdk.annotations.ThreadSafe;
 import software.amazon.awssdk.retries.api.AcquireInitialTokenRequest;
-import software.amazon.awssdk.retries.api.BackoffStrategy;
 import software.amazon.awssdk.retries.api.RetryStrategy;
 import software.amazon.awssdk.retries.internal.DefaultAdaptiveRetryStrategy;
 import software.amazon.awssdk.retries.internal.circuitbreaker.TokenBucketStore;
@@ -30,10 +29,11 @@
  * <p>
  * Unlike {@link StandardRetryStrategy}, care should be taken when using this strategy. Specifically, it should be used:
  * <ol>
- * <li>When the availability of downstream resources are mostly affected by callers that are also using
- * the {@link AdaptiveRetryStrategy}.
- * <li>The scope (either the whole strategy or the {@link AcquireInitialTokenRequest#scope}) of the strategy is constrained
- * to target "resource", so that availability issues in one resource cannot delay other, unrelated resource's availability.
+ *   <li>When the availability of downstream resources are mostly affected by callers that are also using
+ *   the {@link AdaptiveRetryStrategy}.
+ *   <li>The scope (either the whole strategy or the {@link AcquireInitialTokenRequest#scope}) of the strategy is constrained
+ *   to target "resource", so that availability issues in one resource cannot delay other, unrelated resource's availability.
+ * </ol>
  * <p>
  * The adaptive retry strategy by default:
  * <ol>
@@ -70,8 +70,6 @@ static AdaptiveRetryStrategy.Builder builder() {
                                               .tokenBucketMaxCapacity(DefaultRetryStrategy.Standard.TOKEN_BUCKET_SIZE)
                                               .build())
             .tokenBucketExceptionCost(DefaultRetryStrategy.Standard.DEFAULT_EXCEPTION_TOKEN_COST)
-            .backoffStrategy(BackoffStrategy.exponentialDelay(DefaultRetryStrategy.Standard.BASE_DELAY,
-                                                              DefaultRetryStrategy.Standard.MAX_BACKOFF))
             .rateLimiterTokenBucketStore(RateLimiterTokenBucketStore.builder().build());
     }
 

core/retries/src/main/java/software/amazon/awssdk/retries/DefaultRetryStrategy.java

@@ -53,7 +53,10 @@ public static StandardRetryStrategy doNotRetry() {
     public static StandardRetryStrategy.Builder standardStrategyBuilder() {
         return StandardRetryStrategy.builder()
                                     .maxAttempts(Standard.MAX_ATTEMPTS)
-                                    .backoffStrategy(BackoffStrategy.exponentialDelay(Standard.BASE_DELAY, Standard.MAX_BACKOFF));
+                                    .backoffStrategy(BackoffStrategy.exponentialDelay(Standard.BASE_DELAY, Standard.MAX_BACKOFF))
+                                    .throttlingBackoffStrategy(BackoffStrategy.exponentialDelay(
+                                        Standard.THROTTLED_BASE_DELAY,
+                                        Standard.MAX_BACKOFF));
     }
 
     /**
@@ -91,12 +94,18 @@ public static LegacyRetryStrategy.Builder legacyStrategyBuilder() {
      */
     public static AdaptiveRetryStrategy.Builder adaptiveStrategyBuilder() {
         return AdaptiveRetryStrategy.builder()
-                                    .maxAttempts(Adaptive.MAX_ATTEMPTS);
+                                    .maxAttempts(Adaptive.MAX_ATTEMPTS)
+                                    .backoffStrategy(BackoffStrategy.exponentialDelay(Standard.BASE_DELAY,
+                                                                                      Standard.MAX_BACKOFF))
+                                    .throttlingBackoffStrategy(BackoffStrategy.exponentialDelay(
+                                        Standard.THROTTLED_BASE_DELAY,
+                                        Standard.MAX_BACKOFF));
     }
 
     static final class Standard {
         static final int MAX_ATTEMPTS = 3;
         static final Duration BASE_DELAY = Duration.ofMillis(100);
+        static final Duration THROTTLED_BASE_DELAY = Duration.ofSeconds(1);
         static final Duration MAX_BACKOFF = Duration.ofSeconds(20);
         static final int TOKEN_BUCKET_SIZE = 500;
         static final int DEFAULT_EXCEPTION_TOKEN_COST = 5;

core/retries/src/main/java/software/amazon/awssdk/retries/StandardRetryStrategy.java

@@ -71,7 +71,7 @@ static Builder builder() {
 
     interface Builder extends RetryStrategy.Builder<Builder, StandardRetryStrategy> {
         /**
-         * Whether circuit breaking is enabled for this executor.
+         * Whether circuit breaking is enabled for this strategy.
          *
          * <p>The circuit breaker will prevent attempts (even below the {@link #maxAttempts(int)}) if a large number of
          * failures are observed by this executor.

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

@@ -51,6 +51,8 @@ public abstract class BaseRetryStrategy implements RetryStrategy {
     protected final int maxAttempts;
     protected final boolean circuitBreakerEnabled;
     protected final BackoffStrategy backoffStrategy;
+    protected final BackoffStrategy throttlingBackoffStrategy;
+    protected final Predicate<Throwable> treatAsThrottling;
     protected final int exceptionCost;
     protected final TokenBucketStore tokenBucketStore;
 
@@ -60,6 +62,8 @@ public abstract class BaseRetryStrategy implements RetryStrategy {
         this.maxAttempts = Validate.isPositive(builder.maxAttempts, "maxAttempts");
         this.circuitBreakerEnabled = builder.circuitBreakerEnabled == null || builder.circuitBreakerEnabled;
         this.backoffStrategy = Validate.paramNotNull(builder.backoffStrategy, "backoffStrategy");
+        this.throttlingBackoffStrategy = Validate.paramNotNull(builder.throttlingBackoffStrategy, "throttlingBackoffStrategy");
+        this.treatAsThrottling = Validate.paramNotNull(builder.treatAsThrottling, "treatAsThrottling");
         this.exceptionCost = Validate.paramNotNull(builder.exceptionCost, "exceptionCost");
         this.tokenBucketStore = Validate.paramNotNull(builder.tokenBucketStore, "tokenBucketStore");
     }
@@ -149,7 +153,12 @@ protected Duration computeInitialBackoff(AcquireInitialTokenRequest request) {
      * compute different a different depending on their logic.
      */
     protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) {
-        Duration backoff = backoffStrategy.computeDelay(token.attempt());
+        Duration backoff;
+        if (treatAsThrottling.test(request.failure())) {
+            backoff = throttlingBackoffStrategy.computeDelay(token.attempt());
+        } else {
+            backoff = backoffStrategy.computeDelay(token.attempt());
+        }
         Duration suggested = request.suggestedDelay().orElse(Duration.ZERO);
         return maxOf(suggested, backoff);
     }
@@ -340,6 +349,8 @@ static class Builder {
         private Boolean circuitBreakerEnabled;
         private Integer exceptionCost;
         private BackoffStrategy backoffStrategy;
+        private BackoffStrategy throttlingBackoffStrategy;
+        private Predicate<Throwable> treatAsThrottling = throwable -> false;
         private TokenBucketStore tokenBucketStore;
 
         Builder() {
@@ -352,6 +363,8 @@ static class Builder {
             this.circuitBreakerEnabled = strategy.circuitBreakerEnabled;
             this.exceptionCost = strategy.exceptionCost;
             this.backoffStrategy = strategy.backoffStrategy;
+            this.throttlingBackoffStrategy = strategy.throttlingBackoffStrategy;
+            this.treatAsThrottling = strategy.treatAsThrottling;
             this.tokenBucketStore = strategy.tokenBucketStore;
         }
 
@@ -375,6 +388,14 @@ void setBackoffStrategy(BackoffStrategy backoffStrategy) {
             this.backoffStrategy = backoffStrategy;
         }
 
+        void setThrottlingBackoffStrategy(BackoffStrategy throttlingBackoffStrategy) {
+            this.throttlingBackoffStrategy = throttlingBackoffStrategy;
+        }
+
+        void setTreatAsThrottling(Predicate<Throwable> treatAsThrottling) {
+            this.treatAsThrottling = treatAsThrottling;
+        }
+
         void setTokenBucketExceptionCost(int exceptionCost) {
             this.exceptionCost = exceptionCost;
         }

core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultAdaptiveRetryStrategy.java

@@ -33,12 +33,10 @@ public final class DefaultAdaptiveRetryStrategy
     extends BaseRetryStrategy implements AdaptiveRetryStrategy {
 
     private static final Logger LOG = Logger.loggerFor(DefaultAdaptiveRetryStrategy.class);
-    private final Predicate<Throwable> treatAsThrottling;
     private final RateLimiterTokenBucketStore rateLimiterTokenBucketStore;
 
     DefaultAdaptiveRetryStrategy(Builder builder) {
         super(LOG, builder);
-        this.treatAsThrottling = Validate.paramNotNull(builder.treatAsThrottling, "treatAsThrottling");
         this.rateLimiterTokenBucketStore = Validate.paramNotNull(builder.rateLimiterTokenBucketStore,
                                                                  "rateLimiterTokenBucketStore");
     }
@@ -81,15 +79,13 @@ public static Builder builder() {
     }
 
     public static class Builder extends BaseRetryStrategy.Builder implements AdaptiveRetryStrategy.Builder {
-        private Predicate<Throwable> treatAsThrottling;
         private RateLimiterTokenBucketStore rateLimiterTokenBucketStore;
 
         Builder() {
         }
 
         Builder(DefaultAdaptiveRetryStrategy strategy) {
             super(strategy);
-            this.treatAsThrottling = strategy.treatAsThrottling;
             this.rateLimiterTokenBucketStore = strategy.rateLimiterTokenBucketStore;
         }
 
@@ -107,7 +103,7 @@ public Builder maxAttempts(int maxAttempts) {
 
         @Override
         public Builder treatAsThrottling(Predicate<Throwable> treatAsThrottling) {
-            this.treatAsThrottling = treatAsThrottling;
+            setTreatAsThrottling(treatAsThrottling);
             return this;
         }
 
@@ -117,6 +113,12 @@ public Builder backoffStrategy(BackoffStrategy backoffStrategy) {
             return this;
         }
 
+        @Override
+        public Builder throttlingBackoffStrategy(BackoffStrategy backoffStrategy) {
+            setThrottlingBackoffStrategy(backoffStrategy);
+            return this;
+        }
+
         public Builder circuitBreakerEnabled(Boolean circuitBreakerEnabled) {
             setCircuitBreakerEnabled(circuitBreakerEnabled);
             return this;

core/retries/src/main/java/software/amazon/awssdk/retries/internal/DefaultLegacyRetryStrategy.java

@@ -15,7 +15,6 @@
 
 package software.amazon.awssdk.retries.internal;
 
-import java.time.Duration;
 import java.util.function.Predicate;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.retries.LegacyRetryStrategy;
@@ -29,28 +28,11 @@
 public final class DefaultLegacyRetryStrategy
     extends BaseRetryStrategy implements LegacyRetryStrategy {
     private static final Logger LOG = Logger.loggerFor(LegacyRetryStrategy.class);
-    private final BackoffStrategy throttlingBackoffStrategy;
     private final int throttlingExceptionCost;
-    private final Predicate<Throwable> treatAsThrottling;
 
     DefaultLegacyRetryStrategy(Builder builder) {
         super(LOG, builder);
         this.throttlingExceptionCost = Validate.paramNotNull(builder.throttlingExceptionCost, "throttlingExceptionCost");
-        this.throttlingBackoffStrategy = Validate.paramNotNull(builder.throttlingBackoffStrategy, "throttlingBackoffStrategy");
-        this.treatAsThrottling = Validate.paramNotNull(builder.treatAsThrottling, "treatAsThrottling");
-    }
-
-    @Override
-    protected Duration computeBackoff(RefreshRetryTokenRequest request, DefaultRetryToken token) {
-        Duration backoff;
-        if (treatAsThrottling.test(request.failure())) {
-            backoff = throttlingBackoffStrategy.computeDelay(token.attempt());
-        } else {
-            backoff = backoffStrategy.computeDelay(token.attempt());
-        }
-        // Take the max delay between the suggested delay and the backoff delay.
-        Duration suggested = request.suggestedDelay().orElse(Duration.ZERO);
-        return maxOf(suggested, backoff);
     }
 
     @Override
@@ -74,17 +56,13 @@ public static Builder builder() {
     }
 
     public static class Builder extends BaseRetryStrategy.Builder implements LegacyRetryStrategy.Builder {
-        private BackoffStrategy throttlingBackoffStrategy;
         private Integer throttlingExceptionCost;
-        private Predicate<Throwable> treatAsThrottling;
 
         Builder() {
         }
 
         Builder(DefaultLegacyRetryStrategy strategy) {
             super(strategy);
-            this.throttlingBackoffStrategy = strategy.throttlingBackoffStrategy;
-            this.treatAsThrottling = strategy.treatAsThrottling;
             this.throttlingExceptionCost = strategy.throttlingExceptionCost;
         }
 
@@ -108,7 +86,7 @@ public Builder backoffStrategy(BackoffStrategy backoffStrategy) {
 
         @Override
         public Builder throttlingBackoffStrategy(BackoffStrategy throttlingBackoffStrategy) {
-            this.throttlingBackoffStrategy = throttlingBackoffStrategy;
+            setThrottlingBackoffStrategy(throttlingBackoffStrategy);
             return this;
         }
 
@@ -120,7 +98,7 @@ public Builder circuitBreakerEnabled(Boolean circuitBreakerEnabled) {
 
         @Override
         public Builder treatAsThrottling(Predicate<Throwable> treatAsThrottling) {
-            this.treatAsThrottling = treatAsThrottling;
+            setTreatAsThrottling(treatAsThrottling);
             return this;
         }