From 867d3a3373495cf5a007a3dbc13534f1f887a5ee Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 14 Feb 2019 15:09:02 +0100 Subject: Use a more agile retry policy with quick start and exponetial backoff. --- .../routing/RetryTransientErrorsPolicy.java | 21 +++++++++++----- .../yahoo/messagebus/routing/ResenderTestCase.java | 8 +++--- .../messagebus/routing/RetryPolicyTestCase.java | 29 +++++++++++----------- 3 files changed, 34 insertions(+), 24 deletions(-) (limited to 'messagebus') 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..915be2fa279 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 = Math.min(10000, 1L << (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)); } } -- cgit v1.2.3