From f8f456f0f674090652e64ac4f5aa276bf81f621d Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Fri, 21 Nov 2025 17:19:52 +0000 Subject: [PATCH] Fix default max interval calculation in FailureDetector Patch by Sam Tunnicliffe; reviewed by Maxim Muzafarov and Stefan Miklosovic for CASSANDRA-21025 --- CHANGES.txt | 1 + .../apache/cassandra/gms/FailureDetector.java | 17 +++++++-- .../cassandra/gms/FailureDetectorTest.java | 35 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index f6dfe4045a..d98f1b69ec 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.7 + * Correctly calculate default for FailureDetector max interval (CASSANDRA-21025) * Adding missing configs in system_views.settings to be backward compatible (CASSANDRA-20863) * Heap dump should not be generated on handled exceptions (CASSANDRA-20974) Merged from 4.1: diff --git a/src/java/org/apache/cassandra/gms/FailureDetector.java b/src/java/org/apache/cassandra/gms/FailureDetector.java index 9cf7ad0392..c2e148de1f 100644 --- a/src/java/org/apache/cassandra/gms/FailureDetector.java +++ b/src/java/org/apache/cassandra/gms/FailureDetector.java @@ -43,6 +43,7 @@ import javax.management.openmbean.TabularData; import javax.management.openmbean.TabularDataSupport; import javax.management.openmbean.TabularType; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -416,6 +417,17 @@ public class FailureDetector implements IFailureDetector, FailureDetectorMBean sb.append("-----------------------------------------------------------------------"); return sb.toString(); } + + /** + * Only for testing. In production code, ArrivalWindow instances call getMaxInterval() during + * intitialization and use the value to set the private final field MAX_INTERVAL_IN_NANO + * @return the value that would be used for to populate any new ArrivalWindow instance + */ + @VisibleForTesting + static long calculateMaxInterval() + { + return ArrivalWindow.getMaxInterval(); + } } /* @@ -485,9 +497,10 @@ class ArrivalWindow arrivalIntervals = new ArrayBackedBoundedStats(size); } - private static long getMaxInterval() + @VisibleForTesting + static long getMaxInterval() { - long newValue = FD_MAX_INTERVAL_MS.getLong(FailureDetector.INITIAL_VALUE_NANOS); + long newValue = FD_MAX_INTERVAL_MS.getLong(TimeUnit.NANOSECONDS.toMillis(FailureDetector.INITIAL_VALUE_NANOS)); if (newValue != FailureDetector.INITIAL_VALUE_NANOS) logger.info("Overriding {} from {}ms to {}ms", FD_MAX_INTERVAL_MS.getKey(), FailureDetector.INITIAL_VALUE_NANOS, newValue); return TimeUnit.NANOSECONDS.convert(newValue, TimeUnit.MILLISECONDS); diff --git a/test/unit/org/apache/cassandra/gms/FailureDetectorTest.java b/test/unit/org/apache/cassandra/gms/FailureDetectorTest.java index 8ec3dae913..32524cef9e 100644 --- a/test/unit/org/apache/cassandra/gms/FailureDetectorTest.java +++ b/test/unit/org/apache/cassandra/gms/FailureDetectorTest.java @@ -23,11 +23,13 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.UUID; +import java.util.concurrent.TimeUnit; import org.junit.BeforeClass; import org.junit.Test; import org.apache.cassandra.Util; +import org.apache.cassandra.config.CassandraRelevantProperties; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.commitlog.CommitLog; import org.apache.cassandra.dht.IPartitioner; @@ -38,6 +40,7 @@ import org.apache.cassandra.locator.TokenMetadata; import org.apache.cassandra.service.StorageService; import static org.apache.cassandra.config.CassandraRelevantProperties.MAX_LOCAL_PAUSE_IN_MS; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; public class FailureDetectorTest @@ -87,4 +90,36 @@ public class FailureDetectorTest FailureDetector.instance.interpret(leftHost); assertFalse("Left endpoint not convicted", FailureDetector.instance.isAlive(leftHost)); } + + @Test + public void testMaxIntervalCalculation() + { + // Default value for ArrivalWindow.MAX_INTERVAL_IN_NANO, which is supplied by + // ArrivalWindow::getMaxInterval should be 2000000000ns/2 seconds. + Long initialPropertyValue = CassandraRelevantProperties.FD_MAX_INTERVAL_MS.isPresent() + ? CassandraRelevantProperties.FD_MAX_INTERVAL_MS.getLong() + : null; + try + { + // verify that max interval isn't being set directly using system property + CassandraRelevantProperties.FD_MAX_INTERVAL_MS.reset(); + assertFalse(CassandraRelevantProperties.FD_MAX_INTERVAL_MS.isPresent()); + // in which case, max interval should default to INITIAL_VALUE_NANOS + assertEquals(FailureDetector.INITIAL_VALUE_NANOS, FailureDetector.calculateMaxInterval()); + + // max interval can be overridden, but it's value should be supplied in millis + long overrideMillis = TimeUnit.NANOSECONDS.toMillis(FailureDetector.INITIAL_VALUE_NANOS * 2); + CassandraRelevantProperties.FD_MAX_INTERVAL_MS.setLong(overrideMillis); + // max interval is a nanos value, so convert the override to get the expected value + long expectedNanos = TimeUnit.NANOSECONDS.convert(overrideMillis, TimeUnit.MILLISECONDS); + assertEquals(expectedNanos, FailureDetector.calculateMaxInterval()); + } + finally + { + if (initialPropertyValue == null) + CassandraRelevantProperties.FD_MAX_INTERVAL_MS.reset(); + else + CassandraRelevantProperties.FD_MAX_INTERVAL_MS.setLong(initialPropertyValue); + } + } }