Skip to content

Commit ed89a50

Browse files
committed
Add duration based maxRetries option to Retry class
1 parent f3d5fb0 commit ed89a50

File tree

2 files changed

+151
-0
lines changed

2 files changed

+151
-0
lines changed

core/src/main/java/org/apache/accumulo/core/util/Retry.java

+53
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,12 @@ public interface NeedsRetries {
272272
* @return this builder with the maximum number of retries set to the provided value
273273
*/
274274
NeedsRetryDelay maxRetries(long max);
275+
276+
/**
277+
* @return this builder with the maximum number of retries set to the number of retries that can
278+
* occur within the given duration
279+
*/
280+
NeedsRetryDelay maxRetriesWithinDuration(Duration duration);
275281
}
276282

277283
public interface NeedsRetryDelay {
@@ -358,6 +364,7 @@ private static class RetryFactoryBuilder
358364
private Duration waitIncrement;
359365
private Duration logInterval;
360366
private double backOffFactor = 1.5;
367+
private Duration retriesForDuration = null;
361368

362369
RetryFactoryBuilder() {}
363370

@@ -381,6 +388,48 @@ public NeedsRetryDelay maxRetries(long max) {
381388
return this;
382389
}
383390

391+
@Override
392+
public NeedsRetryDelay maxRetriesWithinDuration(Duration duration) {
393+
checkState();
394+
Preconditions.checkArgument(!duration.isNegative(),
395+
"Duration for retries must not be negative");
396+
this.retriesForDuration = duration;
397+
return this;
398+
}
399+
400+
/**
401+
* Calculate the maximum number of retries that can occur within {@link #retriesForDuration}
402+
*/
403+
private void calculateRetriesWithinDuration() {
404+
long numberOfRetries = 0;
405+
long cumulativeWaitTimeMillis = 0;
406+
long currentWaitTimeMillis = initialWait.toMillis();
407+
long retriesForDurationMillis = retriesForDuration.toMillis();
408+
409+
while (cumulativeWaitTimeMillis + currentWaitTimeMillis <= retriesForDurationMillis) {
410+
411+
cumulativeWaitTimeMillis += currentWaitTimeMillis;
412+
numberOfRetries++;
413+
414+
if (backOffFactor > 1.0) {
415+
currentWaitTimeMillis = (long) Math.ceil(currentWaitTimeMillis * backOffFactor);
416+
} else {
417+
currentWaitTimeMillis += waitIncrement.toMillis();
418+
}
419+
420+
if (currentWaitTimeMillis > maxWait.toMillis()) {
421+
currentWaitTimeMillis = maxWait.toMillis(); // Ensure wait time does not exceed maxWait
422+
}
423+
424+
// prevent an infinite loop
425+
if (numberOfRetries >= Integer.MAX_VALUE) {
426+
break;
427+
}
428+
}
429+
430+
this.maxRetries = numberOfRetries;
431+
}
432+
384433
@Override
385434
public NeedsTimeIncrement retryAfter(Duration duration) {
386435
checkState();
@@ -434,6 +483,10 @@ public RetryFactory createFactory() {
434483

435484
@Override
436485
public Retry createRetry() {
486+
if (retriesForDuration != null) {
487+
calculateRetriesWithinDuration();
488+
}
489+
this.modifiable = false;
437490
return new Retry(maxRetries, initialWait, waitIncrement, maxWait, logInterval, backOffFactor);
438491
}
439492

core/src/test/java/org/apache/accumulo/core/util/RetryTest.java

+98
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.accumulo.core.util.Retry.RetryFactory;
3434
import org.easymock.EasyMock;
3535
import org.junit.jupiter.api.BeforeEach;
36+
import org.junit.jupiter.api.Nested;
3637
import org.junit.jupiter.api.Test;
3738
import org.slf4j.Logger;
3839
import org.slf4j.LoggerFactory;
@@ -351,4 +352,101 @@ public void testInfiniteRetryWithBackoff() throws InterruptedException {
351352
}
352353
}
353354
}
355+
356+
@Nested
357+
public class MaxRetriesWithinDuration {
358+
359+
@Test
360+
public void noIncrement() {
361+
Duration retriesForDuration = Duration.ofSeconds(3);
362+
Duration retryAfter = Duration.ofMillis(100);
363+
Retry retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration)
364+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(1000))
365+
.backOffFactor(1.0).logInterval(Duration.ofMinutes(3)).createRetry();
366+
367+
// with no increment, the number of retries should be the duration divided by the retryAfter
368+
// (which is used as the initial wait and in this case does not change)
369+
long expectedRetries = retriesForDuration.dividedBy(retryAfter);
370+
assertEquals(expectedRetries, retry.getMaxRetries());
371+
372+
// try again with lots of expected retries
373+
retriesForDuration = Duration.ofSeconds(30);
374+
retryAfter = Duration.ofMillis(10);
375+
retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration).retryAfter(retryAfter)
376+
.incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(1000)).backOffFactor(1.0)
377+
.logInterval(Duration.ofMinutes(3)).createRetry();
378+
379+
expectedRetries = retriesForDuration.dividedBy(retryAfter);
380+
assertEquals(expectedRetries, retry.getMaxRetries());
381+
}
382+
383+
@Test
384+
public void withIncrement() {
385+
final Duration retriesForDuration = Duration.ofMillis(1500);
386+
final Duration retryAfter = Duration.ofMillis(100);
387+
final Duration increment = Duration.ofMillis(100);
388+
389+
Retry retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration)
390+
.retryAfter(retryAfter).incrementBy(increment).maxWait(Duration.ofMillis(1000))
391+
.backOffFactor(1.0).logInterval(Duration.ofMinutes(3)).createRetry();
392+
393+
// the max retries should be calculated like this:
394+
// 1. 100
395+
// 2. 100 + 100 = 200
396+
// 3. 200 + 100 = 300
397+
// 4. 300 + 100 = 400
398+
// 5. 400 + 100 = 500
399+
400+
// 100 + 200 + 300 + 400 + 500 = 1500
401+
402+
assertEquals(5, retry.getMaxRetries());
403+
}
404+
405+
@Test
406+
public void withBackoffFactorAndMaxWait() {
407+
final Duration retriesForDuration = Duration.ofSeconds(4);
408+
final Duration retryAfter = Duration.ofMillis(100);
409+
Retry retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration)
410+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
411+
.backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
412+
413+
// max retries should be calculated like this:
414+
// 1. 100
415+
// 2. 100 * 1.5 = 150
416+
// 3. 150 * 1.5 = 225
417+
// 4. 225 * 1.5 = 337
418+
// 5. 337 * 1.5 = 505 (which is greater than the max wait of 500 so its capped)
419+
420+
// 100 + 150 + 225 + 337 + 500 + 500 + 500 + 500 + 500 + 500 = 3812
421+
assertEquals(10, retry.getMaxRetries());
422+
}
423+
424+
@Test
425+
public void smallDuration() {
426+
Duration retriesForDuration = Duration.ofMillis(0);
427+
final Duration retryAfter = Duration.ofMillis(100);
428+
Retry retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration)
429+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
430+
.backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
431+
assertEquals(0, retry.getMaxRetries());
432+
433+
retriesForDuration = Duration.ofMillis(99);
434+
assertTrue(retriesForDuration.compareTo(retryAfter) < 0);
435+
retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration).retryAfter(retryAfter)
436+
.incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500)).backOffFactor(1.5)
437+
.logInterval(Duration.ofMinutes(3)).createRetry();
438+
assertEquals(0, retry.getMaxRetries());
439+
}
440+
441+
@Test
442+
public void equalDurationAndInitialWait() {
443+
final Duration retriesForDuration = Duration.ofMillis(100);
444+
final Duration retryAfter = Duration.ofMillis(100);
445+
assertEquals(0, retriesForDuration.compareTo(retryAfter));
446+
Retry retry = Retry.builder().maxRetriesWithinDuration(retriesForDuration)
447+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
448+
.backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
449+
assertEquals(1, retry.getMaxRetries());
450+
}
451+
}
354452
}

0 commit comments

Comments
 (0)