You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by al...@apache.org on 2019/08/02 13:18:44 UTC

[cassandra] 03/09: Address Benedict's review feedback

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

aleksey pushed a commit to branch 15202-4.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git

commit 73261324e27049a95b541d82eeacedf509c9ae48
Author: Aleksey Yeshchenko <al...@apache.org>
AuthorDate: Tue Jul 23 10:28:03 2019 +0100

    Address Benedict's review feedback
---
 .../org/apache/cassandra/utils/FBUtilities.java    |  40 --
 .../org/apache/cassandra/utils/MerkleTree.java     | 410 ++++++++++++---------
 .../org/apache/cassandra/utils/MerkleTrees.java    |   5 +-
 .../repair/asymmetric/DifferenceHolderTest.java    |  10 +-
 .../org/apache/cassandra/utils/MerkleTreeTest.java |  16 +-
 .../apache/cassandra/utils/MerkleTreesTest.java    |  13 +-
 6 files changed, 259 insertions(+), 235 deletions(-)

diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java
index 0f7b2ca..f0d9132 100644
--- a/src/java/org/apache/cassandra/utils/FBUtilities.java
+++ b/src/java/org/apache/cassandra/utils/FBUtilities.java
@@ -265,46 +265,6 @@ public class FBUtilities
         return compareUnsigned(bytes1, bytes2, 0, 0, bytes1.length, bytes2.length);
     }
 
-    /**
-     * @return The bitwise XOR of the inputs. The output will be the same length as the
-     * longer input, but if either input is null, the output will be null.
-     */
-    public static byte[] xor(byte[] left, byte[] right)
-    {
-        if (left == null || right == null)
-            return null;
-        if (left.length > right.length)
-        {
-            byte[] swap = left;
-            left = right;
-            right = swap;
-        }
-
-        // left.length is now <= right.length
-        byte[] out = Arrays.copyOf(right, right.length);
-        for (int i = 0; i < left.length; i++)
-        {
-            out[i] = (byte)((left[i] & 0xFF) ^ (right[i] & 0xFF));
-        }
-        return out;
-    }
-
-    /**
-     * Bitwise XOR of the inputs, in place on the left array
-     *
-     * Assumes inputs are same length
-     */
-    static void xorOntoLeft(byte[] left, byte[] right)
-    {
-        if (left == null || right == null)
-            return;
-
-        assert left.length == right.length;
-
-        for (int i = 0; i < left.length; i++)
-            left[i] = (byte) ((left[i] & 0xFF) ^ (right[i] & 0xFF));
-    }
-
     public static void sortSampledKeys(List<DecoratedKey> keys, Range<Token> range)
     {
         if (range.left.compareTo(range.right) >= 0)
diff --git a/src/java/org/apache/cassandra/utils/MerkleTree.java b/src/java/org/apache/cassandra/utils/MerkleTree.java
index 9d9eadb..2052389 100644
--- a/src/java/org/apache/cassandra/utils/MerkleTree.java
+++ b/src/java/org/apache/cassandra/utils/MerkleTree.java
@@ -44,6 +44,7 @@ import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.utils.concurrent.Ref;
 import org.apache.cassandra.utils.memory.MemoryUtil;
 
+import static java.lang.String.format;
 import static org.apache.cassandra.db.TypeSizes.sizeof;
 import static org.apache.cassandra.utils.ByteBufferUtil.compare;
 
@@ -86,12 +87,6 @@ public class MerkleTree
 
     public static final byte RECOMMENDED_DEPTH = Byte.MAX_VALUE - 1;
 
-    @SuppressWarnings("WeakerAccess")
-    static final int CONSISTENT = 0;
-    static final int FULLY_INCONSISTENT = 1;
-    @SuppressWarnings("WeakerAccess")
-    static final int PARTIALLY_INCONSISTENT = 2;
-
     private final int hashdepth;
 
     /** The top level range that this MerkleTree covers. */
@@ -150,13 +145,6 @@ public class MerkleTree
         size = (long) Math.pow(2, depth);
     }
 
