dcapwell commented on code in PR #4549:
URL: https://github.com/apache/cassandra/pull/4549#discussion_r2674061499
##########
src/java/org/apache/cassandra/service/TimeoutStrategy.java:
##########
@@ -97,7 +97,8 @@ interface LatencyModifierFactory
default LatencyModifier identity() { return (l, a) -> l; }
default LatencyModifier multiply(double constant) { return (l, a) ->
saturatedCast(l * constant); }
default LatencyModifier multiplyByAttempts(double multiply) { return
(l, a) -> saturatedCast(l * multiply * a); }
- default LatencyModifier multiplyByAttemptsExp(double base) { return
(l, a) -> saturatedCast(l * pow(base, a)); }
+ // Ensure attempts is non-negative before subtracting 1.
+ default LatencyModifier multiplyByAttemptsExp(double base) { return
(l, a) -> saturatedCast(l * pow(base, max(0, (max(a, 0) - 1)))); }
Review Comment:
this looks safe
```
$ rg '\^attempts' src
src/java/org/apache/cassandra/config/RetrySpec.java
164: return RetryStrategy.parse(spec.baseSleepTime.toMilliseconds() +
"ms * 2^attempts <= " + spec.maxSleepTime.toMilliseconds() + "ms,retries=" +
(spec.maxAttempts.value - 1), LatencySourceFactory.none());
src/java/org/apache/cassandra/service/RetryStrategy.java
72: * <li> {@code 10ms <= p95(rw)*1.8^attempts <= 100ms}
src/java/org/apache/cassandra/service/TimeoutStrategy.java
73: * <li> {@code 10ms <= p95(rw)*1.8^attempts <= 100ms}
```
The only user is repair, so not really a breaking change to paxos. If
anyone was using this in paxos v2, the only impact is that it would have the
first retry respect the min value rather than do `min * 2`, so LGTM
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]