Retry cleanups

nakadi-java-client/src/main/java/nakadi/ExponentialRetry.java

@@ -17,8 +17,7 @@ public class ExponentialRetry implements RetryPolicy {
   int maxAttempts;
   long workingAttempts = 1;
   long maxTime;
-  long workingTime = 0L;
-  TimeUnit unit;
+  private long lastBackoff = 0L;
   private long workingInterval;
   private volatile long startTime = 0L;
   private float percentOfMaxIntervalForJitter;
@@ -28,7 +27,6 @@ public class ExponentialRetry implements RetryPolicy {
     this.maxInterval = builder.maxInterval;
     this.workingInterval = initialInterval;
     this.maxAttempts = builder.maxAttempts;
-    this.unit = builder.unit;
     this.maxTime = builder.maxTime;
     this.percentOfMaxIntervalForJitter = builder.percentOfMaxIntervalForJitter;
   }
@@ -45,27 +43,33 @@ public long maxIntervalMillis() {
     return maxInterval;
   }
 
+  long workingTime() {
+    return lastBackoff - startTime;
+  }
+
   public boolean isFinished() {
-    return workingAttempts >= maxAttempts || workingTime >= maxTime;
+    return workingAttempts >= maxAttempts || workingTime() >= maxTime;
   }
 
   public long nextBackoffMillis() {
+    return nextBackOffMillis(System.currentTimeMillis());
+  }
 
+  long nextBackOffMillis(long nowMillis) {
     if (startTime == 0L) {
-      startTime = System.currentTimeMillis();
-    } else {
-      workingTime += (System.currentTimeMillis() - startTime);
+      startTime = nowMillis;
     }
+    lastBackoff = nowMillis;
 
     if (isFinished()) {
       return STOP;
     }
 
-    workingInterval = unit.toMillis(workingInterval) * (workingAttempts * workingAttempts);
+    workingInterval = workingInterval * (workingAttempts * workingAttempts);
     workingAttempts++;
 
     if (workingInterval <= 0) {
-      workingInterval = unit.toMillis(maxInterval);
+      workingInterval = maxInterval;
     }
 
     if (initialInterval != workingInterval) {
@@ -100,13 +104,10 @@ public int maxAttempts() {
         ", maxInterval=" + maxInterval +
         ", maxAttempts=" + maxAttempts +
         ", workingAttempts=" + workingAttempts +
-        ", unit=" + unit +
         '}';
   }
 
   public static class Builder {
-
-    private final TimeUnit unit = TimeUnit.MILLISECONDS;
     public float percentOfMaxIntervalForJitter = PERCENT_OF_MAX_INTERVAL_AS_JITTER;
     private long initialInterval = DEFAULT_INITIAL_INTERVAL_MILLIS;
     private long maxInterval = DEFAULT_MAX_INTERVAL_MILLIS;
@@ -120,7 +121,7 @@ public Builder initialInterval(long initialInterval, TimeUnit unit) {
       NakadiException.throwNonNull(unit, "Please provide a TimeUnit");
       this.initialInterval = unit.toMillis(initialInterval);
       if (this.initialInterval < INITIAL_INTERVAL_MIN_AS_MILLIS) {
-        NakadiException.throwNonNull(null, "Please provide an initial value of at least "
+        throw new IllegalArgumentException("Please provide an initial value of at least "
             + INITIAL_INTERVAL_MIN_AS_MILLIS
             + " millis");
       }
@@ -131,7 +132,7 @@ public Builder maxInterval(long maxInterval, TimeUnit unit) {
       NakadiException.throwNonNull(unit, "Please provide a TimeUnit");
       this.maxInterval = unit.toMillis(maxInterval);
       if (this.maxInterval < MAX_INTERVAL_MIN_AS_MILLIS) {
-        NakadiException.throwNonNull(null, "Please provide a max interval value of at least "
+        throw new IllegalArgumentException("Please provide a max interval value of at least "
             + MAX_INTERVAL_MIN_AS_MILLIS
             + " millis");
       }

nakadi-java-client/src/test/java/nakadi/ExponentialRetryTest.java

@@ -8,81 +8,88 @@
 public class ExponentialRetryTest {
 
   @Test
-  public void tempusFugit() throws Exception {
-
+  public void tempusFugit_1() throws Exception {
     ExponentialRetry exponentialRetry = ExponentialRetry.newBuilder()
         .initialInterval(11, TimeUnit.MILLISECONDS)
         .maxAttempts(Integer.MAX_VALUE)
         .maxInterval(20, TimeUnit.MILLISECONDS)
         .percentOfMaxIntervalForJitter(20)
         .maxTime(3, TimeUnit.SECONDS)
         .build();
+    runRetries(exponentialRetry);
+    validateTimeoutState(exponentialRetry);
+  }
 
-    while(true) {
-      long l = exponentialRetry.nextBackoffMillis();
-      if(l == -1) {
-        break;
-      }
-      Thread.sleep(l);
-    }
-
-    assertTrue(exponentialRetry.workingTime >= exponentialRetry.maxTime);
-    assertTrue(exponentialRetry.workingAttempts < exponentialRetry.maxAttempts);
-
-    exponentialRetry = ExponentialRetry.newBuilder()
+  @Test
+  public void tempusFugit_2() throws Exception {
+    ExponentialRetry exponentialRetry = ExponentialRetry.newBuilder()
         .maxTime(3, TimeUnit.SECONDS)
         .maxInterval(100, TimeUnit.MILLISECONDS)
         .build();
+    runRetries(exponentialRetry);
+    validateTimeoutState(exponentialRetry);
+  }
 
-    while(true) {
-      long l = exponentialRetry.nextBackoffMillis();
-      if(l == -1) {
-        break;
-      }
-      Thread.sleep(l);
-    }
-
-    assertTrue(exponentialRetry.workingTime >= exponentialRetry.maxTime);
+  private void validateTimeoutState(ExponentialRetry exponentialRetry) {
+    assertTrue(exponentialRetry.workingTime() >= exponentialRetry.maxTime);
     assertTrue(exponentialRetry.workingAttempts < exponentialRetry.maxAttempts);
   }
 
   @Test
-  public void annumero() throws Exception {
-
+  public void annumero_1() throws Exception {
     ExponentialRetry exponentialRetry = ExponentialRetry.newBuilder()
         .initialInterval(101, TimeUnit.MILLISECONDS)
         .maxAttempts(20)
         .maxInterval(100, TimeUnit.MILLISECONDS)
         .maxTime(Integer.MAX_VALUE, TimeUnit.SECONDS)
         .build();
+    runRetries(exponentialRetry);
+    validateRetriesExceededState(exponentialRetry);
+  }
 
-    while(true) {
-      long l = exponentialRetry.nextBackoffMillis();
-      if(l == -1) {
-        break;
-      }
-      Thread.sleep(l);
-    }
-
-    assertTrue(exponentialRetry.workingTime < exponentialRetry.maxTime);
-    assertTrue(exponentialRetry.workingAttempts >= exponentialRetry.maxAttempts);
+  @Test
+  public void annumero_2() throws Exception {
+    ExponentialRetry exponentialRetry = ExponentialRetry.newBuilder()
+            .maxAttempts(3)
+            .maxInterval(100, TimeUnit.MILLISECONDS)
+            .build();
+    runRetries(exponentialRetry);
+    validateRetriesExceededState(exponentialRetry);
+  }
 
+  @Test
+  public void workingTimeCalculation() {
+    ExponentialRetry exponentialRetry = ExponentialRetry.newBuilder()
+            .maxInterval(100, TimeUnit.MILLISECONDS)
+            .maxTime(110, TimeUnit.MILLISECONDS)
+            .build();
+    exponentialRetry.nextBackOffMillis(1);
+    assertFalse(exponentialRetry.isFinished());
+    exponentialRetry.nextBackOffMillis(100);
+    assertFalse(exponentialRetry.isFinished());
+    exponentialRetry.nextBackOffMillis(101);
+    assertFalse(exponentialRetry.isFinished());
+    exponentialRetry.nextBackOffMillis(111);
+    assertTrue(exponentialRetry.isFinished());
+  }
 
-    exponentialRetry = ExponentialRetry.newBuilder()
-        .maxAttempts(3)
-        .maxInterval(100, TimeUnit.MILLISECONDS)
-        .build();
+  private void validateRetriesExceededState(ExponentialRetry exponentialRetry) {
+    assertTrue(exponentialRetry.workingTime() < exponentialRetry.maxTime);
+    assertTrue(exponentialRetry.workingAttempts >= exponentialRetry.maxAttempts);
+  }
 
+  private void runRetries(ExponentialRetry exponentialRetry) throws InterruptedException {
     while(true) {
       long l = exponentialRetry.nextBackoffMillis();
-      if(l == -1) {
+      if(l == RetryPolicy.STOP) {
         break;
       }
+      // This does not hold: l >= exponentialRetry.initialInterval()
+      assertTrue(l <= exponentialRetry.maxIntervalMillis());
+
       Thread.sleep(l);
     }
-
-    assertTrue(exponentialRetry.workingTime < exponentialRetry.maxTime);
-    assertTrue(exponentialRetry.workingAttempts >= exponentialRetry.maxAttempts);
+    assertTrue(exponentialRetry.isFinished());
   }
 
-}
+}