Merge pull request #8504 from vespa-engine/balder/use-a-more-agile-retrypolicy

Use a more agile retry policy with quick start and exponetial backoff.

3 files changed, 34 insertions, 24 deletions

diff --git a/messagebus/src/main/java/com/yahoo/messagebus/routing/RetryTransientErrorsPolicy.java b/messagebus/src/main/java/com/yahoo/messagebus/routing/RetryTransientErrorsPolicy.java
index 2623d2a6e0b..96655e91b90 100644
--- a/messagebus/src/main/java/com/yahoo/messagebus/routing/RetryTransientErrorsPolicy.java
+++ b/messagebus/src/main/java/com/yahoo/messagebus/routing/RetryTransientErrorsPolicy.java
@@ -3,6 +3,9 @@ package com.yahoo.messagebus.routing;
 
 import com.yahoo.messagebus.ErrorCode;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
 /**
  * Implements a retry policy that allows resending of any error that is not fatal. It also does progressive back-off,
  * delaying each attempt by the given time multiplied by the retry attempt.
@@ -11,8 +14,9 @@ import com.yahoo.messagebus.ErrorCode;
  */
 public class RetryTransientErrorsPolicy implements RetryPolicy {
 
-    private volatile boolean enabled = true;
-    private volatile double baseDelay = 1;
+    private static final double US = 1000000;
+    private final AtomicBoolean enabled = new AtomicBoolean(true);
+    private volatile AtomicLong baseDelayUS = new AtomicLong(1000);
 
     /**
      * Sets whether or not this policy should allow retries or not.
@@ -21,7 +25,7 @@ public class RetryTransientErrorsPolicy implements RetryPolicy {
      * @return This, to allow chaining.
      */
     public RetryTransientErrorsPolicy setEnabled(boolean enabled) {
-        this.enabled = enabled;
+        this.enabled.set(enabled);
         return this;
     }
 
@@ -32,17 +36,22 @@ public class RetryTransientErrorsPolicy implements RetryPolicy {
      * @return This, to allow chaining.
      */
     public RetryTransientErrorsPolicy setBaseDelay(double baseDelay) {
-        this.baseDelay = baseDelay;
+        this.baseDelayUS.set((long)(baseDelay*US));
         return this;
     }
 
     @Override
     public boolean canRetry(int errorCode) {
-        return enabled && errorCode < ErrorCode.FATAL_ERROR;
+        return enabled.get() && errorCode < ErrorCode.FATAL_ERROR;
     }
 
     @Override
     public double getRetryDelay(int retry) {
-        return baseDelay * retry;
+        long retryMultiplier = 0l;
+        if (retry > 1) {
+            retryMultiplier = 1L << Math.min(20, retry-1);
+        }
+
+        return Math.min(10.0, (retryMultiplier*baseDelayUS.get())/US);
     }
 }
diff --git a/messagebus/src/test/java/com/yahoo/messagebus/routing/ResenderTestCase.java b/messagebus/src/test/java/com/yahoo/messagebus/routing/ResenderTestCase.java
index 1e3764d8e4e..2973ea11278 100755
--- a/messagebus/src/test/java/com/yahoo/messagebus/routing/ResenderTestCase.java
+++ b/messagebus/src/test/java/com/yahoo/messagebus/routing/ResenderTestCase.java
@@ -155,11 +155,11 @@ public class ResenderTestCase {
         assertNull(((Receptor)dstSession.getMessageHandler()).getMessage(0));
 
         String trace = reply.getTrace().toString();
-        assertTrue(trace.contains("retry 1 in 0.01"));
+        assertTrue(trace.contains("retry 1 in 0.0"));
         assertTrue(trace.contains("retry 2 in 0.02"));
-        assertTrue(trace.contains("retry 3 in 0.03"));
-        assertTrue(trace.contains("retry 4 in 0.04"));
-        assertTrue(trace.contains("retry 5 in 0.05"));
+        assertTrue(trace.contains("retry 3 in 0.04"));
+        assertTrue(trace.contains("retry 4 in 0.08"));
+        assertTrue(trace.contains("retry 5 in 0.16"));
     }
 
     @Test
diff --git a/messagebus/src/test/java/com/yahoo/messagebus/routing/RetryPolicyTestCase.java b/messagebus/src/test/java/com/yahoo/messagebus/routing/RetryPolicyTestCase.java
index 95a6eabf6f1..522c4ba909c 100644
--- a/messagebus/src/test/java/com/yahoo/messagebus/routing/RetryPolicyTestCase.java
+++ b/messagebus/src/test/java/com/yahoo/messagebus/routing/RetryPolicyTestCase.java
@@ -12,26 +12,27 @@ import static org.junit.Assert.assertTrue;
  * @author Simon Thoresen Hult
  */
 public class RetryPolicyTestCase {
+    private static final double SMALL = 0.00000000000000000001;
 
     @Test
     public void testSimpleRetryPolicy() {
         RetryTransientErrorsPolicy policy = new RetryTransientErrorsPolicy();
-        for (int i = 0; i < 5; ++i) {
-            double delay = i / 3.0;
-            policy.setBaseDelay(delay);
-            for (int j = 0; j < 5; ++j) {
-                assertEquals((int)(j * delay), (int)policy.getRetryDelay(j));
-            }
-            for (int j = ErrorCode.NONE; j < ErrorCode.ERROR_LIMIT; ++j) {
-                policy.setEnabled(true);
-                if (j < ErrorCode.FATAL_ERROR) {
-                    assertTrue(policy.canRetry(j));
-                } else {
-                    assertFalse(policy.canRetry(j));
-                }
-                policy.setEnabled(false);
+        assertEquals(0.0, policy.getRetryDelay(0), SMALL);
+        assertEquals(0.0, policy.getRetryDelay(1), SMALL);
+        for (int i = 2; i < 15; i++) {
+            assertEquals(0.001*(1 << (i-1)), policy.getRetryDelay(i), SMALL);
+        }
+        assertEquals(10.0, policy.getRetryDelay(15), SMALL);
+        assertEquals(10.0, policy.getRetryDelay(20), SMALL);
+        for (int j = ErrorCode.NONE; j < ErrorCode.ERROR_LIMIT; ++j) {
+            policy.setEnabled(true);
+            if (j < ErrorCode.FATAL_ERROR) {
+                assertTrue(policy.canRetry(j));
+            } else {
                 assertFalse(policy.canRetry(j));
             }
+            policy.setEnabled(false);
+            assertFalse(policy.canRetry(j));
         }
     }