[james-project] 11/15: JAMES-2817 WaitDelayGenerator jitter could remove time from next iteration

[aduprat]

This is an automated email from the ASF dual-hosted git repository.

aduprat pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit d3c5a61f0591a9538be670188c59894c8f5862c8
Author: Matthieu Baechler <matth...@apache.org>
AuthorDate: Fri Feb 15 12:31:45 2019 +0100

    JAMES-2817 WaitDelayGenerator jitter could remove time from next iteration
---
 .../mailbox/events/ErrorHandlingContract.java      |  4 ++--
 .../james/mailbox/events/WaitDelayGenerator.java   | 24 +++++++++++++---------
 .../mailbox/events/WaitDelayGeneratorTest.java     |  8 ++++----
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git 
a/mailbox/api/src/test/java/org/apache/james/mailbox/events/ErrorHandlingContract.java
 
b/mailbox/api/src/test/java/org/apache/james/mailbox/events/ErrorHandlingContract.java
index c6f4494..d0b3923 100644
--- 
a/mailbox/api/src/test/java/org/apache/james/mailbox/events/ErrorHandlingContract.java
+++ 
b/mailbox/api/src/test/java/org/apache/james/mailbox/events/ErrorHandlingContract.java
@@ -167,8 +167,8 @@ interface ErrorHandlingContract extends EventBusContract {
             softly.assertThat(timeElapsed).hasSize(4);
 
             long minFirstDelayAfter = 100; // first backOff
-            long minSecondDelayAfter = 100; // 200 * jitter factor (200 * 0.5)
-            long minThirdDelayAfter = 200; // 400 * jitter factor (400 * 0.5)
+            long minSecondDelayAfter = 50; // 50 * jitter factor (50 * 0.5)
+            long minThirdDelayAfter = 100; // 100 * jitter factor (100 * 0.5)
 
             softly.assertThat(timeElapsed.get(1))
                 
.isAfterOrEqualTo(timeElapsed.get(0).plusMillis(minFirstDelayAfter));
diff --git 
a/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/WaitDelayGenerator.java
 
b/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/WaitDelayGenerator.java
index ff0378b..33a3586 100644
--- 
a/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/WaitDelayGenerator.java
+++ 
b/mailbox/event/event-rabbitmq/src/main/java/org/apache/james/mailbox/events/WaitDelayGenerator.java
@@ -24,7 +24,7 @@ import java.time.Duration;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-
+import com.google.common.primitives.Ints;
 import reactor.core.publisher.Mono;
 import reactor.core.scheduler.Schedulers;
 
@@ -34,12 +34,14 @@ class WaitDelayGenerator {
         return new WaitDelayGenerator(retryBackoff);
     }
 
-    private static int randomBetween(int lowest, int highest) {
-        Preconditions.checkArgument(lowest <= highest, "lowest always has to 
be less than or equals highest");
-        if (lowest == highest) {
-            return lowest;
+    private static Duration randomBetween(Duration base, Duration jitter) {
+        Preconditions.checkArgument(!jitter.isNegative(), "jitter value should 
always be positive");
+        if (jitter.isZero()) {
+            return base;
         }
-        return SECURE_RANDOM.nextInt(highest - lowest) + lowest;
+        long maxJitterAsMillis = jitter.toMillis();
+        long jitterAsMillis = 
SECURE_RANDOM.nextInt(Ints.checkedCast(maxJitterAsMillis * 2)) / 2;
+        return base.plusMillis(jitterAsMillis);
     }
 
     private static final SecureRandom SECURE_RANDOM = new SecureRandom();
@@ -65,11 +67,13 @@ class WaitDelayGenerator {
         if (!shouldDelay(retryCount)) {
             return Duration.ZERO;
         }
-        int exponentialFactor = Double.valueOf(Math.pow(2, retryCount - 
1)).intValue();
-        int minDelay = exponentialFactor * (int) 
retryBackoff.getFirstBackoff().toMillis();
-        int maxDelay = Double.valueOf(minDelay + minDelay * 
retryBackoff.getJitterFactor()).intValue();
+        long exponentialFactor = Double.valueOf(Math.pow(2, retryCount - 
1)).longValue();
+        Duration minDelay = 
retryBackoff.getFirstBackoff().multipliedBy(exponentialFactor);
+        Duration jitterDelay = retryBackoff.getFirstBackoff()
+            .multipliedBy(Double.valueOf(retryBackoff.getJitterFactor() * 
100).intValue())
+            .dividedBy(100);
 
-        return Duration.ofMillis(randomBetween(minDelay, maxDelay));
+        return randomBetween(minDelay, jitterDelay);
     }
 
     private boolean shouldDelay(int retryCount) {
diff --git 
a/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/WaitDelayGeneratorTest.java
 
b/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/WaitDelayGeneratorTest.java
index 9cb2d62..e7f1fb6 100644
--- 
a/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/WaitDelayGeneratorTest.java
+++ 
b/mailbox/event/event-rabbitmq/src/test/java/org/apache/james/mailbox/events/WaitDelayGeneratorTest.java
@@ -46,13 +46,13 @@ class WaitDelayGeneratorTest {
 
         SoftAssertions.assertSoftly(softly -> {
             softly.assertThat(generator.generateDelay(1).toMillis())
-                .isBetween(100L, 150L);
+                .isBetween(50L, 150L);
             softly.assertThat(generator.generateDelay(2).toMillis())
-                .isBetween(200L, 300L);
+                .isBetween(100L, 300L);
             softly.assertThat(generator.generateDelay(3).toMillis())
-                .isBetween(400L, 600L);
+                .isBetween(200L, 600L);
             softly.assertThat(generator.generateDelay(4).toMillis())
-                .isBetween(800L, 1200L);
+                .isBetween(300L, 1200L);
         });
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org