diff --git a/CHANGES.txt b/CHANGES.txt index 577b2ee1b5..7937add486 100644 --- a/CHANGES.txt +++ b/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) diff --git a/src/java/org/apache/cassandra/service/RetryStrategy.java b/src/java/org/apache/cassandra/service/RetryStrategy.java index ba1b01dfc8..e11c507bf5 100644 --- a/src/java/org/apache/cassandra/service/RetryStrategy.java +++ b/src/java/org/apache/cassandra/service/RetryStrategy.java @@ -66,7 +66,7 @@ import static org.apache.cassandra.utils.Clock.Global.nanoTime; *
  • dynamic constant {@code pX() * constant} *
  • dynamic linear {@code pX() * constant * attempts} *
  • dynamic exponential {@code pX() * constant ^ attempts} - * + *
  • Note: for dynamic exponential, attempts is subtracted by 1, such that times begin at {@code pX() * constant}. * e.g. *
  • {@code 10ms <= p50(rw)*0.66...p99(rw)} *
  • {@code 10ms <= p95(rw)*1.8^attempts <= 100ms} diff --git a/src/java/org/apache/cassandra/service/TimeoutStrategy.java b/src/java/org/apache/cassandra/service/TimeoutStrategy.java index 0c2e64d49a..68be0553d7 100644 --- a/src/java/org/apache/cassandra/service/TimeoutStrategy.java +++ b/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 diff --git a/test/unit/org/apache/cassandra/service/TimeoutStrategyTest.java b/test/unit/org/apache/cassandra/service/TimeoutStrategyTest.java new file mode 100644 index 0000000000..fe3b7d8186 --- /dev/null +++ b/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)); + } +}