This is an automated email from the ASF dual-hosted git repository.
domgarguilo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 5d741657e7 Add duration based maxRetries option to Retry class (#4337)
5d741657e7 is described below
commit 5d741657e7d3f003e5c817963318e47c271f8e45
Author: Dom G <domgargu...@apache.org>
AuthorDate: Fri Mar 22 08:38:41 2024 -0400
Add duration based maxRetries option to Retry class (#4337)
* Add duration based maxRetries option to Retry class
---
.../java/org/apache/accumulo/core/util/Retry.java | 53 ++++++++++++
.../org/apache/accumulo/core/util/RetryTest.java | 98 ++++++++++++++++++++++
2 files changed, 151 insertions(+)
diff --git a/core/src/main/java/org/apache/accumulo/core/util/Retry.java
b/core/src/main/java/org/apache/accumulo/core/util/Retry.java
index 28659b376f..916537db3e 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/Retry.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/Retry.java
@@ -272,6 +272,12 @@ public class Retry {
* @return this builder with the maximum number of retries set to the
provided value
*/
NeedsRetryDelay maxRetries(long max);
+
+ /**
+ * @return this builder with the maximum number of retries set to the
number of retries that can
+ * occur within the given duration
+ */
+ NeedsRetryDelay maxRetriesWithinDuration(Duration duration);
}
public interface NeedsRetryDelay {
@@ -358,6 +364,7 @@ public class Retry {
private Duration waitIncrement;
private Duration logInterval;
private double backOffFactor = 1.5;
+ private Duration retriesForDuration = null;
RetryFactoryBuilder() {}
@@ -381,6 +388,48 @@ public class Retry {
return this;
}
+ @Override
+ public NeedsRetryDelay maxRetriesWithinDuration(Duration duration) {
+ checkState();
+ Preconditions.checkArgument(!duration.isNegative(),
+ "Duration for retries must not be negative");
+ this.retriesForDuration = duration;
+ return this;
+ }
+
+ /**
+ * Calculate the maximum number of retries that can occur within {@link
#retriesForDuration}
+ */
+ private void calculateRetriesWithinDuration() {
+ long numberOfRetries = 0;
+ long cumulativeWaitTimeMillis = 0;
+ long currentWaitTimeMillis = initialWait.toMillis();
+ final long retriesForDurationMillis = retriesForDuration.toMillis();
+
+ // set an upper bound for the number of retries
+ final long maxRetries = Duration.ofHours(1).toMillis();
+
+ while (cumulativeWaitTimeMillis + currentWaitTimeMillis <=
retriesForDurationMillis
+ && numberOfRetries < maxRetries) {
+
+ cumulativeWaitTimeMillis += currentWaitTimeMillis;
+ numberOfRetries++;
+
+ if (backOffFactor > 1.0) {
+ currentWaitTimeMillis = (long) Math.ceil(currentWaitTimeMillis *
backOffFactor);
+ } else {
+ currentWaitTimeMillis += waitIncrement.toMillis();
+ }
+
+ if (currentWaitTimeMillis > maxWait.toMillis()) {
+ currentWaitTimeMillis = maxWait.toMillis(); // Ensure wait time does
not exceed maxWait
+ }
+
+ }
+
+ this.maxRetries = numberOfRetries;
+ }
+
@Override
public NeedsTimeIncrement retryAfter(Duration duration) {
checkState();
@@ -434,6 +483,10 @@ public class Retry {
@Override
public Retry createRetry() {
+ if (retriesForDuration != null) {
+ calculateRetriesWithinDuration();
+ }
+ this.modifiable = false;
return new Retry(maxRetries, initialWait, waitIncrement, maxWait,
logInterval, backOffFactor);
}
diff --git a/core/src/test/java/org/apache/accumulo/core/util/RetryTest.java
b/core/src/test/java/org/apache/accumulo/core/util/RetryTest.java
index 45992ec9b5..c05189a514 100644
--- a/core/src/test/java/org/apache/accumulo/core/util/RetryTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/util/RetryTest.java
@@ -33,6 +33,7 @@ import org.apache.accumulo.core.util.Retry.NeedsTimeIncrement;
import org.apache.accumulo.core.util.Retry.RetryFactory;
import org.easymock.EasyMock;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -351,4 +352,101 @@ public class RetryTest {
}
}
}
+
+ @Nested
+ public class MaxRetriesWithinDuration {
+
+ @Test
+ public void noIncrement() {
+ Duration retriesForDuration = Duration.ofSeconds(3);
+ Duration retryAfter = Duration.ofMillis(100);
+ Retry retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration)
+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(1000))
+ .backOffFactor(1.0).logInterval(Duration.ofMinutes(3)).createRetry();
+
+ // with no increment, the number of retries should be the duration
divided by the retryAfter
+ // (which is used as the initial wait and in this case does not change)
+ long expectedRetries = retriesForDuration.dividedBy(retryAfter);
+ assertEquals(expectedRetries, retry.getMaxRetries());
+
+ // try again with lots of expected retries
+ retriesForDuration = Duration.ofSeconds(30);
+ retryAfter = Duration.ofMillis(10);
+ retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration).retryAfter(retryAfter)
+
.incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(1000)).backOffFactor(1.0)
+ .logInterval(Duration.ofMinutes(3)).createRetry();
+
+ expectedRetries = retriesForDuration.dividedBy(retryAfter);
+ assertEquals(expectedRetries, retry.getMaxRetries());
+ }
+
+ @Test
+ public void withIncrement() {
+ final Duration retriesForDuration = Duration.ofMillis(1500);
+ final Duration retryAfter = Duration.ofMillis(100);
+ final Duration increment = Duration.ofMillis(100);
+
+ Retry retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration)
+
.retryAfter(retryAfter).incrementBy(increment).maxWait(Duration.ofMillis(1000))
+ .backOffFactor(1.0).logInterval(Duration.ofMinutes(3)).createRetry();
+
+ // the max retries should be calculated like this:
+ // 1. 100
+ // 2. 100 + 100 = 200
+ // 3. 200 + 100 = 300
+ // 4. 300 + 100 = 400
+ // 5. 400 + 100 = 500
+
+ // 100 + 200 + 300 + 400 + 500 = 1500
+
+ assertEquals(5, retry.getMaxRetries());
+ }
+
+ @Test
+ public void withBackoffFactorAndMaxWait() {
+ final Duration retriesForDuration = Duration.ofSeconds(4);
+ final Duration retryAfter = Duration.ofMillis(100);
+ Retry retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration)
+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
+ .backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
+
+ // max retries should be calculated like this:
+ // 1. 100
+ // 2. 100 * 1.5 = 150
+ // 3. 150 * 1.5 = 225
+ // 4. 225 * 1.5 = 337
+ // 5. 337 * 1.5 = 505 (which is greater than the max wait of 500 so its
capped)
+
+ // 100 + 150 + 225 + 337 + 500 + 500 + 500 + 500 + 500 + 500 = 3812
+ assertEquals(10, retry.getMaxRetries());
+ }
+
+ @Test
+ public void smallDuration() {
+ Duration retriesForDuration = Duration.ofMillis(0);
+ final Duration retryAfter = Duration.ofMillis(100);
+ Retry retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration)
+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
+ .backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
+ assertEquals(0, retry.getMaxRetries());
+
+ retriesForDuration = Duration.ofMillis(99);
+ assertTrue(retriesForDuration.compareTo(retryAfter) < 0);
+ retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration).retryAfter(retryAfter)
+
.incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500)).backOffFactor(1.5)
+ .logInterval(Duration.ofMinutes(3)).createRetry();
+ assertEquals(0, retry.getMaxRetries());
+ }
+
+ @Test
+ public void equalDurationAndInitialWait() {
+ final Duration retriesForDuration = Duration.ofMillis(100);
+ final Duration retryAfter = Duration.ofMillis(100);
+ assertEquals(0, retriesForDuration.compareTo(retryAfter));
+ Retry retry =
Retry.builder().maxRetriesWithinDuration(retriesForDuration)
+
.retryAfter(retryAfter).incrementBy(Duration.ofMillis(0)).maxWait(Duration.ofMillis(500))
+ .backOffFactor(1.5).logInterval(Duration.ofMinutes(3)).createRetry();
+ assertEquals(1, retry.getMaxRetries());
+ }
+ }
}
