Updated CappedExponentialDelay timeout logic (#424)

Author: shrsrivz

src/main/java/software/amazon/cloudformation/proxy/delay/CappedExponential.java

@@ -16,27 +16,25 @@
 
 import com.google.common.base.Preconditions;
 import java.time.Duration;
-import java.util.concurrent.TimeUnit;
 
 public class CappedExponential extends MinDelayAbstractBase {
 
     final double powerBy;
 
-    final long startTimeInMillis;
-
     final Duration maxDelay;
 
+    private Duration accrued = Duration.ZERO;
+
     CappedExponential(Duration timeout,
                       Duration minDelay,
                       Double powerBy,
                       Duration maxDelay) {
         super(timeout, minDelay);
         Preconditions.checkArgument(powerBy >= 1.0, "powerBy >= 1.0");
-        Preconditions.checkArgument(maxDelay != null && maxDelay.toMillis() >= 0, "maxDelay must be > 0");
+        Preconditions.checkArgument(maxDelay != null && maxDelay.toMillis() > 0, "maxDelay must be > 0");
         Preconditions.checkArgument(maxDelay.compareTo(minDelay) >= 0, "maxDelay.compareTo(minDelay) >= 0");
         this.powerBy = powerBy;
         this.maxDelay = maxDelay;
-        this.startTimeInMillis = System.currentTimeMillis();
     }
 
     public static Builder of() {
@@ -47,6 +45,8 @@ public static final class Builder extends MinDelayBasedBuilder<CappedExponential
         private double powerBy = 2;
         private Duration maxDelay = Duration.ofSeconds(20);
 
+        private Duration minDelay = Duration.ofSeconds(1);
+
         public CappedExponential.Builder powerBy(Double powerBy) {
             this.powerBy = powerBy;
             return this;
@@ -57,6 +57,11 @@ public CappedExponential.Builder maxDelay(Duration maxDelay) {
             return this;
         }
 
+        public CappedExponential.Builder minDelay(Duration minDelay) {
+            this.minDelay = minDelay;
+            return this;
+        }
+
         @Override
         public CappedExponential build() {
             return new CappedExponential(timeout, minDelay, powerBy, maxDelay);
@@ -65,14 +70,17 @@ public CappedExponential build() {
 
     @Override
     public Duration nextDelay(int attempt) {
-        if (attempt == 0) {
-            return minDelay;
-        }
-        if (System.currentTimeMillis() - startTimeInMillis > timeout.toMillis()) {
-            return Duration.ZERO;
-        }
-        long next = Math.round(minDelay.toMillis() * Math.pow(powerBy, attempt - 1));
-        return Duration.ofSeconds(Math.min(maxDelay.getSeconds(), TimeUnit.MILLISECONDS.toSeconds(next)));
+        Duration next = Duration.ofSeconds(Math.round(Math.pow(powerBy, attempt)));
+        Duration nextDelay = Duration.ofSeconds(Math.min(maxDelay.getSeconds(), next.getSeconds()));
+        accrued = accrued.plus(nextDelay);
+        return enforceBounds(accrued, nextDelay);
+
+    }
+
+    @Override
+    public String toString() {
+        return "CappedExponential{" + "powerBy=" + powerBy + ", maxDelay=" + maxDelay + ", accrued=" + accrued + ", minDelay="
+            + minDelay + ", timeout=" + timeout + '}';
     }
 
 }

src/test/java/software/amazon/cloudformation/proxy/DelayTest.java

@@ -205,30 +205,19 @@ public void cappedExponentialDelays() {
         Duration MAX_DELAY = Duration.ofSeconds(15);
         final Delay cappedExponential = CappedExponential.of().timeout(Duration.ofMinutes(20)).maxDelay(MAX_DELAY).powerBy(1.3)
             .minDelay(Duration.ofSeconds(1)).build();
-        int[] results = { 1, 1, 1, 1, 2, 2, 3, 4, 6, 8, 10, 13, 15, 15, 15, 15 };
+        int[] results = { 1, 1, 2, 2, 3, 4, 5, 6, 8, 11, 14, 15, 15, 15, 15, 15, 15 };
         for (int tries = 0; tries <= 15; tries++) {
             Duration delay = cappedExponential.nextDelay(tries);
             assertThat(results[tries]).isEqualTo((int) delay.getSeconds());
-            if (tries >= 12) {
+            if (tries >= 11) {
                 assertThat(MAX_DELAY.getSeconds()).isEqualTo(delay.getSeconds());
             }
         }
 
-        // If minDelay is not set, the retry is without delay.
-        final Delay cappedExponentialNoDelay = CappedExponential.of().timeout(Duration.ofSeconds(12)).build();
-        for (int tries = 0; tries <= 15; tries++) {
-            Duration delay = cappedExponentialNoDelay.nextDelay(tries);
-            assertThat(0).isEqualTo((int) delay.getSeconds());
-            if (tries >= 12) {
-                assertThat(0).isEqualTo(delay.getSeconds());
-            }
-        }
-
         // If powerBy is not passed, it's set to default 2.
         final Delay cappedExponentialNoPower = CappedExponential.of().timeout(Duration.ofMinutes(20)).maxDelay(MAX_DELAY)
-            .minDelay(Duration.ofSeconds(1)).build();
-
-        int[] resultsNoPower = { 1, 1, 2, 4, 8, 15, 15, 15, 15, 15 };
+            .minDelay(Duration.ofSeconds(2)).build();
+        int[] resultsNoPower = { 2, 2, 4, 8, 15, 15, 15, 15, 15 };
         for (int tries = 0; tries <= 6; tries++) {
             Duration delay = cappedExponentialNoPower.nextDelay(tries);
             assertThat(resultsNoPower[tries]).isEqualTo((int) delay.getSeconds());
@@ -237,5 +226,46 @@ public void cappedExponentialDelays() {
             }
         }
 
+        // If timeout is reached the delay is 0
+        final Delay cappedExponentialTimeout = CappedExponential.of().timeout(Duration.ofSeconds(5))
+            .maxDelay(Duration.ofSeconds(1)).powerBy(1.0).minDelay(Duration.ofSeconds(1)).build();
+
+        int[] resultsTimeout = { 1, 1, 1, 1, 1, 0 };
+        for (int tries = 0; tries <= 5; tries++) {
+            Duration delay = cappedExponentialTimeout.nextDelay(tries);
+            assertThat(resultsTimeout[tries]).isEqualTo((int) delay.getSeconds());
+            if (tries >= 5) {
+                assertThat(0).isEqualTo(delay.getSeconds());
+            }
+        }
+
+        // If minDelay is not passed, it's set to default 1.
+        final Delay cappedExponentialNoMinDelay = CappedExponential.of().timeout(Duration.ofSeconds(5))
+            .maxDelay(Duration.ofSeconds(1)).powerBy(1.0).build();
+        int[] resultsNoMinDelay = { 1, 1, 1, 1, 1, 0 };
+        for (int tries = 0; tries <= 5; tries++) {
+            Duration delay = cappedExponentialNoMinDelay.nextDelay(tries);
+            assertThat(resultsNoMinDelay[tries]).isEqualTo((int) delay.getSeconds());
+        }
+
+        // If maxDelay is not passed, it's set to default 20 sec.
+        final Delay cappedExponentialNoMaxDelay = CappedExponential.of().timeout(Duration.ofMinutes(20))
+            .minDelay(Duration.ofSeconds(2)).build();
+        int[] resultsNoMaxDelay = { 2, 2, 4, 8, 16, 20, 20, 20, 20 };
+        for (int tries = 0; tries <= 6; tries++) {
+            Duration delay = cappedExponentialNoMaxDelay.nextDelay(tries);
+            assertThat(resultsNoMaxDelay[tries]).isEqualTo((int) delay.getSeconds());
+        }
+
+        final Delay cappedExponentialSameMinMaxDelay = CappedExponential.of().timeout(Duration.ofSeconds(5))
+            .maxDelay(Duration.ofSeconds(1)).powerBy(1.3).minDelay(Duration.ofSeconds(1)).build();
+        int[] resultsSameMinMaxDelay = { 1, 1, 1, 1, 1, 0 };
+        for (int tries = 0; tries <= 5; tries++) {
+            Duration delay = cappedExponentialSameMinMaxDelay.nextDelay(tries);
+            assertThat(resultsSameMinMaxDelay[tries]).isEqualTo((int) delay.getSeconds());
+            if (tries >= 5) {
+                assertThat(0).isEqualTo(delay.getSeconds());
+            }
+        }
     }
 }