Repository: cassandra Updated Branches: refs/heads/trunk d6a67598f -> 8b7e96761
Fix scheduling of speculative retry threshold recalculation patch by Aleksey Yeschenko; reviewed by Sam Tunnicliffe for CASSANDRA-14338 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/8b7e9676 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8b7e9676 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8b7e9676 Branch: refs/heads/trunk Commit: 8b7e96761f968b346aed08c0c201a8d40d801b19 Parents: d6a6759 Author: Aleksey Yeshchenko <alek...@apple.com> Authored: Fri Mar 23 13:08:39 2018 +0000 Committer: Aleksey Yeshchenko <alek...@apple.com> Committed: Fri Mar 23 13:43:00 2018 +0000 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/db/ColumnFamilyStore.java | 37 +++++++------------- .../cassandra/service/CassandraDaemon.java | 8 +++++ .../reads/AlwaysSpeculativeRetryPolicy.java | 6 ---- .../reads/FixedSpeculativeRetryPolicy.java | 6 ---- .../reads/HybridSpeculativeRetryPolicy.java | 6 ---- .../reads/NeverSpeculativeRetryPolicy.java | 6 ---- .../reads/PercentileSpeculativeRetryPolicy.java | 6 ---- .../service/reads/SpeculativeRetryPolicy.java | 2 -- 9 files changed, 21 insertions(+), 57 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index ac5269c..d4e5e37 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Fix scheduling of speculative retry threshold recalculation (CASSANDRA-14338) * Add support for hybrid MIN(), MAX() speculative retry policies (CASSANDRA-14293) * Correct and clarify SSLFactory.getSslContext method and call sites (CASSANDRA-14314) * Use zero as default score in DynamicEndpointSnitch (CASSANDRA-14252) http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/db/ColumnFamilyStore.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 52c280c..4f74667 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -77,7 +77,6 @@ import org.apache.cassandra.metrics.TableMetrics; import org.apache.cassandra.metrics.TableMetrics.Sampler; import org.apache.cassandra.schema.*; import org.apache.cassandra.schema.CompactionParams.TombstoneOption; -import org.apache.cassandra.service.reads.SpeculativeRetryPolicy; import org.apache.cassandra.service.CacheService; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.streaming.TableStreamManager; @@ -214,7 +213,6 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean public final TableMetrics metric; public volatile long sampleLatencyNanos; - private final ScheduledFuture<?> latencyCalculator; private final CassandraStreamManager streamManager; @@ -442,35 +440,27 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean { throw new RuntimeException(e); } - - SpeculativeRetryPolicy retryPolicy = metadata.get().params.speculativeRetry; - logger.trace("retryPolicy for {} is {}", name, retryPolicy); - if (!retryPolicy.isDynamic()) - { - // avoid scheduling the task in the first place for non-dynamic speculative retry policies - // e.g. always and never speculative policies will never change so we can just calculate once - // and avoid doing unnecessary work every ReadRpcTimeout() - sampleLatencyNanos = retryPolicy.calculateThreshold(metric.coordinatorReadLatency); - latencyCalculator = null; - } - else - { - latencyCalculator = ScheduledExecutors.optionalTasks.scheduleWithFixedDelay(() -> - { - SpeculativeRetryPolicy retryPolicy1 = metadata.get().params.speculativeRetry; - sampleLatencyNanos = retryPolicy1.calculateThreshold(metric.coordinatorReadLatency); - }, DatabaseDescriptor.getReadRpcTimeout(), DatabaseDescriptor.getReadRpcTimeout(), TimeUnit.MILLISECONDS); - } } else { - latencyCalculator = ScheduledExecutors.optionalTasks.schedule(Runnables.doNothing(), 0, TimeUnit.NANOSECONDS); mbeanName = null; oldMBeanName= null; } streamManager = new CassandraStreamManager(this); } + public void updateSpeculationThreshold() + { + try + { + sampleLatencyNanos = metadata().params.speculativeRetry.calculateThreshold(metric.coordinatorReadLatency); + } + catch (Throwable e) + { + logger.error("Exception caught while calculating speculative retry threshold for {}: {}", metadata(), e); + } + } + public TableStreamManager getStreamManager() { return streamManager; @@ -527,9 +517,6 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean } } - if (latencyCalculator != null) - latencyCalculator.cancel(false); - compactionStrategyManager.shutdown(); SystemKeyspace.removeTruncationRecord(metadata.id); http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/CassandraDaemon.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/CassandraDaemon.java b/src/java/org/apache/cassandra/service/CassandraDaemon.java index 295a33b..95dd6ac 100644 --- a/src/java/org/apache/cassandra/service/CassandraDaemon.java +++ b/src/java/org/apache/cassandra/service/CassandraDaemon.java @@ -407,6 +407,14 @@ public class CassandraDaemon // due to scheduling errors or race conditions ScheduledExecutors.optionalTasks.scheduleWithFixedDelay(ColumnFamilyStore.getBackgroundCompactionTaskSubmitter(), 5, 1, TimeUnit.MINUTES); + // schedule periodic recomputation of speculative retry thresholds + ScheduledExecutors.optionalTasks.scheduleWithFixedDelay( + () -> Keyspace.all().forEach(k -> k.getColumnFamilyStores().forEach(ColumnFamilyStore::updateSpeculationThreshold)), + DatabaseDescriptor.getReadRpcTimeout(), + DatabaseDescriptor.getReadRpcTimeout(), + TimeUnit.MILLISECONDS + ); + // schedule periodic dumps of table size estimates into SystemKeyspace.SIZE_ESTIMATES_CF // set cassandra.size_recorder_interval to 0 to disable int sizeRecorderInterval = Integer.getInteger("cassandra.size_recorder_interval", 5 * 60); http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/AlwaysSpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/AlwaysSpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/AlwaysSpeculativeRetryPolicy.java index d054b09..4623cb1 100644 --- a/src/java/org/apache/cassandra/service/reads/AlwaysSpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/AlwaysSpeculativeRetryPolicy.java @@ -31,12 +31,6 @@ public class AlwaysSpeculativeRetryPolicy implements SpeculativeRetryPolicy } @Override - public boolean isDynamic() - { - return false; - } - - @Override public long calculateThreshold(Timer readLatency) { return 0; http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/FixedSpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/FixedSpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/FixedSpeculativeRetryPolicy.java index fb63ac2..2cd9788 100644 --- a/src/java/org/apache/cassandra/service/reads/FixedSpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/FixedSpeculativeRetryPolicy.java @@ -34,12 +34,6 @@ public class FixedSpeculativeRetryPolicy implements SpeculativeRetryPolicy } @Override - public boolean isDynamic() - { - return false; - } - - @Override public long calculateThreshold(Timer readLatency) { return TimeUnit.MILLISECONDS.toNanos(speculateAtMilliseconds); http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/HybridSpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/HybridSpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/HybridSpeculativeRetryPolicy.java index 78e4fcd..d49cfe4 100644 --- a/src/java/org/apache/cassandra/service/reads/HybridSpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/HybridSpeculativeRetryPolicy.java @@ -43,12 +43,6 @@ public class HybridSpeculativeRetryPolicy implements SpeculativeRetryPolicy } @Override - public boolean isDynamic() - { - return true; - } - - @Override public long calculateThreshold(Timer readLatency) { long percentileThreshold = percentilePolicy.calculateThreshold(readLatency); http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/NeverSpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/NeverSpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/NeverSpeculativeRetryPolicy.java index 219adb5..c46a899 100644 --- a/src/java/org/apache/cassandra/service/reads/NeverSpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/NeverSpeculativeRetryPolicy.java @@ -31,12 +31,6 @@ public class NeverSpeculativeRetryPolicy implements SpeculativeRetryPolicy } @Override - public boolean isDynamic() - { - return false; - } - - @Override public long calculateThreshold(Timer readLatency) { return Long.MAX_VALUE; http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/PercentileSpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/PercentileSpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/PercentileSpeculativeRetryPolicy.java index 6b2cbb0..172cc0c 100644 --- a/src/java/org/apache/cassandra/service/reads/PercentileSpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/PercentileSpeculativeRetryPolicy.java @@ -32,12 +32,6 @@ public class PercentileSpeculativeRetryPolicy implements SpeculativeRetryPolicy } @Override - public boolean isDynamic() - { - return true; - } - - @Override public long calculateThreshold(Timer readLatency) { return (long) readLatency.getSnapshot().getValue(percentile / 100); http://git-wip-us.apache.org/repos/asf/cassandra/blob/8b7e9676/src/java/org/apache/cassandra/service/reads/SpeculativeRetryPolicy.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/reads/SpeculativeRetryPolicy.java b/src/java/org/apache/cassandra/service/reads/SpeculativeRetryPolicy.java index 399f291..225ab26 100644 --- a/src/java/org/apache/cassandra/service/reads/SpeculativeRetryPolicy.java +++ b/src/java/org/apache/cassandra/service/reads/SpeculativeRetryPolicy.java @@ -42,8 +42,6 @@ public interface SpeculativeRetryPolicy Pattern.CASE_INSENSITIVE); public static final SpeculativeRetryPolicy DEFAULT = new PercentileSpeculativeRetryPolicy(99.0); - boolean isDynamic(); - long calculateThreshold(Timer readLatency); Kind kind(); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org