diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java index b60132f2ef10..790a9bfef51a 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/BackoffStrategy.java @@ -24,6 +24,12 @@ @FunctionalInterface public interface BackoffStrategy { + /** + * Max permitted retry times. To prevent exponentialDelay from overflow, there must be 2 ^ retriesAttempted + * <= 2 ^ 31 - 1, which means retriesAttempted <= 30, so that is the ceil for retriesAttempted. + */ + int RETRIES_ATTEMPTED_CEILING = (int) Math.floor(Math.log(Integer.MAX_VALUE) / Math.log(2)); + /** * Compute the delay before the next retry request. This strategy is only consulted when there will be a next retry. * @@ -33,7 +39,8 @@ public interface BackoffStrategy { Duration computeDelayBeforeNextRetry(RetryPolicyContext context); default int calculateExponentialDelay(int retriesAttempted, Duration baseDelay, Duration maxBackoffTime) { - return (int) Math.min((1L << retriesAttempted) * baseDelay.toMillis(), maxBackoffTime.toMillis()); + int cappedRetries = Math.min(retriesAttempted, RETRIES_ATTEMPTED_CEILING); + return (int) Math.min(baseDelay.multipliedBy(1L << cappedRetries).toMillis(), maxBackoffTime.toMillis()); } static BackoffStrategy defaultStrategy() { diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java index f6904ac636d1..8da04dd9ec64 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategy.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.core.retry.backoff; +import static software.amazon.awssdk.utils.NumericUtils.min; import static software.amazon.awssdk.utils.Validate.isNotNegative; import java.time.Duration; @@ -44,13 +45,21 @@ public final class EqualJitterBackoffStrategy implements BackoffStrategy, ToCopyableBuilder { + private static final Duration BASE_DELAY_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days + private static final Duration MAX_BACKOFF_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days + private final Duration baseDelay; private final Duration maxBackoffTime; - private final Random random = new Random(); + private final Random random; private EqualJitterBackoffStrategy(BuilderImpl builder) { - this.baseDelay = isNotNegative(builder.baseDelay, "baseDelay"); - this.maxBackoffTime = isNotNegative(builder.maxBackoffTime, "maxBackoffTime"); + this(builder.baseDelay, builder.maxBackoffTime, new Random()); + } + + EqualJitterBackoffStrategy(final Duration baseDelay, final Duration maxBackoffTime, final Random random) { + this.baseDelay = min(isNotNegative(baseDelay, "baseDelay"), BASE_DELAY_CEILING); + this.maxBackoffTime = min(isNotNegative(maxBackoffTime, "maxBackoffTime"), MAX_BACKOFF_CEILING); + this.random = random; } @Override diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java index 6e67dc585071..0c71534f4aca 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/retry/backoff/FullJitterBackoffStrategy.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.core.retry.backoff; +import static software.amazon.awssdk.utils.NumericUtils.min; import static software.amazon.awssdk.utils.Validate.isNotNegative; import java.time.Duration; @@ -40,14 +41,21 @@ public final class FullJitterBackoffStrategy implements BackoffStrategy, ToCopyableBuilder { + private static final Duration BASE_DELAY_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days + private static final Duration MAX_BACKOFF_CEILING = Duration.ofMillis(Integer.MAX_VALUE); // Around 24 days private final Duration baseDelay; private final Duration maxBackoffTime; - private final Random random = new Random(); + private final Random random; private FullJitterBackoffStrategy(BuilderImpl builder) { - this.baseDelay = isNotNegative(builder.baseDelay, "baseDelay"); - this.maxBackoffTime = isNotNegative(builder.maxBackoffTime, "maxBackoffTime"); + this(builder.baseDelay, builder.maxBackoffTime, new Random()); + } + + FullJitterBackoffStrategy(final Duration baseDelay, final Duration maxBackoffTime, final Random random) { + this.baseDelay = min(isNotNegative(baseDelay, "baseDelay"), BASE_DELAY_CEILING); + this.maxBackoffTime = min(isNotNegative(maxBackoffTime, "maxBackoffTime"), MAX_BACKOFF_CEILING); + this.random = random; } @Override diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategyTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategyTest.java new file mode 100644 index 000000000000..8e5723e140bb --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/retry/backoff/EqualJitterBackoffStrategyTest.java @@ -0,0 +1,116 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.core.retry.backoff; + +import org.junit.Test; +import org.mockito.Mock; +import software.amazon.awssdk.core.retry.RetryPolicyContext; + +import java.time.Duration; +import java.util.Random; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertThrows; + +public class EqualJitterBackoffStrategyTest { + + private static final int RANDOM_RESULT = 12345; + private static final Duration FIVE_DAYS = Duration.ofDays(5); + private static final Duration MAX_DURATION = Duration.ofSeconds(Long.MAX_VALUE); + private static final Duration ONE_SECOND = Duration.ofSeconds(1); + private static final Duration ONE_NANO_SECOND = Duration.ofNanos(1); + private static final int NANO_IN_MILLISECONDS = 1_000_000; + private static final Duration NEGATIVE_ONE_SECOND = Duration.ofSeconds(-1); + + @Mock + private Random mockRandom = mock(Random.class); + + @Test + public void exponentialDelayOverflowWithMaxBackoffTest() { + test(FIVE_DAYS, MAX_DURATION, 3, Integer.MAX_VALUE); + } + + @Test + public void exponentialDelayOverflowWithMaxBaseDelayTest() { + test(MAX_DURATION, MAX_DURATION, 1, Integer.MAX_VALUE); + } + + @Test + public void maxBaseDelayShortBackoffTest() { + test(MAX_DURATION, ONE_SECOND, 1, (int) ONE_SECOND.toMillis()); + } + + @Test + public void normalConditionTest() { + test(ONE_SECOND, MAX_DURATION, 10, (1 << 10) * (int) ONE_SECOND.toMillis()); + } + + @Test + public void tinyBackoffNormalRetriesTest() { + test(MAX_DURATION, ONE_NANO_SECOND, 10, 0); + } + + @Test + public void tinyBaseDelayNormalRetriesTest() { + test(ONE_NANO_SECOND, MAX_DURATION, 30, (int) (1L << 30) / NANO_IN_MILLISECONDS); + } + + @Test + public void tinyBaseDelayExtraRetriesTest() { + test(ONE_NANO_SECOND, MAX_DURATION, 100, + (int) (1L << 30) / NANO_IN_MILLISECONDS); // RETRIES_ATTEMPTED_CEILING == 30 + } + + @Test + public void exponentialDelayOverflowWithExtraRetriesTest() { + test(MAX_DURATION, MAX_DURATION, 100, Integer.MAX_VALUE); + } + + @Test + public void tinyBaseDelayUnderflowTest() { + test(ONE_NANO_SECOND, MAX_DURATION, 0, 0); + } + + @Test + public void negativeBaseDelayTest() { + assertThrows(IllegalArgumentException.class, () -> + test(NEGATIVE_ONE_SECOND, MAX_DURATION, 1, 0)); + } + + @Test + public void negativeBackoffTest() { + assertThrows(IllegalArgumentException.class, () -> + test(ONE_SECOND, NEGATIVE_ONE_SECOND, 1, 0)); + } + + private void test(final Duration baseDelay, final Duration maxBackoffTime, + final int retriesAttempted, final int expectedCeilingMillis) { + final BackoffStrategy backoffStrategy = new EqualJitterBackoffStrategy(baseDelay, maxBackoffTime, mockRandom); + + when(mockRandom.nextInt(expectedCeilingMillis /2 + 1)).thenReturn(RANDOM_RESULT); + + assertThat(backoffStrategy.computeDelayBeforeNextRetry(toRetryContext(retriesAttempted)), + is(Duration.ofMillis(expectedCeilingMillis / 2 + RANDOM_RESULT))); + } + + private static RetryPolicyContext toRetryContext(final int retriesAttempted) { + return RetryPolicyContext.builder().retriesAttempted(retriesAttempted).build(); + } + +} \ No newline at end of file diff --git a/utils/src/main/java/software/amazon/awssdk/utils/NumericUtils.java b/utils/src/main/java/software/amazon/awssdk/utils/NumericUtils.java index a6d95301d6b2..47aab28f2b7a 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/NumericUtils.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/NumericUtils.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.utils; +import java.time.Duration; import software.amazon.awssdk.annotations.SdkProtectedApi; @SdkProtectedApi @@ -41,4 +42,12 @@ public static int saturatedCast(long value) { return (int) value; } + public static Duration min(Duration a, Duration b) { + return (a.compareTo(b) < 0) ? a : b; + } + + public static Duration max(Duration a, Duration b) { + return (a.compareTo(b) > 0) ? a : b; + } + } diff --git a/utils/src/test/java/software/amazon/awssdk/utils/NumericUtilsTest.java b/utils/src/test/java/software/amazon/awssdk/utils/NumericUtilsTest.java new file mode 100644 index 000000000000..7bef92be97b2 --- /dev/null +++ b/utils/src/test/java/software/amazon/awssdk/utils/NumericUtilsTest.java @@ -0,0 +1,70 @@ +package software.amazon.awssdk.utils; + +import java.time.Duration; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.*; +import static software.amazon.awssdk.utils.NumericUtils.max; +import static software.amazon.awssdk.utils.NumericUtils.min; + +public class NumericUtilsTest { + + private final Duration SHORT_DURATION = Duration.ofMillis(10); + private final Duration SHORT_SAME_DURATION = Duration.ofMillis(10); + private final Duration LONG_DURATION = Duration.ofMillis(100); + private final Duration NEGATIVE_SHORT_DURATION = Duration.ofMillis(-10); + private final Duration NEGATIVE_SHORT_SAME_DURATION = Duration.ofMillis(-10); + private final Duration NEGATIVE_LONG_DURATION = Duration.ofMillis(-100); + + @Test + public void minTestDifferentDurations() { + assertThat(min(SHORT_DURATION, LONG_DURATION), is(SHORT_DURATION)); + } + + @Test + public void minTestDifferentDurationsReverse() { + assertThat(min(LONG_DURATION, SHORT_DURATION), is(SHORT_DURATION)); + } + + @Test + public void minTestSameDurations() { + assertThat(min(SHORT_DURATION, SHORT_SAME_DURATION), is(SHORT_SAME_DURATION)); + } + + @Test + public void minTestDifferentNegativeDurations() { + assertThat(min(NEGATIVE_SHORT_DURATION, NEGATIVE_LONG_DURATION), is(NEGATIVE_LONG_DURATION)); + } + + @Test + public void minTestNegativeSameDurations() { + assertThat(min(NEGATIVE_SHORT_DURATION, NEGATIVE_SHORT_SAME_DURATION), is(NEGATIVE_SHORT_DURATION)); + } + + @Test + public void maxTestDifferentDurations() { + assertThat(max(LONG_DURATION, SHORT_DURATION), is(LONG_DURATION)); + } + + @Test + public void maxTestDifferentDurationsReverse() { + assertThat(max(SHORT_DURATION, LONG_DURATION), is(LONG_DURATION)); + } + + @Test + public void maxTestSameDurations() { + assertThat(max(SHORT_DURATION, SHORT_SAME_DURATION), is(SHORT_SAME_DURATION)); + } + + @Test + public void maxTestDifferentNegativeDurations() { + assertThat(max(NEGATIVE_SHORT_DURATION, NEGATIVE_LONG_DURATION), is(NEGATIVE_SHORT_DURATION)); + } + + @Test + public void maxTestNegativeSameDurations() { + assertThat(max(NEGATIVE_SHORT_DURATION, NEGATIVE_SHORT_SAME_DURATION), is(NEGATIVE_SHORT_DURATION)); + } +} \ No newline at end of file