From 3f427faf3b2118db0e9ab78f149426d63ae94927 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Jan 2024 16:38:08 +0000 Subject: [PATCH 1/6] Add retry & error policies --- .../main/java/tech/ydb/core/ErrorPolicy.java | 30 +++++++++++++++++++ .../main/java/tech/ydb/core/RetryPolicy.java | 20 +++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 core/src/main/java/tech/ydb/core/ErrorPolicy.java create mode 100644 core/src/main/java/tech/ydb/core/RetryPolicy.java diff --git a/core/src/main/java/tech/ydb/core/ErrorPolicy.java b/core/src/main/java/tech/ydb/core/ErrorPolicy.java new file mode 100644 index 000000000..d80c7a940 --- /dev/null +++ b/core/src/main/java/tech/ydb/core/ErrorPolicy.java @@ -0,0 +1,30 @@ +package tech.ydb.core; + +/** + * Recipes should use the configured error policy to decide how to retry + * errors like unsuccessful {@link tech.ydb.core.StatusCode}. + * + * @author Aleksandr Gorshenin + * @param Type of errors to check + */ +public interface ErrorPolicy { + + /** + * Returns true if the given value should be retried + * + * @param value value to check + * @return true if value is retryable + */ + boolean isRetryable(T value); + + /** + * Returns true if the given exception should be retried + * Usually exceptions are never retried, but some policies can implement more difficult logic + * + * @param ex exception to check + * @return true if exception is retryable + */ + default boolean isRetryable(Exception ex) { + return false; + } +} diff --git a/core/src/main/java/tech/ydb/core/RetryPolicy.java b/core/src/main/java/tech/ydb/core/RetryPolicy.java new file mode 100644 index 000000000..9546eb035 --- /dev/null +++ b/core/src/main/java/tech/ydb/core/RetryPolicy.java @@ -0,0 +1,20 @@ +package tech.ydb.core; + +/** + * Abstracts the policy to use when retrying some actions + * + * @author Aleksandr Gorshenin + */ +public interface RetryPolicy { + /** + * Called when an operation has failed for some reason. This method may return next values + * positive number N : operation must be retried in N milliseconds + * zero : operation must be retried immediately + * negative number : retry is not allowed, operation must be failed + * + * @param retryCount the number of times retried so far (0 the first time) + * @param elapsedTimeMs the elapsed time in ms since the operation was attempted + * @return number of milliseconds for next retry + */ + long nextRetryMs(int retryCount, long elapsedTimeMs); +} From 91be48ca8dc7054ebe04cba37ad31ab889ec753f Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Jan 2024 16:38:40 +0000 Subject: [PATCH 2/6] Add base implementation of RetryPolicy --- .../core/retry/ExponentialBackoffRetry.java | 36 +++++++++++++++++++ .../tech/ydb/core/retry/ForeverRetry.java | 20 +++++++++++ .../tech/ydb/core/retry/MaxElapsedRetry.java | 25 +++++++++++++ .../tech/ydb/core/retry/MaxRetriesRetry.java | 24 +++++++++++++ 4 files changed, 105 insertions(+) create mode 100644 core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java create mode 100644 core/src/main/java/tech/ydb/core/retry/ForeverRetry.java create mode 100644 core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java create mode 100644 core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java diff --git a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java new file mode 100644 index 000000000..2b44e755d --- /dev/null +++ b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java @@ -0,0 +1,36 @@ +package tech.ydb.core.retry; + +import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; + +import com.google.common.annotations.VisibleForTesting; + +import tech.ydb.core.RetryPolicy; + +/** + * + * @author Aleksandr Gorshenin + */ +public abstract class ExponentialBackoffRetry implements RetryPolicy { + private final int backoffMs; + private final int backoffCeiling; + + protected ExponentialBackoffRetry(int backoffMs, int backoffCeiling) { + this.backoffMs = backoffMs; + this.backoffCeiling = backoffCeiling; + } + + protected long backoffTimeMillis(int retryNumber, Random random) { + int slots = 1 << Math.min(retryNumber, backoffCeiling); + int delay = backoffMs * slots; + return delay + random.nextInt(delay); + } + + @Override + public long nextRetryMs(int retryCount, long elapsedTimeMs) { + return nextRetryMs(retryCount, elapsedTimeMs, ThreadLocalRandom.current()); + } + + @VisibleForTesting + abstract long nextRetryMs(int retryCount, long elapsedTimeMs, Random random); +} diff --git a/core/src/main/java/tech/ydb/core/retry/ForeverRetry.java b/core/src/main/java/tech/ydb/core/retry/ForeverRetry.java new file mode 100644 index 000000000..5f5a0c78f --- /dev/null +++ b/core/src/main/java/tech/ydb/core/retry/ForeverRetry.java @@ -0,0 +1,20 @@ +package tech.ydb.core.retry; + +import tech.ydb.core.RetryPolicy; + +/** + * + * @author Aleksandr Gorshenin + */ +public class ForeverRetry implements RetryPolicy { + private final long intervalMs; + + public ForeverRetry(long intervalMs) { + this.intervalMs = intervalMs; + } + + @Override + public long nextRetryMs(int retryCount, long elapsedTimeMs) { + return intervalMs; + } +} diff --git a/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java b/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java new file mode 100644 index 000000000..768f77af1 --- /dev/null +++ b/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java @@ -0,0 +1,25 @@ +package tech.ydb.core.retry; + +import java.util.Random; + +/** + * + * @author Aleksandr Gorshenin + */ +public class MaxElapsedRetry extends ExponentialBackoffRetry { + private final long maxElapsedMs; + + public MaxElapsedRetry(long maxElapsedMs, int backoffMs, int backoffCeiling) { + super(backoffMs, backoffCeiling); + this.maxElapsedMs = maxElapsedMs; + } + + @Override + long nextRetryMs(int retryCount, long elapsedTimeMs, Random random) { + if (elapsedTimeMs > maxElapsedMs) { + return -1; + } + long backoff = backoffTimeMillis(retryCount, random); + return (elapsedTimeMs + backoff < maxElapsedMs) ? backoff : maxElapsedMs - elapsedTimeMs; + } +} diff --git a/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java b/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java new file mode 100644 index 000000000..84786ad4f --- /dev/null +++ b/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java @@ -0,0 +1,24 @@ +package tech.ydb.core.retry; + +import java.util.Random; + +/** + * + * @author Aleksandr Gorshenin + */ +public class MaxRetriesRetry extends ExponentialBackoffRetry { + private final int maxRetries; + + public MaxRetriesRetry(int maxRetries, int backoffMs, int backoffCeiling) { + super(backoffMs, backoffCeiling); + this.maxRetries = maxRetries; + } + + @Override + long nextRetryMs(int retryCount, long elapsedTimeMs, Random random) { + if (retryCount < maxRetries) { + return -1; + } + return backoffTimeMillis(retryCount, random); + } +} From 104f7ee1780b7b1b0cc232d7a55f67e757f9f4ae Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Fri, 19 Jan 2024 18:34:58 +0000 Subject: [PATCH 3/6] Added test for RetryPolicy --- .../main/java/tech/ydb/core/RetryPolicy.java | 8 +- .../core/retry/ExponentialBackoffRetry.java | 21 ++--- .../tech/ydb/core/retry/MaxElapsedRetry.java | 10 +-- .../tech/ydb/core/retry/MaxRetriesRetry.java | 10 +-- .../ydb/core/retry/RetryPoliciesTest.java | 76 +++++++++++++++++++ 5 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java diff --git a/core/src/main/java/tech/ydb/core/RetryPolicy.java b/core/src/main/java/tech/ydb/core/RetryPolicy.java index 9546eb035..e921e8e81 100644 --- a/core/src/main/java/tech/ydb/core/RetryPolicy.java +++ b/core/src/main/java/tech/ydb/core/RetryPolicy.java @@ -8,9 +8,11 @@ public interface RetryPolicy { /** * Called when an operation has failed for some reason. This method may return next values - * positive number N : operation must be retried in N milliseconds - * zero : operation must be retried immediately - * negative number : retry is not allowed, operation must be failed + *
    + *
  • Positive number N - operation must be retried in N milliseconds
    • + *
  • Zero : operation must be retried immediately
    • + *
  • Negative number : retry is not allowed, operation must be failed
    • + *
* * @param retryCount the number of times retried so far (0 the first time) * @param elapsedTimeMs the elapsed time in ms since the operation was attempted diff --git a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java index 2b44e755d..b0e0a9e53 100644 --- a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java @@ -1,10 +1,7 @@ package tech.ydb.core.retry; -import java.util.Random; import java.util.concurrent.ThreadLocalRandom; -import com.google.common.annotations.VisibleForTesting; - import tech.ydb.core.RetryPolicy; /** @@ -12,25 +9,17 @@ * @author Aleksandr Gorshenin */ public abstract class ExponentialBackoffRetry implements RetryPolicy { - private final int backoffMs; + private final long backoffMs; private final int backoffCeiling; - protected ExponentialBackoffRetry(int backoffMs, int backoffCeiling) { + protected ExponentialBackoffRetry(long backoffMs, int backoffCeiling) { this.backoffMs = backoffMs; this.backoffCeiling = backoffCeiling; } - protected long backoffTimeMillis(int retryNumber, Random random) { + protected long backoffTimeMillis(int retryNumber) { int slots = 1 << Math.min(retryNumber, backoffCeiling); - int delay = backoffMs * slots; - return delay + random.nextInt(delay); - } - - @Override - public long nextRetryMs(int retryCount, long elapsedTimeMs) { - return nextRetryMs(retryCount, elapsedTimeMs, ThreadLocalRandom.current()); + long delay = backoffMs * slots; + return delay + ThreadLocalRandom.current().nextLong(delay); } - - @VisibleForTesting - abstract long nextRetryMs(int retryCount, long elapsedTimeMs, Random random); } diff --git a/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java b/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java index 768f77af1..c568aaf74 100644 --- a/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java @@ -1,7 +1,5 @@ package tech.ydb.core.retry; -import java.util.Random; - /** * * @author Aleksandr Gorshenin @@ -9,17 +7,17 @@ public class MaxElapsedRetry extends ExponentialBackoffRetry { private final long maxElapsedMs; - public MaxElapsedRetry(long maxElapsedMs, int backoffMs, int backoffCeiling) { + public MaxElapsedRetry(long maxElapsedMs, long backoffMs, int backoffCeiling) { super(backoffMs, backoffCeiling); this.maxElapsedMs = maxElapsedMs; } @Override - long nextRetryMs(int retryCount, long elapsedTimeMs, Random random) { - if (elapsedTimeMs > maxElapsedMs) { + public long nextRetryMs(int retryCount, long elapsedTimeMs) { + if (maxElapsedMs <= elapsedTimeMs) { return -1; } - long backoff = backoffTimeMillis(retryCount, random); + long backoff = backoffTimeMillis(retryCount); return (elapsedTimeMs + backoff < maxElapsedMs) ? backoff : maxElapsedMs - elapsedTimeMs; } } diff --git a/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java b/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java index 84786ad4f..ef76e002d 100644 --- a/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java @@ -1,7 +1,5 @@ package tech.ydb.core.retry; -import java.util.Random; - /** * * @author Aleksandr Gorshenin @@ -9,16 +7,16 @@ public class MaxRetriesRetry extends ExponentialBackoffRetry { private final int maxRetries; - public MaxRetriesRetry(int maxRetries, int backoffMs, int backoffCeiling) { + public MaxRetriesRetry(int maxRetries, long backoffMs, int backoffCeiling) { super(backoffMs, backoffCeiling); this.maxRetries = maxRetries; } @Override - long nextRetryMs(int retryCount, long elapsedTimeMs, Random random) { - if (retryCount < maxRetries) { + public long nextRetryMs(int retryCount, long elapsedTimeMs) { + if (retryCount >= maxRetries) { return -1; } - return backoffTimeMillis(retryCount, random); + return backoffTimeMillis(retryCount); } } diff --git a/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java new file mode 100644 index 000000000..3a58cecf3 --- /dev/null +++ b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java @@ -0,0 +1,76 @@ +package tech.ydb.core.retry; + +import org.junit.Assert; +import org.junit.Test; + +import tech.ydb.core.RetryPolicy; + +/** + * + * @author Aleksandr Gorshenin + */ +public class RetryPoliciesTest { + + @Test + public void foreverRetryTest() { + RetryPolicy policy = new ForeverRetry(1234); + + Assert.assertEquals(1234, policy.nextRetryMs(0, 0)); + Assert.assertEquals(1234, policy.nextRetryMs(Integer.MAX_VALUE, 0)); + Assert.assertEquals(1234, policy.nextRetryMs(0, Integer.MAX_VALUE)); + Assert.assertEquals(1234, policy.nextRetryMs(Integer.MAX_VALUE, Integer.MAX_VALUE)); + } + + @Test + public void zeroRetriesTest() { + RetryPolicy policy = new MaxRetriesRetry(0, 100, 3); + + Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(0, Integer.MAX_VALUE)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, Integer.MAX_VALUE)); + } + + @Test + public void zeroTimeoutTest() { + RetryPolicy policy = new MaxElapsedRetry(0, 100, 3); + + Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(0, Integer.MAX_VALUE)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, Integer.MAX_VALUE)); + } + + @Test + public void maxRetriesTest() { + RetryPolicy policy = new MaxRetriesRetry(5, 100, 3); + + assertDuration(100, 200, policy.nextRetryMs(0, 0)); + assertDuration(200, 400, policy.nextRetryMs(1, 150)); + assertDuration(400, 800, policy.nextRetryMs(2, 400)); + assertDuration(800, 1600, policy.nextRetryMs(3, 1600)); + assertDuration(800, 1600, policy.nextRetryMs(4, 2800)); + + Assert.assertEquals(-1, policy.nextRetryMs(5, 4000)); + } + + @Test + public void maxTimeoutTest() { + RetryPolicy policy = new MaxElapsedRetry(2500, 50, 3); + + assertDuration(50, 100, policy.nextRetryMs(0, 0)); + assertDuration(100, 200, policy.nextRetryMs(1, 75)); + assertDuration(200, 400, policy.nextRetryMs(2, 225)); + assertDuration(400, 800, policy.nextRetryMs(3, 525)); + assertDuration(400, 800, policy.nextRetryMs(4, 1125)); + assertDuration(400, 800, policy.nextRetryMs(5, 1725)); + + Assert.assertEquals(175, policy.nextRetryMs(6, 2325)); + Assert.assertEquals(-1, policy.nextRetryMs(7, 2500)); + } + + private void assertDuration(long from, long to, long ms) { + Assert.assertTrue(from <= ms); + Assert.assertTrue(to >= ms); + } +} From be6840b1d9701b1a830c4f12d49d8e48b0789a4d Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 22 Jan 2024 11:57:49 +0000 Subject: [PATCH 4/6] Rename retry policies --- core/src/main/java/tech/ydb/core/RetryPolicy.java | 11 ++++++----- ...fRetry.java => ExponentialBackoffRetryPolicy.java} | 4 ++-- .../retry/{ForeverRetry.java => RetryForever.java} | 4 ++-- .../retry/{MaxRetriesRetry.java => RetryNTimes.java} | 4 ++-- .../{MaxElapsedRetry.java => RetryUntilElapsed.java} | 4 ++-- .../java/tech/ydb/core/retry/RetryPoliciesTest.java | 10 +++++----- 6 files changed, 19 insertions(+), 18 deletions(-) rename core/src/main/java/tech/ydb/core/retry/{ExponentialBackoffRetry.java => ExponentialBackoffRetryPolicy.java} (77%) rename core/src/main/java/tech/ydb/core/retry/{ForeverRetry.java => RetryForever.java} (76%) rename core/src/main/java/tech/ydb/core/retry/{MaxRetriesRetry.java => RetryNTimes.java} (73%) rename core/src/main/java/tech/ydb/core/retry/{MaxElapsedRetry.java => RetryUntilElapsed.java} (76%) diff --git a/core/src/main/java/tech/ydb/core/RetryPolicy.java b/core/src/main/java/tech/ydb/core/RetryPolicy.java index e921e8e81..a925c4228 100644 --- a/core/src/main/java/tech/ydb/core/RetryPolicy.java +++ b/core/src/main/java/tech/ydb/core/RetryPolicy.java @@ -7,16 +7,17 @@ */ public interface RetryPolicy { /** - * Called when an operation has failed for some reason. This method may return next values + * Called when an operation is failed for some reason to determine if it should be retried. + * And if so, returns the delay to make the next retry attempt after + * + * @param retryCount the number of times retried so far (0 the first time) + * @param elapsedTimeMs the elapsed time in ms since the operation was attempted + * @return delay for the next retry *
    *
  • Positive number N - operation must be retried in N milliseconds
    • *
  • Zero : operation must be retried immediately
    • *
  • Negative number : retry is not allowed, operation must be failed
    • *
- * - * @param retryCount the number of times retried so far (0 the first time) - * @param elapsedTimeMs the elapsed time in ms since the operation was attempted - * @return number of milliseconds for next retry */ long nextRetryMs(int retryCount, long elapsedTimeMs); } diff --git a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java similarity index 77% rename from core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java rename to core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java index b0e0a9e53..47d5eb107 100644 --- a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java @@ -8,11 +8,11 @@ * * @author Aleksandr Gorshenin */ -public abstract class ExponentialBackoffRetry implements RetryPolicy { +public abstract class ExponentialBackoffRetryPolicy implements RetryPolicy { private final long backoffMs; private final int backoffCeiling; - protected ExponentialBackoffRetry(long backoffMs, int backoffCeiling) { + protected ExponentialBackoffRetryPolicy(long backoffMs, int backoffCeiling) { this.backoffMs = backoffMs; this.backoffCeiling = backoffCeiling; } diff --git a/core/src/main/java/tech/ydb/core/retry/ForeverRetry.java b/core/src/main/java/tech/ydb/core/retry/RetryForever.java similarity index 76% rename from core/src/main/java/tech/ydb/core/retry/ForeverRetry.java rename to core/src/main/java/tech/ydb/core/retry/RetryForever.java index 5f5a0c78f..5a7042c59 100644 --- a/core/src/main/java/tech/ydb/core/retry/ForeverRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryForever.java @@ -6,10 +6,10 @@ * * @author Aleksandr Gorshenin */ -public class ForeverRetry implements RetryPolicy { +public class RetryForever implements RetryPolicy { private final long intervalMs; - public ForeverRetry(long intervalMs) { + public RetryForever(long intervalMs) { this.intervalMs = intervalMs; } diff --git a/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java similarity index 73% rename from core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java rename to core/src/main/java/tech/ydb/core/retry/RetryNTimes.java index ef76e002d..cfc2cc0e8 100644 --- a/core/src/main/java/tech/ydb/core/retry/MaxRetriesRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java @@ -4,10 +4,10 @@ * * @author Aleksandr Gorshenin */ -public class MaxRetriesRetry extends ExponentialBackoffRetry { +public class RetryNTimes extends ExponentialBackoffRetryPolicy { private final int maxRetries; - public MaxRetriesRetry(int maxRetries, long backoffMs, int backoffCeiling) { + public RetryNTimes(int maxRetries, long backoffMs, int backoffCeiling) { super(backoffMs, backoffCeiling); this.maxRetries = maxRetries; } diff --git a/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java similarity index 76% rename from core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java rename to core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java index c568aaf74..926a4a7cd 100644 --- a/core/src/main/java/tech/ydb/core/retry/MaxElapsedRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java @@ -4,10 +4,10 @@ * * @author Aleksandr Gorshenin */ -public class MaxElapsedRetry extends ExponentialBackoffRetry { +public class RetryUntilElapsed extends ExponentialBackoffRetryPolicy { private final long maxElapsedMs; - public MaxElapsedRetry(long maxElapsedMs, long backoffMs, int backoffCeiling) { + public RetryUntilElapsed(long maxElapsedMs, long backoffMs, int backoffCeiling) { super(backoffMs, backoffCeiling); this.maxElapsedMs = maxElapsedMs; } diff --git a/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java index 3a58cecf3..a33aeedfe 100644 --- a/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java +++ b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java @@ -13,7 +13,7 @@ public class RetryPoliciesTest { @Test public void foreverRetryTest() { - RetryPolicy policy = new ForeverRetry(1234); + RetryPolicy policy = new RetryForever(1234); Assert.assertEquals(1234, policy.nextRetryMs(0, 0)); Assert.assertEquals(1234, policy.nextRetryMs(Integer.MAX_VALUE, 0)); @@ -23,7 +23,7 @@ public void foreverRetryTest() { @Test public void zeroRetriesTest() { - RetryPolicy policy = new MaxRetriesRetry(0, 100, 3); + RetryPolicy policy = new RetryNTimes(0, 100, 3); Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); @@ -33,7 +33,7 @@ public void zeroRetriesTest() { @Test public void zeroTimeoutTest() { - RetryPolicy policy = new MaxElapsedRetry(0, 100, 3); + RetryPolicy policy = new RetryUntilElapsed(0, 100, 3); Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); @@ -43,7 +43,7 @@ public void zeroTimeoutTest() { @Test public void maxRetriesTest() { - RetryPolicy policy = new MaxRetriesRetry(5, 100, 3); + RetryPolicy policy = new RetryNTimes(5, 100, 3); assertDuration(100, 200, policy.nextRetryMs(0, 0)); assertDuration(200, 400, policy.nextRetryMs(1, 150)); @@ -56,7 +56,7 @@ public void maxRetriesTest() { @Test public void maxTimeoutTest() { - RetryPolicy policy = new MaxElapsedRetry(2500, 50, 3); + RetryPolicy policy = new RetryUntilElapsed(2500, 50, 3); assertDuration(50, 100, policy.nextRetryMs(0, 0)); assertDuration(100, 200, policy.nextRetryMs(1, 75)); From 15d6954f127a5e43e7b3cde395ea8d9f07d8717e Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 22 Jan 2024 13:19:23 +0000 Subject: [PATCH 5/6] Added modify methods --- ...olicy.java => ExponentialBackoffRetry.java} | 12 ++++++++++-- .../java/tech/ydb/core/retry/RetryForever.java | 8 ++++++++ .../java/tech/ydb/core/retry/RetryNTimes.java | 18 +++++++++++++++++- .../tech/ydb/core/retry/RetryUntilElapsed.java | 18 +++++++++++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) rename core/src/main/java/tech/ydb/core/retry/{ExponentialBackoffRetryPolicy.java => ExponentialBackoffRetry.java} (65%) diff --git a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java similarity index 65% rename from core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java rename to core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java index 47d5eb107..1bfaf1028 100644 --- a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetryPolicy.java +++ b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java @@ -8,11 +8,11 @@ * * @author Aleksandr Gorshenin */ -public abstract class ExponentialBackoffRetryPolicy implements RetryPolicy { +public abstract class ExponentialBackoffRetry implements RetryPolicy { private final long backoffMs; private final int backoffCeiling; - protected ExponentialBackoffRetryPolicy(long backoffMs, int backoffCeiling) { + protected ExponentialBackoffRetry(long backoffMs, int backoffCeiling) { this.backoffMs = backoffMs; this.backoffCeiling = backoffCeiling; } @@ -22,4 +22,12 @@ protected long backoffTimeMillis(int retryNumber) { long delay = backoffMs * slots; return delay + ThreadLocalRandom.current().nextLong(delay); } + + public long getBackoffMillis() { + return backoffMs; + } + + public int getBackoffCeiling() { + return backoffCeiling; + } } diff --git a/core/src/main/java/tech/ydb/core/retry/RetryForever.java b/core/src/main/java/tech/ydb/core/retry/RetryForever.java index 5a7042c59..24f249c07 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryForever.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryForever.java @@ -17,4 +17,12 @@ public RetryForever(long intervalMs) { public long nextRetryMs(int retryCount, long elapsedTimeMs) { return intervalMs; } + + /** + * Create new retry policy with specified retry interval + * @param ms new interval in milliseconds + * @return updated retry policy */ + public RetryForever withIntervalMs(long ms) { + return new RetryForever(ms); + } } diff --git a/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java index cfc2cc0e8..6c4d88875 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java @@ -4,7 +4,7 @@ * * @author Aleksandr Gorshenin */ -public class RetryNTimes extends ExponentialBackoffRetryPolicy { +public class RetryNTimes extends ExponentialBackoffRetry { private final int maxRetries; public RetryNTimes(int maxRetries, long backoffMs, int backoffCeiling) { @@ -19,4 +19,20 @@ public long nextRetryMs(int retryCount, long elapsedTimeMs) { } return backoffTimeMillis(retryCount); } + + /** + * Create new retry policy with specified backoff duration + * @param ms new backoff duration in milliseconds + * @return updated retry policy */ + public RetryNTimes withBackoffMs(long ms) { + return new RetryNTimes(maxRetries, ms, getBackoffCeiling()); + } + + /** + * Create new retry policy with specified backoff ceiling + * @param ceiling new backoff ceiling + * @return updated retry policy */ + public RetryNTimes withBackoffCeiling(int ceiling) { + return new RetryNTimes(maxRetries, getBackoffMillis(), ceiling); + } } diff --git a/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java index 926a4a7cd..ad91e598b 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java @@ -4,7 +4,7 @@ * * @author Aleksandr Gorshenin */ -public class RetryUntilElapsed extends ExponentialBackoffRetryPolicy { +public class RetryUntilElapsed extends ExponentialBackoffRetry { private final long maxElapsedMs; public RetryUntilElapsed(long maxElapsedMs, long backoffMs, int backoffCeiling) { @@ -20,4 +20,20 @@ public long nextRetryMs(int retryCount, long elapsedTimeMs) { long backoff = backoffTimeMillis(retryCount); return (elapsedTimeMs + backoff < maxElapsedMs) ? backoff : maxElapsedMs - elapsedTimeMs; } + + /** + * Create new retry policy with specified backoff duration + * @param ms new backoff duration + * @return new retry policy */ + public RetryUntilElapsed withBackoffMs(long ms) { + return new RetryUntilElapsed(maxElapsedMs, ms, getBackoffCeiling()); + } + + /** + * Create new retry policy with specified backoff ceiling + * @param ceiling new backoff ceiling + * @return new retry policy */ + public RetryUntilElapsed withBackoffCeiling(int ceiling) { + return new RetryUntilElapsed(maxElapsedMs, getBackoffMillis(), ceiling); + } } From 93ca5accdfb210718058da136ebbee78e73166db Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 22 Jan 2024 15:00:33 +0000 Subject: [PATCH 6/6] Added getters and tests --- .../core/retry/ExponentialBackoffRetry.java | 8 ++ .../tech/ydb/core/retry/RetryForever.java | 8 ++ .../java/tech/ydb/core/retry/RetryNTimes.java | 16 ++++ .../ydb/core/retry/RetryUntilElapsed.java | 16 ++++ .../ydb/core/retry/RetryPoliciesTest.java | 75 +++++++++++++++---- 5 files changed, 107 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java index 1bfaf1028..973ad655b 100644 --- a/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java +++ b/core/src/main/java/tech/ydb/core/retry/ExponentialBackoffRetry.java @@ -23,10 +23,18 @@ protected long backoffTimeMillis(int retryNumber) { return delay + ThreadLocalRandom.current().nextLong(delay); } + /** + * Return current base of backoff delays + * @return backoff base duration in milliseconds + */ public long getBackoffMillis() { return backoffMs; } + /** + * Return current maximal level of backoff exponent + * @return maximal level of backoff exponent + */ public int getBackoffCeiling() { return backoffCeiling; } diff --git a/core/src/main/java/tech/ydb/core/retry/RetryForever.java b/core/src/main/java/tech/ydb/core/retry/RetryForever.java index 24f249c07..e75ed0f1e 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryForever.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryForever.java @@ -18,6 +18,14 @@ public long nextRetryMs(int retryCount, long elapsedTimeMs) { return intervalMs; } + /** + * Return current interval of retries + * @return retry interval in milliseconds + */ + public long getIntervalMillis() { + return intervalMs; + } + /** * Create new retry policy with specified retry interval * @param ms new interval in milliseconds diff --git a/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java index 6c4d88875..47282a58a 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryNTimes.java @@ -20,6 +20,22 @@ public long nextRetryMs(int retryCount, long elapsedTimeMs) { return backoffTimeMillis(retryCount); } + /** + * Return maximal count of retries + * @return maximal count of retries + */ + public int getMaxRetries() { + return maxRetries; + } + + /** + * Create new retry policy with specified max retries count + * @param maxRetries new value of max count of retries + * @return updated retry policy */ + public RetryNTimes withMaxRetries(int maxRetries) { + return new RetryNTimes(maxRetries, getBackoffMillis(), getBackoffCeiling()); + } + /** * Create new retry policy with specified backoff duration * @param ms new backoff duration in milliseconds diff --git a/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java index ad91e598b..11fb4fecc 100644 --- a/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java +++ b/core/src/main/java/tech/ydb/core/retry/RetryUntilElapsed.java @@ -21,6 +21,22 @@ public long nextRetryMs(int retryCount, long elapsedTimeMs) { return (elapsedTimeMs + backoff < maxElapsedMs) ? backoff : maxElapsedMs - elapsedTimeMs; } + /** + * Return maximal count of elapsed milliseconds + * @return maximal count of elapsed milliseconds + */ + public long getMaxElapsedMillis() { + return maxElapsedMs; + } + + /** + * Create new retry policy with specified count of elapsed milliseconds + * @param maxElapsedMs new value of max elapsed milliseconds + * @return updated retry policy */ + public RetryUntilElapsed withMaxElapsedMs(long maxElapsedMs) { + return new RetryUntilElapsed(maxElapsedMs, getBackoffMillis(), getBackoffCeiling()); + } + /** * Create new retry policy with specified backoff duration * @param ms new backoff duration diff --git a/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java index a33aeedfe..52418f08e 100644 --- a/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java +++ b/core/src/test/java/tech/ydb/core/retry/RetryPoliciesTest.java @@ -3,8 +3,6 @@ import org.junit.Assert; import org.junit.Test; -import tech.ydb.core.RetryPolicy; - /** * * @author Aleksandr Gorshenin @@ -13,7 +11,7 @@ public class RetryPoliciesTest { @Test public void foreverRetryTest() { - RetryPolicy policy = new RetryForever(1234); + RetryForever policy = new RetryForever(1234); Assert.assertEquals(1234, policy.nextRetryMs(0, 0)); Assert.assertEquals(1234, policy.nextRetryMs(Integer.MAX_VALUE, 0)); @@ -22,18 +20,19 @@ public void foreverRetryTest() { } @Test - public void zeroRetriesTest() { - RetryPolicy policy = new RetryNTimes(0, 100, 3); - - Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); - Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); - Assert.assertEquals(-1, policy.nextRetryMs(0, Integer.MAX_VALUE)); - Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, Integer.MAX_VALUE)); + public void foreverUpdateTest() { + RetryForever policy = new RetryForever(50); + Assert.assertEquals(50, policy.getIntervalMillis()); + Assert.assertEquals(50, policy.nextRetryMs(0, 0)); + + RetryForever updated = policy.withIntervalMs(150); + Assert.assertEquals(150, updated.getIntervalMillis()); + Assert.assertEquals(150, updated.nextRetryMs(0, 0)); } @Test - public void zeroTimeoutTest() { - RetryPolicy policy = new RetryUntilElapsed(0, 100, 3); + public void zeroRetriesTest() { + RetryNTimes policy = new RetryNTimes(0, 100, 3); Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); @@ -42,8 +41,8 @@ public void zeroTimeoutTest() { } @Test - public void maxRetriesTest() { - RetryPolicy policy = new RetryNTimes(5, 100, 3); + public void nRetriesTest() { + RetryNTimes policy = new RetryNTimes(5, 100, 3); assertDuration(100, 200, policy.nextRetryMs(0, 0)); assertDuration(200, 400, policy.nextRetryMs(1, 150)); @@ -55,8 +54,35 @@ public void maxRetriesTest() { } @Test - public void maxTimeoutTest() { - RetryPolicy policy = new RetryUntilElapsed(2500, 50, 3); + public void updateNRetriesTest() { + RetryNTimes policy = new RetryNTimes(5, 100, 3); + + Assert.assertEquals(5, policy.getMaxRetries()); + Assert.assertEquals(100, policy.getBackoffMillis()); + Assert.assertEquals(3, policy.getBackoffCeiling()); + assertDuration(100, 200, policy.nextRetryMs(0, 0)); + + RetryNTimes updated = policy.withMaxRetries(4).withBackoffMs(150).withBackoffCeiling(1); + + Assert.assertEquals(4, updated.getMaxRetries()); + Assert.assertEquals(150, updated.getBackoffMillis()); + Assert.assertEquals(1, updated.getBackoffCeiling()); + assertDuration(150, 300, updated.nextRetryMs(0, 0)); + } + + @Test + public void zeroElapsedTest() { + RetryUntilElapsed policy = new RetryUntilElapsed(0, 100, 3); + + Assert.assertEquals(-1, policy.nextRetryMs(0, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, 0)); + Assert.assertEquals(-1, policy.nextRetryMs(0, Integer.MAX_VALUE)); + Assert.assertEquals(-1, policy.nextRetryMs(Integer.MAX_VALUE, Integer.MAX_VALUE)); + } + + @Test + public void untilElapsedTest() { + RetryUntilElapsed policy = new RetryUntilElapsed(2500, 50, 3); assertDuration(50, 100, policy.nextRetryMs(0, 0)); assertDuration(100, 200, policy.nextRetryMs(1, 75)); @@ -69,6 +95,23 @@ public void maxTimeoutTest() { Assert.assertEquals(-1, policy.nextRetryMs(7, 2500)); } + @Test + public void updateElapsedTest() { + RetryUntilElapsed policy = new RetryUntilElapsed(2500, 50, 3); + + Assert.assertEquals(2500, policy.getMaxElapsedMillis()); + Assert.assertEquals(50, policy.getBackoffMillis()); + Assert.assertEquals(3, policy.getBackoffCeiling()); + assertDuration(50, 100, policy.nextRetryMs(0, 0)); + + RetryUntilElapsed updated = policy.withMaxElapsedMs(1000).withBackoffMs(100).withBackoffCeiling(1); + + Assert.assertEquals(1000, updated.getMaxElapsedMillis()); + Assert.assertEquals(100, updated.getBackoffMillis()); + Assert.assertEquals(1, updated.getBackoffCeiling()); + assertDuration(100, 200, updated.nextRetryMs(0, 0)); + } + private void assertDuration(long from, long to, long ms) { Assert.assertTrue(from <= ms); Assert.assertTrue(to >= ms);