-    public void release()
-    {
-        if (root instanceof OffHeapNode)
-            ((OffHeapNode) root).release();
-        root = null;
-    }
-
     private OnHeapNode initHelper(Token left, Token right, int depth, int max)
     {
         if (depth == max)
@@ -172,6 +160,13 @@ public class MerkleTree
         return new OnHeapInner(midpoint, leftChild, rightChild);
     }
 
+    public void release()
+    {
+        if (root instanceof OffHeapNode)
+            ((OffHeapNode) root).release();
+        root = null;
+    }
+
     public IPartitioner partitioner()
     {
         return partitioner;
@@ -222,7 +217,7 @@ public class MerkleTree
             else
             {
                 logger.debug("Digest mismatch detected, traversing trees [{}, {}]", ltree, rtree);
-                if (FULLY_INCONSISTENT == differenceHelper(ltree, rtree, diff, active))
+                if (Difference.FULLY_INCONSISTENT == differenceHelper(ltree, rtree, diff, active))
                 {
                     logger.debug("Range {} fully inconsistent", active);
                     diff.add(active);
@@ -233,6 +228,8 @@ public class MerkleTree
         return diff;
     }
 
+    enum Difference { CONSISTENT, FULLY_INCONSISTENT, PARTIALLY_INCONSISTENT }
+
     /**
      * TODO: This function could be optimized into a depth first traversal of the two trees in parallel.
      *
@@ -240,10 +237,10 @@ public class MerkleTree
      * @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)
+    static Difference differenceHelper(MerkleTree ltree, MerkleTree rtree, List<TreeRange> diff, TreeRange active)
     {
         if (active.depth == Byte.MAX_VALUE)
-            return CONSISTENT;
+            return Difference.CONSISTENT;
 
         Token midpoint = ltree.partitioner().midpoint(active.left, active.right);
         // sanity check for midpoint calculation, see CASSANDRA-13052
@@ -252,7 +249,7 @@ public class MerkleTree
             // 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;
+            return Difference.FULLY_INCONSISTENT;
         }
 
         TreeRange left = new TreeRange(active.left, midpoint, active.depth + 1);
@@ -264,13 +261,13 @@ public class MerkleTree
         lnode = ltree.find(left);
         rnode = rtree.find(left);
 
-        int ldiff = CONSISTENT;
+        Difference ldiff = Difference.CONSISTENT;
         if (lnode.hashesDiffer(rnode))
         {
             logger.debug("({}) Inconsistent digest on left sub-range {}: [{}, {}]", active.depth, left, lnode, rnode);
 
             if (lnode instanceof Leaf)
-                ldiff = FULLY_INCONSISTENT;
+                ldiff = Difference.FULLY_INCONSISTENT;
             else
                 ldiff = differenceHelper(ltree, rtree, diff, left);
         }
@@ -279,118 +276,37 @@ public class MerkleTree
         lnode = ltree.find(right);
         rnode = rtree.find(right);
 
-        int rdiff = CONSISTENT;
+        Difference rdiff = Difference.CONSISTENT;
         if (lnode.hashesDiffer(rnode))
         {
             logger.debug("({}) Inconsistent digest on right sub-range {}: [{}, {}]", active.depth, right, lnode, rnode);
 
             if (rnode instanceof Leaf)
-                rdiff = FULLY_INCONSISTENT;
+                rdiff = Difference.FULLY_INCONSISTENT;
             else
                 rdiff = differenceHelper(ltree, rtree, diff, right);
         }
 
-        if (ldiff == FULLY_INCONSISTENT && rdiff == FULLY_INCONSISTENT)
+        if (ldiff == Difference.FULLY_INCONSISTENT && rdiff == Difference.FULLY_INCONSISTENT)
         {
             // both children are fully inconsistent
             logger.debug("({}) Fully inconsistent range [{}, {}]", active.depth, left, right);
-            return FULLY_INCONSISTENT;
+            return Difference.FULLY_INCONSISTENT;
         }
-        else if (ldiff == FULLY_INCONSISTENT)
+        else if (ldiff == Difference.FULLY_INCONSISTENT)
         {
             logger.debug("({}) Adding left sub-range to diff as fully inconsistent {}", active.depth, left);
             diff.add(left);
-            return PARTIALLY_INCONSISTENT;
+            return Difference.PARTIALLY_INCONSISTENT;
         }
-        else if (rdiff == FULLY_INCONSISTENT)
+        else if (rdiff == Difference.FULLY_INCONSISTENT)
         {
             logger.debug("({}) Adding right sub-range to diff as fully inconsistent {}", active.depth, right);
             diff.add(right);
-            return PARTIALLY_INCONSISTENT;
+            return Difference.PARTIALLY_INCONSISTENT;
         }
         logger.debug("({}) Range {} partially inconstent", active.depth, active);
-        return PARTIALLY_INCONSISTENT;
-    }
-
-    /**
-     * For testing purposes.
-     * Gets the smallest range containing the token.
-     */
-    public TreeRange get(Token t)
-    {
-        return getHelper(root, fullRange.left, fullRange.right, t);
-    }
-
-    private TreeRange getHelper(Node node, Token pleft, Token pright, Token t)
-    {
-        int depth = 0;
-
-        while (true)
-        {
-            if (node instanceof Leaf)
-            {
-                // we've reached a hash: wrap it up and deliver it
-                return new TreeRange(this, pleft, pright, depth, node);
-            }
-
-            assert node instanceof Inner;
-            Inner inner = (Inner) node;
-
-            if (Range.contains(pleft, inner.token(), t)) // left child contains token
-            {
-                pright = inner.token();
-                node = inner.left();
-            }
-            else // right child contains token
-            {
-                pleft = inner.token();
-                node = inner.right();
-            }
-
-            depth++;
-        }
-    }
-
-    /**
-     * Invalidates the ranges containing the given token.
-     * Useful for testing.
-     */
-    public void invalidate(Token t)
-    {
-        invalidateHelper(root, fullRange.left, t);
-    }
-
-    private void invalidateHelper(Node node, Token pleft, Token t)
-    {
-        node.hash(EMPTY_HASH);
-
-        if (node instanceof Leaf)
-            return;
-
-        assert node instanceof Inner;
-        Inner inner = (Inner) node;
-        // TODO: reset computed flag on OnHeapInners
-
-        if (Range.contains(pleft, inner.token(), t))
-            invalidateHelper(inner.left(), pleft, t); // left child contains token
-        else
-            invalidateHelper(inner.right(), inner.token(), t); // right child contains token
-    }
-
-    /**
-     * Hash the given range in the tree. The range must have been generated
-     * with recursive applications of partitioner.midpoint().
-     *
-     * NB: Currently does not support wrapping ranges that do not end with
-     * partitioner.getMinimumToken().
-     *
-     * @return {@link #EMPTY_HASH} if any subrange of the range is invalid, or if the exact
-     *         range cannot be calculated using this tree.
-     */
-    @VisibleForTesting
-    public byte[] hash(Range<Token> range)
-    {
-        return find(range).hash();
+        return Difference.PARTIALLY_INCONSISTENT;
     }
 
     /**
@@ -409,46 +325,19 @@ public class MerkleTree
      * @param range Range to find
      * @return {@link Node} found. If nothing found, return {@link Leaf} with empty hash.
      */
-    private Node find(Range<Token> range)
-    {
-        try
-        {
-            return findHelper(root, new Range<>(fullRange.left, fullRange.right), range);
-        }
-        catch (StopRecursion e)
-        {
-            return new OnHeapLeaf();
-        }
-    }
-
-    interface Consumer<E extends Exception>
-    {
-        void accept(Node node) throws E;
-    }
-
     @VisibleForTesting
-    <E extends Exception> boolean ifHashesRange(Range<Token> range, Consumer<E> consumer) throws E
+    Node find(Range<Token> range)
     {
         try
         {
-            Node node = findHelper(root, new Range<>(fullRange.left, fullRange.right), range);
-            boolean hasHash = !node.hasEmptyHash();
-            if (hasHash)
-                consumer.accept(node);
-            return hasHash;
+            return findHelper(root, fullRange, range);
         }
         catch (StopRecursion e)
         {
-            return false;
+            return new OnHeapLeaf();
         }
     }
 
-    @VisibleForTesting
-    boolean hashesRange(Range<Token> range)
-    {
-        return ifHashesRange(range, n -> {});
-    }
-
     /**
      * @throws StopRecursion If no match could be found for the range.
      */
@@ -812,18 +701,33 @@ public class MerkleTree
         return new MerkleTree(root, partitioner, fullRange, hashDepth, maxSize, innerNodeCount);
     }
 
-    private static boolean warnedOnce;
-    private static Node deserializeTree(DataInputPlus in, IPartitioner partitioner, int innerNodeCount, boolean offHeapRequested, int version) throws IOException
+    private static boolean shouldUseOffHeapTrees(IPartitioner partitioner, boolean offHeapRequested)
     {
         boolean offHeapSupported = partitioner instanceof Murmur3Partitioner || partitioner instanceof RandomPartitioner;
 
         if (offHeapRequested && !offHeapSupported && !warnedOnce)
         {
-            logger.warn("Configuration requests offheap memtables, but partitioner does not support it. Ignoring.");
+            logger.warn("Configuration requests off-heap merkle trees, but partitioner does not support it. Ignoring.");
             warnedOnce = true;
         }
 
-        return offHeapRequested && offHeapSupported
+        return offHeapRequested && offHeapSupported;
+    }
+    private static boolean warnedOnce;
+
+    private static ByteBuffer allocate(int innerNodeCount, IPartitioner partitioner)
+    {
+        int size = offHeapBufferSize(innerNodeCount, partitioner);
+        logger.debug("Allocating direct buffer of size {} for an off-heap merkle tree", size);
+        ByteBuffer buffer = ByteBuffer.allocateDirect(size);
+        if (Ref.DEBUG_ENABLED)
+            MemoryUtil.setAttachment(buffer, new Ref<>(null, null));
+        return buffer;
+    }
+
+    private static Node deserializeTree(DataInputPlus in, IPartitioner partitioner, int innerNodeCount, boolean offHeapRequested, int version) throws IOException
+    {
+        return shouldUseOffHeapTrees(partitioner, offHeapRequested)
              ? deserializeOffHeap(in, partitioner, innerNodeCount, version)
              : OnHeapNode.deserialize(in, partitioner, version);
     }
@@ -833,39 +737,27 @@ public class MerkleTree
      * On the deserialization path, we know in advance what the tree looks like,
      * So we can pre-size an offheap buffer and deserialize into that.
      */
-
     MerkleTree tryMoveOffHeap() throws IOException
     {
-        boolean offHeapEnabled = DatabaseDescriptor.getOffheapMerkleTreesEnabled();
-        boolean offHeapSupported = partitioner instanceof Murmur3Partitioner || partitioner instanceof RandomPartitioner;
-
-        if (offHeapEnabled && !offHeapSupported && !warnedOnce)
-        {
-            logger.warn("Configuration requests offheap memtables, but partitioner does not support it. Ignoring.");
-            warnedOnce = true;
-        }
-
-        return root instanceof OnHeapNode && offHeapEnabled && offHeapSupported ? moveOffHeap() : this;
+        return root instanceof OnHeapNode && shouldUseOffHeapTrees(partitioner, DatabaseDescriptor.getOffheapMerkleTreesEnabled())
+             ? moveOffHeap()
+             : this;
     }
 
     private MerkleTree moveOffHeap() throws IOException
     {
         assert root instanceof OnHeapNode;
-        int bufferSize = offHeapBufferSize(Ints.checkedCast(size), partitioner);
-        logger.debug("Allocating direct buffer of size {} to move merkle tree off heap", bufferSize);
-        ByteBuffer buffer = ByteBuffer.allocateDirect(bufferSize);
+        ByteBuffer buffer = allocate(Ints.checkedCast(size), partitioner);
         int pointer = ((OnHeapNode) root).serializeOffHeap(buffer, partitioner);
-        OffHeapNode newRoot = fromPointer(pointer, buffer, partitioner).attachRef();
+        OffHeapNode newRoot = fromPointer(pointer, buffer, partitioner);
         return new MerkleTree(newRoot, partitioner, fullRange, hashdepth, maxsize, size);
     }
 
     private static OffHeapNode deserializeOffHeap(DataInputPlus in, IPartitioner partitioner, int innerNodeCount, int version) throws IOException
     {
-        int bufferSize = offHeapBufferSize(innerNodeCount, partitioner);
-        logger.debug("Allocating direct buffer of size {} for merkle tree deserialization", bufferSize);
-        ByteBuffer buffer = ByteBuffer.allocateDirect(bufferSize);
+        ByteBuffer buffer = allocate(innerNodeCount, partitioner);
         int pointer = OffHeapNode.deserialize(in, buffer, partitioner, version);
-        return fromPointer(pointer, buffer, partitioner).attachRef();
+        return fromPointer(pointer, buffer, partitioner);
     }
 
     private static OffHeapNode fromPointer(int pointer, ByteBuffer buffer, IPartitioner partitioner)
@@ -1059,13 +951,6 @@ public class MerkleTree
             return false;
         }
 
