Updated retryWith values and added salting to match compute/.NET beha…

…vior (#24619) * updated values and added salt * added tests * removed unused logic * addressed comments * using threadlocalrandom * removed unused import, made random private
Azure · Oct 8, 2021 · a272b6d · a272b6d
1 parent e719c23
commit a272b6d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 3 deletions.
diff --git a/.../java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java b/.../java/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicy.java
@@ -23,6 +23,7 @@
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.concurrent.ThreadLocalRandom;
 
 import static com.azure.cosmos.implementation.guava25.base.Preconditions.checkNotNull;
 
@@ -36,6 +37,7 @@ public class GoneAndRetryWithRetryPolicy implements IRetryPolicy {
 
     private volatile RetryWithException lastRetryWithException;
     private RetryContext retryContext;
+    private static final ThreadLocalRandom random = ThreadLocalRandom.current();
 
     public GoneAndRetryWithRetryPolicy(RxDocumentServiceRequest request, Integer waitTimeInSeconds) {
         this.retryContext = BridgeInternal.getRetryContext(request.requestContext.cosmosDiagnostics);
@@ -170,7 +172,6 @@ public Mono<ShouldRetryResult> 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());
@@ -290,9 +291,10 @@ private Pair<Mono<ShouldRetryResult>, 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;
@@ -333,7 +335,7 @@ public Mono<ShouldRetryResult> 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());

diff --git a/...a/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java b/...a/com/azure/cosmos/implementation/directconnectivity/GoneAndRetryWithRetryPolicyTest.java
@@ -18,6 +18,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.mockito.Mockito;
 import org.testng.annotations.Test;
 import reactor.core.publisher.Mono;
@@ -325,6 +326,49 @@ 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 = Mockito.mock(RetryWithException.class);
+
+        Mono<ShouldRetryResult> 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) {
+        assertThat(retryResult.shouldRetry).isTrue();
+        assertThat(retryResult.backOffTime.toMillis() >= 0).isTrue();
+        assertThat(retryResult.backOffTime.toMillis() > expectedDelayInMs - saltValueInMs).isTrue();
+        assertThat(retryResult.backOffTime.toMillis() < expectedDelayInMs + saltValueInMs).isTrue();
+    }
+
     /**
      * After waitTimeInSeconds exhausted, retryWithException will not be retried.
      */