Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.0 0493545dd -> bc5c2316c
  refs/heads/cassandra-3.11 3f5f70281 -> 90b39dc61
  refs/heads/trunk fa36044a5 -> dcef7c7ed


Change repair midpoint logging for tiny ranges

Patch by Jeff Jirsa; Reviewed by Blake Eggleston for CASSANDRA-13603


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/bc5c2316
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/bc5c2316
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/bc5c2316

Branch: refs/heads/cassandra-3.0
Commit: bc5c2316c5acfc1ee0ef101a3ebf00d03c89e283
Parents: 0493545
Author: Jeff Jirsa <jji...@apple.com>
Authored: Tue Sep 5 11:57:58 2017 -0700
Committer: Jeff Jirsa <jji...@apple.com>
Committed: Tue Sep 5 11:57:58 2017 -0700

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../org/apache/cassandra/utils/MerkleTree.java  | 23 +++++++++----
 .../apache/cassandra/utils/MerkleTreeTest.java  | 34 ++++++++++++++++++++
 3 files changed, 51 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc5c2316/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index a3eccf2..4302fdf 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.15
+ * Change repair midpoint logging for tiny ranges (CASSANDRA-13603)
  * Better handle corrupt final commitlog segment (CASSANDRA-11995)
  * StreamingHistogram is not thread safe (CASSANDRA-13756)
  * Fix MV timestamp issues (CASSANDRA-11500)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc5c2316/src/java/org/apache/cassandra/utils/MerkleTree.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/MerkleTree.java 
b/src/java/org/apache/cassandra/utils/MerkleTree.java
index 0d5a469..22b61e8 100644
--- a/src/java/org/apache/cassandra/utils/MerkleTree.java
+++ b/src/java/org/apache/cassandra/utils/MerkleTree.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.util.*;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.PeekingIterator;
 
@@ -246,12 +247,20 @@ public class MerkleTree implements Serializable
 
         if (lhash != null && rhash != null && !Arrays.equals(lhash, rhash))
         {
-            logger.debug("Digest mismatch detected, traversing trees [{}, 
{}]", ltree, rtree);
-            if (FULLY_INCONSISTENT == differenceHelper(ltree, rtree, diff, 
active))
+            if(lnode instanceof  Leaf || rnode instanceof Leaf)
             {
-                logger.debug("Range {} fully inconsistent", active);
+                logger.debug("Digest mismatch detected among leaf nodes {}, 
{}", lnode, rnode);
                 diff.add(active);
             }
+            else
+            {
+                logger.debug("Digest mismatch detected, traversing trees [{}, 
{}]", ltree, rtree);
+                if (FULLY_INCONSISTENT == differenceHelper(ltree, rtree, diff, 
active))
+                {
+                    logger.debug("Range {} fully inconsistent", active);
+                    diff.add(active);
+                }
+            }
         }
         else if (lhash == null || rhash == null)
             diff.add(active);
@@ -265,6 +274,7 @@ public class MerkleTree implements Serializable
      * Takes two trees and a range for which they have hashes, but are 
inconsistent.
      * @return FULLY_INCONSISTENT if active is inconsistent, 
PARTIALLY_INCONSISTENT if only a subrange is inconsistent.
      */
+    @VisibleForTesting
     static int differenceHelper(MerkleTree ltree, MerkleTree rtree, 
List<TreeRange> diff, TreeRange active)
     {
         if (active.depth == Byte.MAX_VALUE)
@@ -274,10 +284,9 @@ public class MerkleTree implements Serializable
         // sanity check for midpoint calculation, see CASSANDRA-13052
         if (midpoint.equals(active.left) || midpoint.equals(active.right))
         {
-            // Unfortunately we can't throw here to abort the validation 
process, as the code is executed in it's own
-            // thread with the caller waiting for a condition to be signaled 
after completion and without an option
-            // to indicate an error (2.x only).
-            logger.error("Invalid midpoint {} for [{},{}], range will be 
reported inconsistent", midpoint, active.left, active.right);
+            // If the midpoint equals either the left or the right, we have a 
range that's too small to split - we'll simply report the
+            // whole range as inconsistent
+            logger.debug("({}) No sane midpoint ({}) for range {} , marking 
whole range as inconsistent", active.depth, midpoint, active);
             return FULLY_INCONSISTENT;
         }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc5c2316/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java 
b/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
index c9aa09f..64aea24 100644
--- a/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
+++ b/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
@@ -472,6 +472,40 @@ public class MerkleTreeTest
 
         List<TreeRange> diffs = MerkleTree.difference(ltree, rtree);
         assertEquals(Lists.newArrayList(range), diffs);
+        assertEquals(MerkleTree.FULLY_INCONSISTENT, 
MerkleTree.differenceHelper(ltree, rtree, new ArrayList<>(), new 
MerkleTree.TreeDifference(ltree.fullRange.left, ltree.fullRange.right, 
(byte)0)));
+    }
+
+    /**
+     * matching should behave as expected, even with extremely small ranges
+     */
+    @Test
+    public void matchingSmallRange()
+    {
+        Token start = new BigIntegerToken("9");
+        Token end = new BigIntegerToken("10");
+        Range<Token> range = new Range<>(start, end);
+
+        MerkleTree ltree = new MerkleTree(partitioner, range, 
RECOMMENDED_DEPTH, 16);
+        ltree.init();
+        MerkleTree rtree = new MerkleTree(partitioner, range, 
RECOMMENDED_DEPTH, 16);
+        rtree.init();
+
+        byte[] h1 = "asdf".getBytes();
+        byte[] h2 = "asdf".getBytes();
+
+
+        // add dummy hashes to both trees
+        for (TreeRange tree : ltree.invalids())
+        {
+            tree.addHash(new RowHash(range.right, h1, h1.length));
+        }
+        for (TreeRange tree : rtree.invalids())
+        {
+            tree.addHash(new RowHash(range.right, h2, h2.length));
+        }
+
+        // top level difference() should show no differences
+        assertEquals(MerkleTree.difference(ltree, rtree), 
Lists.newArrayList());
     }
 
     /**


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

Reply via email to