diff --git a/nakadi-java-client/src/main/java/nakadi/ExponentialRetry.java b/nakadi-java-client/src/main/java/nakadi/ExponentialRetry.java index 57b03847..b4e900e6 100644 --- a/nakadi-java-client/src/main/java/nakadi/ExponentialRetry.java +++ b/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"); } diff --git a/nakadi-java-client/src/test/java/nakadi/ExponentialRetryTest.java b/nakadi-java-client/src/test/java/nakadi/ExponentialRetryTest.java index 46ebc174..0f808b77 100644 --- a/nakadi-java-client/src/test/java/nakadi/ExponentialRetryTest.java +++ b/nakadi-java-client/src/test/java/nakadi/ExponentialRetryTest.java @@ -8,8 +8,7 @@ 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) @@ -17,72 +16,80 @@ public void tempusFugit() throws Exception { .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()); } -} \ No newline at end of file +}