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

Reply via email to