-        OffHeapNode attachRef()
-        {
-            if (Ref.DEBUG_ENABLED)
-                MemoryUtil.setAttachment(buffer, new Ref<>(this, null));
-            return this;
-        }
-
         void release()
         {
             Object attachment = MemoryUtil.getAttachment(buffer);
@@ -1160,7 +1045,7 @@ public class MerkleTree
             if (hasEmptyHash())
                 hash(partitionHash);
             else
-                FBUtilities.xorOntoLeft(hash, partitionHash);
+                xorOntoLeft(hash, partitionHash);
 
             sizeOfRange += partitionSize;
             partitionsInRange += 1;
@@ -1168,15 +1053,17 @@ public class MerkleTree
 
         static OnHeapLeaf deserializeWithoutIdent(DataInputPlus in) throws IOException
         {
-            if (in.readByte() > 0)
+            int size = in.readByte();
+            switch (size)
             {
-                byte[] hash = new byte[HASH_SIZE];
-                in.readFully(hash);
-                return new OnHeapLeaf(hash);
-            }
-            else
-            {
-                return new OnHeapLeaf();
+                case HASH_SIZE:
+                    byte[] hash = new byte[HASH_SIZE];
+                    in.readFully(hash);
+                    return new OnHeapLeaf(hash);
+                case 0:
+                    return new OnHeapLeaf();
+                default:
+                    throw new IllegalStateException(format("Hash of size %d encountered, expecting %d or %d", size, HASH_SIZE, 0));
             }
         }
 
@@ -1318,6 +1205,10 @@ public class MerkleTree
             Inner that = (Inner) other;
             return !hashesDiffer(other) && this.left().equals(that.left()) && this.right().equals(that.right());
         }
