[cassandra] branch trunk updated: Only track ideal CL failure when request CL met

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rustyrazorblade pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 0e0d288  Only track ideal CL failure when request CL met
0e0d288 is described below

commit 0e0d288ab7e87e7d4a7542c955dd06701798bd06
Author: Jon Haddad jon@jonhaddad.com <jo...@jonhaddad.com>
AuthorDate: Mon Apr 6 12:53:27 2020 -0700

    Only track ideal CL failure when request CL met
    
    Ideal consistency level tracking should not report a failure when requested CL
    was also not met either.
    
    Patch by Jon Haddad; Reviewed by Dinesh Joshi for CASSANDRA-15696.
---
 CHANGES.txt                                        |  1 +
 .../service/AbstractWriteResponseHandler.java      | 17 ++++++++++++--
 .../service/WriteResponseHandlerTest.java          | 26 ++++++++++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index fb881de..95a6802 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0-alpha4
+ * Only track ideal CL failure when request CL met (CASSANDRA-15696)
  * Fix flaky CoordinatorMessagingTest and docstring in OutboundSink and ConsistentSession (CASSANDRA-15672)
  * Fix force compaction of wrapping ranges (CASSANDRA-15664)
  * Expose repair streaming metrics (CASSANDRA-15656)
diff --git a/src/java/org/apache/cassandra/service/AbstractWriteResponseHandler.java b/src/java/org/apache/cassandra/service/AbstractWriteResponseHandler.java
index 1889c79..b1eb5b3 100644
--- a/src/java/org/apache/cassandra/service/AbstractWriteResponseHandler.java
+++ b/src/java/org/apache/cassandra/service/AbstractWriteResponseHandler.java
@@ -74,6 +74,11 @@ public abstract class AbstractWriteResponseHandler<T> implements RequestCallback
     private AbstractWriteResponseHandler idealCLDelegate;
 
     /**
+     * We don't want to increment the writeFailedIdealCL if we didn't achieve the original requested CL
+     */
+    private boolean requestedCLAchieved = false;
+
+    /**
      * @param callback           A callback to be called when the write is successful.
      * @param queryStartNanoTime
      */
@@ -232,6 +237,13 @@ public abstract class AbstractWriteResponseHandler<T> implements RequestCallback
 
     protected void signal()
     {
+        //The ideal CL should only count as a strike if the requested CL was achieved.
+        //If the requested CL is not achieved it's fine for the ideal CL to also not be achieved.
+        if (idealCLDelegate != null)
+        {
+            idealCLDelegate.requestedCLAchieved = true;
+        }
+
         condition.signalAll();
         if (callback != null)
             callback.run();
@@ -279,8 +291,9 @@ public abstract class AbstractWriteResponseHandler<T> implements RequestCallback
         int decrementedValue = responsesAndExpirations.decrementAndGet();
         if (decrementedValue == 0)
         {
-            //The condition being signaled is a valid proxy for the CL being achieved
-            if (!condition.isSignaled())
+            // The condition being signaled is a valid proxy for the CL being achieved
+            // Only mark it as failed if the requested CL was achieved.
+            if (!condition.isSignaled() && requestedCLAchieved)
             {
                 replicaPlan.keyspace().metric.writeFailedIdealCL.inc();
             }
diff --git a/test/unit/org/apache/cassandra/service/WriteResponseHandlerTest.java b/test/unit/org/apache/cassandra/service/WriteResponseHandlerTest.java
index f06b706..5d8d191 100644
--- a/test/unit/org/apache/cassandra/service/WriteResponseHandlerTest.java
+++ b/test/unit/org/apache/cassandra/service/WriteResponseHandlerTest.java
@@ -232,6 +232,32 @@ public class WriteResponseHandlerTest
         assertEquals(0, ks.metric.idealCLWriteLatency.totalLatency.getCount());
     }
 
+    /**
+     * Validate that failing to achieve ideal CL doesn't increase the failure counter when not meeting CL
+     * @throws Throwable
+     */
+    @Test
+    public void failedIdealCLDoesNotIncrementsStatOnQueryFailure() throws Throwable
+    {
+        AbstractWriteResponseHandler awr = createWriteResponseHandler(ConsistencyLevel.LOCAL_QUORUM, ConsistencyLevel.EACH_QUORUM);
+
+        long startingCount = ks.metric.writeFailedIdealCL.getCount();
+
+        // Failure in local DC
+        awr.onResponse(createDummyMessage(0));
+        
+        awr.expired();
+        awr.expired();
+
+        //Fail in remote DC
+        awr.expired();
+        awr.expired();
+        awr.expired();
+
+        assertEquals(startingCount, ks.metric.writeFailedIdealCL.getCount());
+    }
+
+
     private static AbstractWriteResponseHandler createWriteResponseHandler(ConsistencyLevel cl, ConsistencyLevel ideal)
     {
         return createWriteResponseHandler(cl, ideal, System.nanoTime());


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org