szetszwo commented on code in PR #1354:
URL: https://github.com/apache/ratis/pull/1354#discussion_r2824448018
##########
ratis-docs/src/site/markdown/configurations.md:
##########
@@ -509,6 +509,14 @@ The follower's statemachine is responsible for fetching
and installing snapshot
| **Type** | string |
| **Default** | 1ms,10, 1s,20, 5s,1000 |
+Format:
+`<classname>,<params...>`
+If `<classname>` is omitted, it defaults to `MultipleLinearRandomRetry` for
backward compatibility.
+
+Examples:
+- `MultipleLinearRandomRetry,1ms,10,1s,20,5s,1000`
+- `1ms,10,1s,20,5s,1000` (same as above)
Review Comment:
Let's add one more example:
```md
- `ExponentialBackoffRetry,100ms,5s,100`
```
##########
ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java:
##########
@@ -95,7 +96,31 @@ static RetryPolicy parse(String commaSeparated) {
.setMaxAttempts(Integer.parseInt(args[3].trim()))
.build();
}
+ if (classname.equals(MultipleLinearRandomRetry.class.getSimpleName())) {
+ if (args.length == 1) {
+ throw new IllegalArgumentException("Failed to parse
MultipleLinearRandomRetry: args.length = "
+ + args.length + " <= 1 for " + commaSeparated);
Review Comment:
Let's say the that "the parameter list is empty", i.e.
```java
"Failed to parse MultipleLinearRandomRetry: the parameter list is empty for
" + commaSeparated
```
##########
ratis-common/src/main/java/org/apache/ratis/retry/RetryPolicy.java:
##########
@@ -95,7 +96,31 @@ static RetryPolicy parse(String commaSeparated) {
.setMaxAttempts(Integer.parseInt(args[3].trim()))
.build();
}
+ if (classname.equals(MultipleLinearRandomRetry.class.getSimpleName())) {
+ if (args.length == 1) {
+ throw new IllegalArgumentException("Failed to parse
MultipleLinearRandomRetry: args.length = "
+ + args.length + " <= 1 for " + commaSeparated);
+ }
+ final String params = String.join(",", Arrays.copyOfRange(args, 1,
args.length));
+ return MultipleLinearRandomRetry.parseCommaSeparated(params);
+ }
+ // Backward compatibility: legacy config omits class name and starts with
a duration (e.g. "1ms").
+ if (isLegacyMultipleLinearRandomRetryParams(classname)) {
+ return MultipleLinearRandomRetry.parseCommaSeparated(commaSeparated);
+ }
+ // If a class name is present but unknown, fail fast to surface config
errors.
throw new IllegalArgumentException("Failed to parse RetryPolicy: unknown
class "
+ args[0] + " for " + commaSeparated);
}
+
+ static boolean isLegacyMultipleLinearRandomRetryParams(String firstElement) {
+ // The legacy format starts with a duration token, not a class name.
+ final String trimmed = firstElement.trim().replace("_", "");
Review Comment:
This line is not needed since TimeDuration.valueOf(..) will take care of it.
##########
ratis-docs/src/site/markdown/configurations.md:
##########
Review Comment:
Could you also update this paragraph?
```md
For `MultipleLinearRandomRetry`, the parameter "1ms,10, 1s,20, 5s,1000" means
that the wait time is $1$ms on average for the first 10 retries.
Then, it becomes $1$s on average for next 20 retries
and $5$s on average for the last 1000 retires.
For `ExponentialBackoffRetry`, the parameter "100ms,5s,100" means
that the base wait time is $100$ms, the maximum wait time is $5$s
and the number of attempts is 100.
The wait time is $\max(2^{(n-1)}\times 100\text{ms}, 5\text{s})$ on average
for the $n$-th retry.
In other words,
the wait time is on average $100$ms, $200$ms, $400$ms, $800$ms, $1.6$s,
$3.2$s, $5$s, $5$s and so on.
Note that the actual wait time is randomized by a multiplier in the range
$[0.5, 1.5)$ for all retry policies.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