+
+        default void unsafeInvalidate()
+        {
+        }
     }
 
     static class OnHeapInner extends OnHeapNode implements Inner
@@ -1372,7 +1263,7 @@ public class MerkleTree
                 right.compute();
 
                 if (!left.hasEmptyHash() && !right.hasEmptyHash())
-                    hash = FBUtilities.xor(left.hash(), right.hash());
+                    hash = xor(left.hash(), right.hash());
                 else if (left.hasEmptyHash())
                     hash = right.hash();
                 else if (right.hasEmptyHash())
@@ -1432,6 +1323,12 @@ public class MerkleTree
             toString(buff, 1);
             return buff.toString();
         }
+
+        @Override
+        public void unsafeInvalidate()
+        {
+            computed = false;
+        }
     }
 
     static class OffHeapInner extends OffHeapNode implements Inner
@@ -1468,13 +1365,17 @@ public class MerkleTree
 
         public Node left()
         {
-            int pointer = buffer.getInt(offset + LEFT_CHILD_POINTER_OFFSET);
-            return pointer >= 0 ? new OffHeapInner(buffer, pointer, partitioner) : new OffHeapLeaf(buffer, ~pointer);
+            return child(LEFT_CHILD_POINTER_OFFSET);
         }
 
         public Node right()
         {
-            int pointer = buffer.getInt(offset + RIGHT_CHILD_POINTER_OFFSET);
+            return child(RIGHT_CHILD_POINTER_OFFSET);
+        }
+
+        private Node child(int childOffset)
+        {
+            int pointer = buffer.getInt(offset + childOffset);
             return pointer >= 0 ? new OffHeapInner(buffer, pointer, partitioner) : new OffHeapLeaf(buffer, ~pointer);
         }
 
