Stop merkle tree recursion at leafs

patch by Stefan Podkowinski; reviewed by Blake Eggleston for CASSANDRA-13052
This commit is contained in:
Stefan Podkowinski 2016-12-20 14:40:18 +01:00
parent 7db6a3b3cd
commit 3872580245
5 changed files with 79 additions and 4 deletions

View File

@ -1,4 +1,5 @@
3.0.14
* Fix repair process violating start/end token limits for small ranges (CASSANDRA-13052)
* Add storage port options to sstableloader (CASSANDRA-13518)
* Properly handle quoted index names in cqlsh DESCRIBE output (CASSANDRA-12847)
* Avoid reading static row twice from old format sstables (CASSANDRA-13236)

View File

@ -158,6 +158,10 @@ public class RepairOption
}
Token parsedBeginToken = partitioner.getTokenFactory().fromString(rangeStr[0].trim());
Token parsedEndToken = partitioner.getTokenFactory().fromString(rangeStr[1].trim());
if (parsedBeginToken.equals(parsedEndToken))
{
throw new IllegalArgumentException("Start and end tokens must be different.");
}
ranges.add(new Range<>(parsedBeginToken, parsedEndToken));
}
}

View File

@ -25,6 +25,9 @@ import java.util.*;
import com.google.common.base.Preconditions;
import com.google.common.collect.PeekingIterator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.db.TypeSizes;
import org.apache.cassandra.dht.IPartitioner;
import org.apache.cassandra.dht.IPartitionerDependentSerializer;
@ -58,6 +61,8 @@ import org.apache.cassandra.net.MessagingService;
*/
public class MerkleTree implements Serializable
{
private static Logger logger = LoggerFactory.getLogger(MerkleTree.class);
public static final MerkleTreeSerializer serializer = new MerkleTreeSerializer();
private static final long serialVersionUID = 2L;
@ -241,8 +246,12 @@ 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))
{
logger.debug("Range {} fully inconsistent", active);
diff.add(active);
}
}
else if (lhash == null || rhash == null)
diff.add(active);
@ -262,8 +271,19 @@ public class MerkleTree implements Serializable
return CONSISTENT;
Token midpoint = ltree.partitioner().midpoint(active.left, active.right);
// 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);
return FULLY_INCONSISTENT;
}
TreeDifference left = new TreeDifference(active.left, midpoint, inc(active.depth));
TreeDifference right = new TreeDifference(midpoint, active.right, inc(active.depth));
logger.debug("({}) Hashing sub-ranges [{}, {}] for {} divided by midpoint {}", active.depth, left, right, active, midpoint);
byte[] lhash, rhash;
Hashable lnode, rnode;
@ -278,9 +298,16 @@ public class MerkleTree implements Serializable
int ldiff = CONSISTENT;
boolean lreso = lhash != null && rhash != null;
if (lreso && !Arrays.equals(lhash, rhash))
ldiff = differenceHelper(ltree, rtree, diff, left);
{
logger.debug("({}) Inconsistent digest on left sub-range {}: [{}, {}]", active.depth, left, lnode, rnode);
if (lnode instanceof Leaf) ldiff = FULLY_INCONSISTENT;
else ldiff = differenceHelper(ltree, rtree, diff, left);
}
else if (!lreso)
{
logger.debug("({}) Left sub-range fully inconsistent {}", active.depth, right);
ldiff = FULLY_INCONSISTENT;
}
// see if we should recurse right
lnode = ltree.find(right);
@ -293,25 +320,36 @@ public class MerkleTree implements Serializable
int rdiff = CONSISTENT;
boolean rreso = lhash != null && rhash != null;
if (rreso && !Arrays.equals(lhash, rhash))
rdiff = differenceHelper(ltree, rtree, diff, right);
{
logger.debug("({}) Inconsistent digest on right sub-range {}: [{}, {}]", active.depth, right, lnode, rnode);
if (rnode instanceof Leaf) rdiff = FULLY_INCONSISTENT;
else rdiff = differenceHelper(ltree, rtree, diff, right);
}
else if (!rreso)
{
logger.debug("({}) Right sub-range fully inconsistent {}", active.depth, right);
rdiff = FULLY_INCONSISTENT;
}
if (ldiff == FULLY_INCONSISTENT && rdiff == FULLY_INCONSISTENT)
{
// both children are fully inconsistent
logger.debug("({}) Fully inconsistent range [{}, {}]", active.depth, left, right);
return FULLY_INCONSISTENT;
}
else if (ldiff == FULLY_INCONSISTENT)
{
logger.debug("({}) Adding left sub-range to diff as fully inconsistent {}", active.depth, left);
diff.add(left);
return PARTIALLY_INCONSISTENT;
}
else if (rdiff == FULLY_INCONSISTENT)
{
logger.debug("({}) Adding right sub-range to diff as fully inconsistent {}", active.depth, right);
diff.add(right);
return PARTIALLY_INCONSISTENT;
}
logger.debug("({}) Range {} partially inconstent", active.depth, active);
return PARTIALLY_INCONSISTENT;
}

View File

@ -122,7 +122,7 @@ public class RepairOptionTest
@Test
public void testIncrementalRepairWithSubrangesIsNotGlobal() throws Exception
{
RepairOption ro = RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, "42:42"),
RepairOption ro = RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, "41:42"),
Murmur3Partitioner.instance);
assertFalse(ro.isGlobal());
ro = RepairOption.parse(ImmutableMap.of(RepairOption.INCREMENTAL_KEY, "true", RepairOption.RANGES_KEY, ""),

View File

@ -21,7 +21,7 @@ package org.apache.cassandra.utils;
import java.math.BigInteger;
import java.util.*;
import org.apache.cassandra.utils.AbstractIterator;
import com.google.common.collect.Lists;
import org.junit.Before;
import org.junit.Test;
@ -442,6 +442,38 @@ public class MerkleTreeTest
assertTrue(diffs.contains(new Range<>(leftmost.left, middle.right)));
}
/**
* difference should behave as expected, even with extremely small ranges
*/
@Test
public void differenceSmallRange()
{
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 = "hjkl".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));
}
List<TreeRange> diffs = MerkleTree.difference(ltree, rtree);
assertEquals(Lists.newArrayList(range), diffs);
}
/**
* Return the root hash of a binary tree with leaves at the given depths
* and with the given hash val in each leaf.