Repository: cassandra Updated Branches: refs/heads/trunk 07bc6884b -> 10eda8b57
Fix the sstable-needs-cleanup check Patch by Jakub Janecek; reviewed by marcuse for CASSANDRA-10740 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/3d99418c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/3d99418c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/3d99418c Branch: refs/heads/trunk Commit: 3d99418cfd641b30b393c9e61cc2b4f5e864d14d Parents: 9768e57 Author: Jakub Janecek <janecek.ja...@gmail.com> Authored: Wed Nov 25 13:51:14 2015 +0100 Committer: Marcus Eriksson <marc...@apache.org> Committed: Thu Nov 26 08:25:31 2015 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../db/compaction/CompactionManager.java | 6 +- .../org/apache/cassandra/db/CleanupTest.java | 75 ++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/3d99418c/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 8548d71..3ede9b7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.12 + * Fix the sstable-needs-cleanup check (CASSANDRA-10740) * (cqlsh) Print column names before COPY operation (CASSANDRA-8935) * Add Native-Transport-Requests back to tpstats (CASSANDRA-10044) * Make paging logic consistent between searcher impls (CASSANDRA-10683) http://git-wip-us.apache.org/repos/asf/cassandra/blob/3d99418c/src/java/org/apache/cassandra/db/compaction/CompactionManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java index b85eb51..b0ad244 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -44,6 +44,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.collect.Multiset; import com.google.common.collect.Sets; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -690,7 +691,8 @@ public class CompactionManager implements CompactionManagerMBean * Determines if a cleanup would actually remove any data in this SSTable based * on a set of owned ranges. */ - static boolean needsCleanup(SSTableReader sstable, Collection<Range<Token>> ownedRanges) + @VisibleForTesting + public static boolean needsCleanup(SSTableReader sstable, Collection<Range<Token>> ownedRanges) { assert !ownedRanges.isEmpty(); // cleanup checks for this @@ -729,7 +731,7 @@ public class CompactionManager implements CompactionManagerMBean } Range<Token> nextRange = sortedRanges.get(i + 1); - if (!nextRange.contains(firstBeyondRange.getToken())) + if (firstBeyondRange.getToken().compareTo(nextRange.left) <= 0) { // we found a key in between the owned ranges return true; http://git-wip-us.apache.org/repos/asf/cassandra/blob/3d99418c/test/unit/org/apache/cassandra/db/CleanupTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/CleanupTest.java b/test/unit/org/apache/cassandra/db/CleanupTest.java index 06f8997..1d04dfa 100644 --- a/test/unit/org/apache/cassandra/db/CleanupTest.java +++ b/test/unit/org/apache/cassandra/db/CleanupTest.java @@ -22,9 +22,11 @@ import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; import java.nio.ByteBuffer; +import java.util.AbstractMap; import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -40,6 +42,7 @@ import org.apache.cassandra.db.index.SecondaryIndex; import org.apache.cassandra.dht.BytesToken; import org.apache.cassandra.dht.Range; import org.apache.cassandra.io.sstable.SSTableReader; +import org.apache.cassandra.dht.Token; import org.apache.cassandra.locator.TokenMetadata; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.utils.ByteBufferUtil; @@ -168,6 +171,78 @@ public class CleanupTest extends SchemaLoader assertEquals(0, rows.size()); } + @Test + public void testNeedsCleanup() throws Exception + { + // setup + StorageService.instance.getTokenMetadata().clearUnsafe(); + Keyspace keyspace = Keyspace.open(KEYSPACE1); + ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF1); + fillCF(cfs, LOOPS); + + // prepare SSTable and some useful tokens + SSTableReader ssTable = cfs.getSSTables().iterator().next(); + final Token ssTableMin = ssTable.first.getToken(); + final Token ssTableMax = ssTable.last.getToken(); + + final Token min = token((byte) 0); + final Token before1 = token((byte) 2); + final Token before2 = token((byte) 5); + final Token before3 = token((byte) 10); + final Token before4 = token((byte) 47); + final Token insideSsTable1 = token((byte) 50); + final Token insideSsTable2 = token((byte) 55); + final Token max = token((byte) 127, (byte) 127, (byte) 127, (byte) 127); + + // test sanity check + assert (min.compareTo(ssTableMin) < 0); + assert (before1.compareTo(ssTableMin) < 0); + assert (before2.compareTo(ssTableMin) < 0); + assert (before3.compareTo(ssTableMin) < 0); + assert (before4.compareTo(ssTableMin) < 0); + assert (ssTableMin.compareTo(insideSsTable1) < 0); + assert (insideSsTable1.compareTo(ssTableMax) < 0); + assert (ssTableMin.compareTo(insideSsTable2) < 0); + assert (insideSsTable2.compareTo(ssTableMax) < 0); + assert (ssTableMax.compareTo(max) < 0); + + // test cases + // key: needs cleanup? + // value: owned ranges + List<Map.Entry<Boolean, List<Range<Token>>>> testCases = new LinkedList<Map.Entry<Boolean, List<Range<Token>>>>() + { + { + add(entry(false, Arrays.asList(range(min, max)))); // SSTable owned as a whole + add(entry(true, Arrays.asList(range(min, insideSsTable1)))); // SSTable owned only partially + add(entry(true, Arrays.asList(range(insideSsTable1, max)))); // SSTable owned only partially + add(entry(true, Arrays.asList(range(min, ssTableMin)))); // SSTable not owned at all + add(entry(true, Arrays.asList(range(ssTableMax, max)))); // only last token of SSTable is owned + add(entry(true, Arrays.asList(range(min, insideSsTable1), range(insideSsTable2, max)))); // SSTable partially owned by two ranges + add(entry(true, Arrays.asList(range(ssTableMin, ssTableMax)))); // first token of SSTable is not owned + add(entry(false, Arrays.asList(range(before4, max)))); // first token of SSTable is not owned + add(entry(false, Arrays.asList(range(min, before1), range(before2, before3), range(before4, max)))); // SSTable owned by the last range + } + }; + + // check all test cases + for (Map.Entry<Boolean, List<Range<Token>>> testCase : testCases) + { + assertEquals(testCase.getKey(), CompactionManager.needsCleanup(ssTable, testCase.getValue())); + } + } + private static BytesToken token(byte ... value) + { + return new BytesToken(value); + } + private static <K, V> Map.Entry<K, V> entry(K k, V v) + { + return new AbstractMap.SimpleEntry<K, V>(k, v); + } + private static Range<Token> range(Token from, Token to) + { + return new Range<>(from, to); + } + protected void fillCF(ColumnFamilyStore cfs, int rowsPerSSTable) { CompactionManager.instance.disableAutoCompaction();