@@ -1539,6 +1440,30 @@ public class MerkleTree
     }
 
     /**
+     * @return The bitwise XOR of the inputs.
+     */
+    static byte[] xor(byte[] left, byte[] right)
+    {
+        assert left.length == right.length;
+
+        byte[] out = Arrays.copyOf(right, right.length);
+        for (int i = 0; i < left.length; i++)
+            out[i] = (byte)((left[i] & 0xFF) ^ (right[i] & 0xFF));
+        return out;
+    }
+
+    /**
+     * Bitwise XOR of the inputs, in place on the left array.
+     */
+    private static void xorOntoLeft(byte[] left, byte[] right)
+    {
+        assert left.length == right.length;
+
+        for (int i = 0; i < left.length; i++)
+            left[i] = (byte) ((left[i] & 0xFF) ^ (right[i] & 0xFF));
+    }
+
+    /**
      * Estimate the allowable depth while keeping the resulting heap usage of this tree under the provided
      * number of bytes. This is important for ensuring that we do not allocate overly large trees that could
      * OOM the JVM and cause instability.
@@ -1583,4 +1508,125 @@ public class MerkleTree
         long adjustedBytes = Math.max(1, (numBytes + sizeOfInner) / (sizeOfLeaf + sizeOfInner));
         return Math.max(1, (int) Math.floor(Math.log(adjustedBytes) / Math.log(2)));
     }
+
+    /*
+     * Test-only methods.
+     */
+
+    /**
+     * Invalidates the ranges containing the given token.
+     * Useful for testing.
+     */
+    @VisibleForTesting
+    void unsafeInvalidate(Token t)
+    {
+        unsafeInvalidateHelper(root, fullRange.left, t);
+    }
+
+    private void unsafeInvalidateHelper(Node node, Token pleft, Token t)
+    {
+        node.hash(EMPTY_HASH);
+
+        if (node instanceof Leaf)
+            return;
+
+        assert node instanceof Inner;
+        Inner inner = (Inner) node;
+        inner.unsafeInvalidate();
+
+        if (Range.contains(pleft, inner.token(), t))
+            unsafeInvalidateHelper(inner.left(), pleft, t); // left child contains token
+        else
+            unsafeInvalidateHelper(inner.right(), inner.token(), t); // right child contains token
+    }
+
+    /**
+     * Hash the given range in the tree. The range must have been generated
+     * with recursive applications of partitioner.midpoint().
+     *
+     * NB: Currently does not support wrapping ranges that do not end with
+     * partitioner.getMinimumToken().
+     *
+     * @return {@link #EMPTY_HASH} if any subrange of the range is invalid, or if the exact
+     *         range cannot be calculated using this tree.
+     */
+    @VisibleForTesting
+    byte[] hash(Range<Token> range)
+    {
+        return find(range).hash();
+    }
+
+    interface Consumer<E extends Exception>
+    {
+        void accept(Node node) throws E;
+    }
+
+    @VisibleForTesting
+    <E extends Exception> boolean ifHashesRange(Range<Token> range, Consumer<E> consumer) throws E
+    {
+        try
+        {
+            Node node = findHelper(root, new Range<>(fullRange.left, fullRange.right), range);
+            boolean hasHash = !node.hasEmptyHash();
+            if (hasHash)
+                consumer.accept(node);
+            return hasHash;
+        }
+        catch (StopRecursion e)
+        {
+            return false;
+        }
+    }
+
+    @VisibleForTesting
+    boolean hashesRange(Range<Token> range)
+    {
+        return ifHashesRange(range, n -> {});
+    }
+
+    /**
+     * For testing purposes.
+     * Gets the smallest range containing the token.
+     */
+    @VisibleForTesting
+    public TreeRange get(Token t)
+    {
+        return getHelper(root, fullRange.left, fullRange.right, t);
+    }
+
+    private TreeRange getHelper(Node node, Token pleft, Token pright, Token t)
+    {
+        int depth = 0;
+
+        while (true)
+        {
+            if (node instanceof Leaf)
+            {
+                // we've reached a hash: wrap it up and deliver it
+                return new TreeRange(this, pleft, pright, depth, node);
+            }
+
+            assert node instanceof Inner;
+            Inner inner = (Inner) node;
+
+            if (Range.contains(pleft, inner.token(), t)) // left child contains token
+            {
+                pright = inner.token();
+                node = inner.left();
+            }
+            else // right child contains token
+            {
+                pleft = inner.token();
+                node = inner.right();
+            }
+
+            depth++;
+        }
+    }
+
+    @VisibleForTesting
+    void compute()
+    {
+        root.compute();
+    }
 }
