Fix off-by-one bug in exponential backoff for repair retry config

patch by Nivy Kani; reviewed by David Capwell, Jyothsna Konisa for CASSANDRA-21102

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Fix off-by-one bug in exponential backoff for repair retry config (CASSANDRA-21102)
  * Move training parameters for Zstd dictionary compression to CQL (CASSANDRA-21078)
  * Add configuration for sorted imports in source files (CASSANDRA-17925)
  * Change the eager reference counting of compression dictionaries to lazy (CASSANDRA-21074)

src/java/org/apache/cassandra/service/RetryStrategy.java

@@ -66,7 +66,7 @@ import static org.apache.cassandra.utils.Clock.Global.nanoTime;
  * <li> dynamic constant    {@code pX() * constant}
  * <li> dynamic linear      {@code pX() * constant * attempts}
  * <li> dynamic exponential {@code pX() * constant ^ attempts}
- *
+ * <li> Note: for dynamic exponential, attempts is subtracted by 1, such that times begin at {@code pX() * constant}.
  * e.g.
  * <li> {@code 10ms <= p50(rw)*0.66...p99(rw)}
  * <li> {@code 10ms <= p95(rw)*1.8^attempts <= 100ms}

src/java/org/apache/cassandra/service/TimeoutStrategy.java

@@ -98,7 +98,8 @@ public class TimeoutStrategy implements WaitStrategy
         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)))); }
     }
 
     public interface Wait

test/unit/org/apache/cassandra/service/TimeoutStrategyTest.java

@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.service;
+
+import java.util.concurrent.TimeUnit;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class TimeoutStrategyTest
+{
+
+    @Test
+    public void testParseLatencyModifierExponential()
+    {
+        long expectedBaseLatencyMicros = TimeUnit.MILLISECONDS.toMicros(30);
+        String spec = "30ms * 2^attempts";
+        TimeoutStrategy.Wait w = TimeoutStrategy.parseWait(spec, TimeoutStrategy.LatencySourceFactory.none());
+
+        // Attempt 1: baseLatency * 2^(1-1) = baseLatency * 1
+        Assertions.assertThat(w.getMicros(1))
+                .isEqualTo(expectedBaseLatencyMicros);
+
+        // Attempt 2: baseLatency * 2^(2-1) = baseLatency * 2
+        Assertions.assertThat(w.getMicros(2))
+                .isEqualTo(expectedBaseLatencyMicros * 2);
+
+        // Attempt 3: baseLatency * 2^(3-1) = baseLatency * 4
+        Assertions.assertThat(w.getMicros(3))
+                .isEqualTo(expectedBaseLatencyMicros * 4);
+
+        // Edge case to check for 0 or negative attempts: max(0, -1) = 0
+        Assertions.assertThat(w.getMicros(0))
+                .isEqualTo(expectedBaseLatencyMicros);
+
+        Assertions.assertThat(w.getMicros(Integer.MIN_VALUE))
+                .isEqualTo(expectedBaseLatencyMicros);
+    }
+
+    @Test
+    public void testParseLatencyModifierFractionalBaseExponential()
+    {
+        long expectedBaseLatencyMicros = TimeUnit.MILLISECONDS.toMicros(30);
+        String spec = "30ms * 1.5^attempts";
+        TimeoutStrategy.Wait w = TimeoutStrategy.parseWait(spec, TimeoutStrategy.LatencySourceFactory.none());
+
+        // Attempt 1: baseLatency * 1.5^(1-1) = baseLatency * 1
+        Assertions.assertThat(w.getMicros(1))
+                .isEqualTo((int) expectedBaseLatencyMicros);
+
+        // Attempt 2: baseLatency * 1.5^(2-1) = baseLatency * 1.5
+        Assertions.assertThat(w.getMicros(2))
+                .isEqualTo((int) (expectedBaseLatencyMicros * 1.5));
+
+        // Attempt 3: baseLatency * 1.5^(3-1) = baseLatency * 2.25
+        Assertions.assertThat(w.getMicros(3))
+                .isEqualTo((int) (expectedBaseLatencyMicros * 2.25));
+    }
+}