This is an automated email from the ASF dual-hosted git repository.
szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git
The following commit(s) were added to refs/heads/master by this push:
new 0443685b0 RATIS-2364. MultipleLinearRandomRetry should throw
exceptions for illegal arguments. (#1319)
0443685b0 is described below
commit 0443685b0c4e5d29a298fcf1d1e83bbfee3fc95d
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Tue Nov 25 23:45:02 2025 -0800
RATIS-2364. MultipleLinearRandomRetry should throw exceptions for illegal
arguments. (#1319)
---
ratis-common/dev-support/findbugsExcludeFile.xml | 8 ----
.../ratis/retry/MultipleLinearRandomRetry.java | 56 +++++++---------------
.../ratis/retry/TestMultipleLinearRandomRetry.java | 10 ++--
3 files changed, 24 insertions(+), 50 deletions(-)
diff --git a/ratis-common/dev-support/findbugsExcludeFile.xml
b/ratis-common/dev-support/findbugsExcludeFile.xml
index 9267e763f..787621f17 100644
--- a/ratis-common/dev-support/findbugsExcludeFile.xml
+++ b/ratis-common/dev-support/findbugsExcludeFile.xml
@@ -23,10 +23,6 @@
<Class name="org.apache.ratis.protocol.GroupInfoReply" />
<Bug pattern="EI_EXPOSE_REP2" />
</Match>
- <Match>
- <Class name="org.apache.ratis.retry.MultipleLinearRandomRetry" />
- <Bug pattern="CT_CONSTRUCTOR_THROW" />
- </Match>
<Match>
<Class name="org.apache.ratis.util.AtomicFileOutputStream" />
<Bug pattern="CT_CONSTRUCTOR_THROW" />
@@ -35,10 +31,6 @@
<Class name="org.apache.ratis.util.Daemon" />
<Bug pattern="EI_EXPOSE_REP2" />
</Match>
- <Match>
- <Class name="org.apache.ratis.retry.MultipleLinearRandomRetry$Pair" />
- <Bug pattern="CT_CONSTRUCTOR_THROW" />
- </Match>
<Match>
<Class name="org.apache.ratis.util.Daemon$Builder" />
<Bug pattern="EI_EXPOSE_REP2" />
diff --git
a/ratis-common/src/main/java/org/apache/ratis/retry/MultipleLinearRandomRetry.java
b/ratis-common/src/main/java/org/apache/ratis/retry/MultipleLinearRandomRetry.java
index bc453f5be..9cceb6bc5 100644
---
a/ratis-common/src/main/java/org/apache/ratis/retry/MultipleLinearRandomRetry.java
+++
b/ratis-common/src/main/java/org/apache/ratis/retry/MultipleLinearRandomRetry.java
@@ -19,8 +19,6 @@ package org.apache.ratis.retry;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.TimeDuration;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
@@ -34,28 +32,19 @@ import java.util.function.Supplier;
* Given pairs of number of retries and sleep time (n0, t0), (n1, t1), ...,
* the first n0 retries sleep t0 milliseconds on average,
* the following n1 retries sleep t1 milliseconds on average, and so on.
- *
+ * <p>
* For all the sleep, the actual sleep time is randomly uniform distributed
* in the close interval [0.5t, 1.5t], where t is the sleep time specified.
- *
+ * <p>
* The objects of this class are immutable.
*/
public final class MultipleLinearRandomRetry implements RetryPolicy {
- static final Logger LOG =
LoggerFactory.getLogger(MultipleLinearRandomRetry.class);
-
/** Pairs of numRetries and sleepSeconds */
- private static class Pair {
+ private static final class Pair {
private final int numRetries;
private final TimeDuration sleepTime;
Pair(int numRetries, TimeDuration sleepTime) {
- if (numRetries < 0) {
- throw new IllegalArgumentException("numRetries = " + numRetries+" <
0");
- }
- if (sleepTime.isNegative()) {
- throw new IllegalArgumentException("sleepTime = " + sleepTime + " <
0");
- }
-
this.numRetries = numRetries;
this.sleepTime = sleepTime;
}
@@ -76,9 +65,6 @@ public final class MultipleLinearRandomRetry implements
RetryPolicy {
private final Supplier<String> myString;
private MultipleLinearRandomRetry(List<Pair> pairs) {
- if (pairs == null || pairs.isEmpty()) {
- throw new IllegalArgumentException("pairs must be neither null nor
empty.");
- }
this.pairs = Collections.unmodifiableList(pairs);
this.myString = JavaUtils.memoize(() ->
JavaUtils.getClassSimpleName(getClass()) + pairs);
}
@@ -131,30 +117,22 @@ public final class MultipleLinearRandomRetry implements
RetryPolicy {
* @return the parsed object, or null if the parsing fails.
*/
public static MultipleLinearRandomRetry parseCommaSeparated(String input) {
- final String[] elements = input.split(",");
- if (elements.length == 0) {
- LOG.warn("Illegal value: there is no element in \"{}\".", input);
- return null;
+ input = input.trim();
+ if (input.isEmpty()) {
+ throw new IllegalArgumentException("Failed to parse \"" + input + "\":
no elements found");
}
+ final String[] elements = input.split(",");
if (elements.length % 2 != 0) {
- LOG.warn("Illegal value: the number of elements in \"{}\" is {} but an
even number of elements is expected.",
- input, elements.length);
- return null;
+ throw new IllegalArgumentException("Failed to parse \"" + input
+ + "\": number of elements (" + elements.length + ") is old");
}
final List<Pair> pairs = new ArrayList<>();
for(int i = 0; i < elements.length; ) {
- //parse the i-th sleep-time
- final TimeDuration sleep = parseElement(elements, i++, input,
MultipleLinearRandomRetry::parsePositiveTime);
- if (sleep == null) {
- return null; //parse fails
- }
- //parse the i-th number-of-retries
- final Integer retries = parseElement(elements, i++, input,
MultipleLinearRandomRetry::parsePositiveInt);
- if (retries == null) {
- return null; //parse fails
- }
-
+ final TimeDuration sleep = parseElement("sleep-time", elements, i++,
input,
+ MultipleLinearRandomRetry::parsePositiveTime);
+ final Integer retries = parseElement("retries", elements, i++, input,
+ MultipleLinearRandomRetry::parsePositiveInt);
pairs.add(new Pair(retries, sleep));
}
return new MultipleLinearRandomRetry(pairs);
@@ -176,13 +154,13 @@ public final class MultipleLinearRandomRetry implements
RetryPolicy {
return n;
}
- private static <E> E parseElement(String[] elements, int i, String input,
Function<String, E> parser) {
+ private static <E> E parseElement(String name, String[] elements, int i,
String input, Function<String, E> parser) {
final String s = elements[i].trim().replace("_", "");
try {
return parser.apply(s);
- } catch(Exception t) {
- LOG.warn("Failed to parse \"{}\", which is the index {} element in
\"{}\"", s, i, input, t);
- return null;
+ } catch (Exception e) {
+ throw new IllegalArgumentException(
+ "Failed to parse \"" + s + "\" as " + name + " (element " + i + " in
\"" + input + "\")", e);
}
}
}
diff --git
a/ratis-test/src/test/java/org/apache/ratis/retry/TestMultipleLinearRandomRetry.java
b/ratis-test/src/test/java/org/apache/ratis/retry/TestMultipleLinearRandomRetry.java
index 77bcb70f7..621d46b5b 100644
---
a/ratis-test/src/test/java/org/apache/ratis/retry/TestMultipleLinearRandomRetry.java
+++
b/ratis-test/src/test/java/org/apache/ratis/retry/TestMultipleLinearRandomRetry.java
@@ -44,10 +44,14 @@ public class TestMultipleLinearRandomRetry extends BaseTest
{
assertLegalInput("[10x100ms, 20x1s, 30x5s]", "100,10, 1s,20, 5s,30");
}
- private static void assertIllegalInput(String input) {
- final MultipleLinearRandomRetry computed =
MultipleLinearRandomRetry.parseCommaSeparated(input);
- Assertions.assertNull(computed);
+ private void assertIllegalInput(String input) {
+ try {
+ MultipleLinearRandomRetry.parseCommaSeparated(input);
+ } catch (IllegalArgumentException e) {
+ LOG.info("Expected to catch: {}", String.valueOf(e));
+ }
}
+
private static MultipleLinearRandomRetry assertLegalInput(String expected,
String input) {
final MultipleLinearRandomRetry computed =
MultipleLinearRandomRetry.parseCommaSeparated(input);
Assertions.assertNotNull(computed);