diff --git a/src/java/org/apache/cassandra/utils/MerkleTrees.java b/src/java/org/apache/cassandra/utils/MerkleTrees.java
index 9cf04c5..0043fe0 100644
--- a/src/java/org/apache/cassandra/utils/MerkleTrees.java
+++ b/src/java/org/apache/cassandra/utils/MerkleTrees.java
@@ -147,7 +147,8 @@ public class MerkleTrees implements Iterable<Map.Entry<Range<Token>, MerkleTree>
      */
     public void release()
     {
-        merkleTrees.values().forEach(MerkleTree::release); merkleTrees.clear();
+        merkleTrees.values().forEach(MerkleTree::release);
+        merkleTrees.clear();
     }
 
     /**
@@ -179,7 +180,7 @@ public class MerkleTrees implements Iterable<Map.Entry<Range<Token>, MerkleTree>
     @VisibleForTesting
     public void invalidate(Token t)
     {
-        getMerkleTree(t).invalidate(t);
+        getMerkleTree(t).unsafeInvalidate(t);
     }
 
     /**
diff --git a/test/unit/org/apache/cassandra/repair/asymmetric/DifferenceHolderTest.java b/test/unit/org/apache/cassandra/repair/asymmetric/DifferenceHolderTest.java
index de69cd7..8881018 100644
--- a/test/unit/org/apache/cassandra/repair/asymmetric/DifferenceHolderTest.java
+++ b/test/unit/org/apache/cassandra/repair/asymmetric/DifferenceHolderTest.java
@@ -31,6 +31,7 @@ import org.apache.cassandra.dht.Range;
 import org.apache.cassandra.dht.Token;
 import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.repair.TreeResponse;
+import org.apache.cassandra.utils.HashingUtils;
 import org.apache.cassandra.utils.MerkleTree;
 import org.apache.cassandra.utils.MerkleTrees;
 import org.apache.cassandra.utils.MerkleTreesTest;
@@ -40,6 +41,11 @@ import static org.junit.Assert.assertTrue;
 
 public class DifferenceHolderTest
 {
+    private static byte[] digest(String string)
+    {
+        return HashingUtils.newMessageDigest("SHA-256").digest(string.getBytes());
+    }
+
     @Test
     public void testFromEmptyMerkleTrees() throws UnknownHostException
     {
@@ -91,8 +97,8 @@ public class DifferenceHolderTest
 
         // set the hashes for the leaf of the created split
         middle = mt1.get(leftmost.right);
-        middle.hash("arbitrary!".getBytes());
-        mt1.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash("even more arbitrary!".getBytes());
+        middle.hash(digest("arbitrary!"));
+        mt1.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash(digest("even more arbitrary!"));
 
         TreeResponse tr1 = new TreeResponse(a1, mt1);
         TreeResponse tr2 = new TreeResponse(a2, mt2);
diff --git a/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java b/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
index d8491d0..6ce2652 100644
--- a/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
+++ b/test/unit/org/apache/cassandra/utils/MerkleTreeTest.java
@@ -46,7 +46,12 @@ import static org.junit.Assert.*;
 
 public class MerkleTreeTest
 {
-    private static final byte[] DUMMY = HashingUtils.newMessageDigest("SHA-256").digest("dummy".getBytes());
+    private static final byte[] DUMMY = digest("dummy");
+
+    private static byte[] digest(String string)
+    {
+        return HashingUtils.newMessageDigest("SHA-256").digest(string.getBytes());
+    }
 
     /**
      * If a test assumes that the tree is 8 units wide, then it should set this value
@@ -452,8 +457,8 @@ public class MerkleTreeTest
 
         // set the hashes for the leaf of the created split
         middle = mt.get(leftmost.right);
-        middle.hash("arbitrary!".getBytes());
-        mt.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash("even more arbitrary!".getBytes());
+        middle.hash(digest("arbitrary!"));
+        mt.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash(digest("even more arbitrary!"));
 
         // trees should disagree for (leftmost.left, middle.right]
         List<TreeRange> diffs = MerkleTree.difference(mt, mt2);
@@ -491,7 +496,8 @@ 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.TreeRange(ltree.fullRange.left, ltree.fullRange.right, (byte)0)));
+        assertEquals(MerkleTree.Difference.FULLY_INCONSISTENT,
+                     MerkleTree.differenceHelper(ltree, rtree, new ArrayList<>(), new MerkleTree.TreeRange(ltree.fullRange.left, ltree.fullRange.right, (byte)0)));
     }
 
     /**
@@ -548,7 +554,7 @@ public class MerkleTreeTest
             while (depth.equals(dstack.peek()))
             {
                 // consume the stack
-                hash = FBUtilities.xor(hstack.pop(), hash);
+                hash = MerkleTree.xor(hstack.pop(), hash);
                 depth = dstack.pop() - 1;
             }
             dstack.push(depth);
diff --git a/test/unit/org/apache/cassandra/utils/MerkleTreesTest.java b/test/unit/org/apache/cassandra/utils/MerkleTreesTest.java
index a1f6068..9e70c20 100644
--- a/test/unit/org/apache/cassandra/utils/MerkleTreesTest.java
+++ b/test/unit/org/apache/cassandra/utils/MerkleTreesTest.java
@@ -42,7 +42,12 @@ import static org.junit.Assert.*;
 
 public class MerkleTreesTest
 {
-    private static final byte[] DUMMY = HashingUtils.newMessageDigest("SHA-256").digest("dummy".getBytes());
+    private static final byte[] DUMMY = digest("dummy");
+
+    private static byte[] digest(String string)
+    {
+        return HashingUtils.newMessageDigest("SHA-256").digest(string.getBytes());
+    }
 
     /**
      * If a test assumes that the tree is 8 units wide, then it should set this value
@@ -469,8 +474,8 @@ public class MerkleTreesTest
 
         // set the hashes for the leaf of the created split
         middle = mts.get(leftmost.right);
-        middle.hash("arbitrary!".getBytes());
-        mts.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash("even more arbitrary!".getBytes());
+        middle.hash(digest("arbitrary!"));
+        mts.get(partitioner.midpoint(leftmost.left, leftmost.right)).hash(digest("even more arbitrary!"));
 
         // trees should disagree for (leftmost.left, middle.right]
         List<Range<Token>> diffs = MerkleTrees.difference(mts, mts2);
@@ -499,7 +504,7 @@ public class MerkleTreesTest
             while (depth.equals(dstack.peek()))
             {
                 // consume the stack
-                hash = FBUtilities.xor(hstack.pop(), hash);
+                hash = MerkleTree.xor(hstack.pop(), hash);
                 depth = dstack.pop()-1;
             }
             dstack.push(depth);


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