From 5271bdd88bc155071674f71733ff6dff603e82d1 Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 12:52:29 -0400 Subject: [PATCH 1/6] updated values and added salt --- .../directconnectivity/GoneAndRetryWithRetryPolicy.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java index e9343dbaa1e61..8f1058c4f8225 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java @@ -23,6 +23,7 @@ import java.time.Duration; import java.time.Instant; +import java.util.Random; import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkNotNull; @@ -291,9 +292,10 @@ private Pair, Boolean> handleInvalidPartitionException(I class RetryWithRetryPolicy implements IRetryPolicy { private final static int DEFAULT_WAIT_TIME_IN_SECONDS = 30; - private final static int MAXIMUM_BACKOFF_TIME_IN_MS = 15000; + private final static int MAXIMUM_BACKOFF_TIME_IN_MS = 1000; private final static int INITIAL_BACKOFF_TIME_MS = 10; private final static int BACK_OFF_MULTIPLIER = 2; + private final static int RANDOM_SALT_IN_MS = 5; private volatile int attemptCount = 1; private volatile int currentBackoffMilliseconds = RetryWithRetryPolicy.INITIAL_BACKOFF_TIME_MS; @@ -311,6 +313,7 @@ public RetryWithRetryPolicy(Integer waitTimeInSeconds, RetryContext retryContext public Mono shouldRetry(Exception exception) { Duration backoffTime; Duration timeout; + Random random = new Random(); if (!(exception instanceof RetryWithException)) { logger.debug("Operation will NOT be retried. Current attempt {}, Exception: ", this.attemptCount, @@ -334,7 +337,7 @@ public Mono shouldRetry(Exception exception) { backoffTime = Duration.ofMillis( Math.min( - Math.min(this.currentBackoffMilliseconds, remainingMilliseconds), + Math.min(this.currentBackoffMilliseconds + random.nextInt(RANDOM_SALT_IN_MS), remainingMilliseconds), RetryWithRetryPolicy.MAXIMUM_BACKOFF_TIME_IN_MS)); this.currentBackoffMilliseconds *= RetryWithRetryPolicy.BACK_OFF_MULTIPLIER; logger.debug("BackoffTime: {} ms.", backoffTime.toMillis()); From 4bcf07950255081dc1f86a88af1d7a1b4d9d84eb Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 13:02:34 -0400 Subject: [PATCH 2/6] added tests --- .../implementation/RetryWithException.java | 7 +++ .../GoneAndRetryWithRetryPolicyTest.java | 45 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java index 30e04668b1fdf..aeedd1de09739 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java @@ -67,4 +67,11 @@ public RetryWithException(String message, HttpHeaders headers, URI requestUri) { super(message, innerException, HttpUtils.asMap(headers), HttpConstants.StatusCodes.RETRY_WITH, requestUri != null ? requestUri.toString() : null); } + + /** + * Instantiates a new Gone exception. + */ + public RetryWithException() { + this(RMResources.RetryWith, null); + } } diff --git a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java index 6019bbda468c3..4a04c41f2db34 100644 --- a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java +++ b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java @@ -17,6 +17,7 @@ import com.azure.cosmos.implementation.RxDocumentServiceRequest; import com.azure.cosmos.implementation.ShouldRetryResult; import com.azure.cosmos.implementation.guava25.base.Supplier; +import com.azure.cosmos.implementation.RetryWithException; import org.testng.annotations.Test; import reactor.core.publisher.Mono; @@ -321,4 +322,48 @@ public void shouldRetryWithGenericException() { assertThat(shouldRetryResult.shouldRetry).isFalse(); } + /** + * Test for custom retryWith values + */ + @Test(groups = { "unit" }, timeOut = TIMEOUT) + public void retryWithDefaultTimeouts() { + int defaultInitialDelayInMs = 10; + int defaultSalt = 5; + RxDocumentServiceRequest request = RxDocumentServiceRequest.create( + mockDiagnosticsClientContext(), + OperationType.Create, + ResourceType.Document); + GoneAndRetryWithRetryPolicy goneAndRetryWithRetryPolicy = new GoneAndRetryWithRetryPolicy(request, 30); + + RetryWithException retryWithException = new RetryWithException(); + + Mono singleShouldRetry = goneAndRetryWithRetryPolicy.shouldRetry(retryWithException); + ShouldRetryResult shouldRetryResult = singleShouldRetry.block(); + assertThat(shouldRetryResult.policyArg.getValue3()).isEqualTo(1); + validateRetryWithTimeRange(defaultInitialDelayInMs, shouldRetryResult, defaultSalt); + + singleShouldRetry = goneAndRetryWithRetryPolicy.shouldRetry(retryWithException); + shouldRetryResult = singleShouldRetry.block(); + assertThat(shouldRetryResult.policyArg.getValue3()).isEqualTo(2); + defaultInitialDelayInMs = defaultInitialDelayInMs * 2; //backoff multiplier + validateRetryWithTimeRange(defaultInitialDelayInMs, shouldRetryResult, defaultSalt); + + singleShouldRetry = goneAndRetryWithRetryPolicy.shouldRetry(retryWithException); + shouldRetryResult = singleShouldRetry.block(); + assertThat(shouldRetryResult.policyArg.getValue3()).isEqualTo(3); + defaultInitialDelayInMs = defaultInitialDelayInMs * 2; //backoff multiplier + validateRetryWithTimeRange(defaultInitialDelayInMs, shouldRetryResult, defaultSalt); + } + + private static void validateRetryWithTimeRange( + int expectedDelayInMs, + ShouldRetryResult retryResult, + Integer saltValueInMs) { + int saltValue = saltValueInMs == null ? 0 : saltValueInMs; + assertThat(retryResult.shouldRetry).isTrue(); + assertThat(retryResult.backOffTime.toMillis() >= 0).isTrue(); + assertThat(retryResult.backOffTime.toMillis() > expectedDelayInMs - saltValue).isTrue(); + assertThat(retryResult.backOffTime.toMillis() < expectedDelayInMs + saltValue).isTrue(); + } + } From 652f177823da840cce7abf33b726acd28548b3f4 Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 13:06:41 -0400 Subject: [PATCH 3/6] removed unused logic --- .../directconnectivity/GoneAndRetryWithRetryPolicyTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java index 4a04c41f2db34..9897cba7c4575 100644 --- a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java +++ b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java @@ -359,11 +359,10 @@ private static void validateRetryWithTimeRange( int expectedDelayInMs, ShouldRetryResult retryResult, Integer saltValueInMs) { - int saltValue = saltValueInMs == null ? 0 : saltValueInMs; assertThat(retryResult.shouldRetry).isTrue(); assertThat(retryResult.backOffTime.toMillis() >= 0).isTrue(); - assertThat(retryResult.backOffTime.toMillis() > expectedDelayInMs - saltValue).isTrue(); - assertThat(retryResult.backOffTime.toMillis() < expectedDelayInMs + saltValue).isTrue(); + assertThat(retryResult.backOffTime.toMillis() > expectedDelayInMs - saltValueInMs).isTrue(); + assertThat(retryResult.backOffTime.toMillis() < expectedDelayInMs + saltValueInMs).isTrue(); } } From 85f0086fc34baa0d741962b0e8bb8d17abe77cf8 Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 15:07:17 -0400 Subject: [PATCH 4/6] addressed comments --- .../azure/cosmos/implementation/RetryWithException.java | 7 ------- .../directconnectivity/GoneAndRetryWithRetryPolicy.java | 5 ++--- .../GoneAndRetryWithRetryPolicyTest.java | 3 ++- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java index aeedd1de09739..30e04668b1fdf 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RetryWithException.java @@ -67,11 +67,4 @@ public RetryWithException(String message, HttpHeaders headers, URI requestUri) { super(message, innerException, HttpUtils.asMap(headers), HttpConstants.StatusCodes.RETRY_WITH, requestUri != null ? requestUri.toString() : null); } - - /** - * Instantiates a new Gone exception. - */ - public RetryWithException() { - this(RMResources.RetryWith, null); - } } diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java index 8f1058c4f8225..d9a0cc30f199d 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java @@ -37,6 +37,7 @@ public class GoneAndRetryWithRetryPolicy implements IRetryPolicy { private volatile RetryWithException lastRetryWithException; private RetryContext retryContext; + static final ThreadLocal random = new ThreadLocal<>(); public GoneAndRetryWithRetryPolicy(RxDocumentServiceRequest request, Integer waitTimeInSeconds) { this.retryContext = BridgeInternal.getRetryContext(request.requestContext.cosmosDiagnostics); @@ -172,7 +173,6 @@ public Mono shouldRetry(Exception exception) { Duration timeout; boolean forceRefreshAddressCache; if (isNonRetryableException(exception)) { - logger.debug("Operation will NOT be retried. Current attempt {}, Exception: ", this.attemptCount, exception); return Mono.just(ShouldRetryResult.noRetryOnNonRelatedException()); @@ -313,7 +313,6 @@ public RetryWithRetryPolicy(Integer waitTimeInSeconds, RetryContext retryContext public Mono shouldRetry(Exception exception) { Duration backoffTime; Duration timeout; - Random random = new Random(); if (!(exception instanceof RetryWithException)) { logger.debug("Operation will NOT be retried. Current attempt {}, Exception: ", this.attemptCount, @@ -337,7 +336,7 @@ public Mono shouldRetry(Exception exception) { backoffTime = Duration.ofMillis( Math.min( - Math.min(this.currentBackoffMilliseconds + random.nextInt(RANDOM_SALT_IN_MS), remainingMilliseconds), + Math.min(this.currentBackoffMilliseconds + random.get().nextInt(RANDOM_SALT_IN_MS), remainingMilliseconds), RetryWithRetryPolicy.MAXIMUM_BACKOFF_TIME_IN_MS)); this.currentBackoffMilliseconds *= RetryWithRetryPolicy.BACK_OFF_MULTIPLIER; logger.debug("BackoffTime: {} ms.", backoffTime.toMillis()); diff --git a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java index 9897cba7c4575..9c6a360a92a80 100644 --- a/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java +++ b/sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java @@ -18,6 +18,7 @@ import com.azure.cosmos.implementation.ShouldRetryResult; import com.azure.cosmos.implementation.guava25.base.Supplier; import com.azure.cosmos.implementation.RetryWithException; +import org.mockito.Mockito; import org.testng.annotations.Test; import reactor.core.publisher.Mono; @@ -335,7 +336,7 @@ public void retryWithDefaultTimeouts() { ResourceType.Document); GoneAndRetryWithRetryPolicy goneAndRetryWithRetryPolicy = new GoneAndRetryWithRetryPolicy(request, 30); - RetryWithException retryWithException = new RetryWithException(); + RetryWithException retryWithException = Mockito.mock(RetryWithException.class); Mono singleShouldRetry = goneAndRetryWithRetryPolicy.shouldRetry(retryWithException); ShouldRetryResult shouldRetryResult = singleShouldRetry.block(); From 312460604196e890789e8f5d5906943ada15abbf Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 15:18:50 -0400 Subject: [PATCH 5/6] using threadlocalrandom --- .../directconnectivity/GoneAndRetryWithRetryPolicy.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java index d9a0cc30f199d..bb8e0daa3f0d8 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java @@ -24,6 +24,7 @@ import java.time.Duration; import java.time.Instant; import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkNotNull; @@ -37,7 +38,7 @@ public class GoneAndRetryWithRetryPolicy implements IRetryPolicy { private volatile RetryWithException lastRetryWithException; private RetryContext retryContext; - static final ThreadLocal random = new ThreadLocal<>(); + static final ThreadLocalRandom random = ThreadLocalRandom.current(); public GoneAndRetryWithRetryPolicy(RxDocumentServiceRequest request, Integer waitTimeInSeconds) { this.retryContext = BridgeInternal.getRetryContext(request.requestContext.cosmosDiagnostics); @@ -336,7 +337,7 @@ public Mono shouldRetry(Exception exception) { backoffTime = Duration.ofMillis( Math.min( - Math.min(this.currentBackoffMilliseconds + random.get().nextInt(RANDOM_SALT_IN_MS), remainingMilliseconds), + Math.min(this.currentBackoffMilliseconds + random.nextInt(RANDOM_SALT_IN_MS), remainingMilliseconds), RetryWithRetryPolicy.MAXIMUM_BACKOFF_TIME_IN_MS)); this.currentBackoffMilliseconds *= RetryWithRetryPolicy.BACK_OFF_MULTIPLIER; logger.debug("BackoffTime: {} ms.", backoffTime.toMillis()); From 581384ec5867282d003a7ec9b8fb6319ebbd5dc3 Mon Sep 17 00:00:00 2001 From: simorenoh Date: Thu, 7 Oct 2021 16:19:09 -0400 Subject: [PATCH 6/6] removed unused import, made random private --- .../directconnectivity/GoneAndRetryWithRetryPolicy.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java index bb8e0daa3f0d8..b5b5a14e5bbd0 100644 --- a/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java +++ b/sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java @@ -23,7 +23,6 @@ import java.time.Duration; import java.time.Instant; -import java.util.Random; import java.util.concurrent.ThreadLocalRandom; import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkNotNull; @@ -38,7 +37,7 @@ public class GoneAndRetryWithRetryPolicy implements IRetryPolicy { private volatile RetryWithException lastRetryWithException; private RetryContext retryContext; - static final ThreadLocalRandom random = ThreadLocalRandom.current(); + private static final ThreadLocalRandom random = ThreadLocalRandom.current(); public GoneAndRetryWithRetryPolicy(RxDocumentServiceRequest request, Integer waitTimeInSeconds) { this.retryContext = BridgeInternal.getRetryContext(request.requestContext.cosmosDiagnostics);