cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From marc...@apache.org
Subject [1/4] cassandra git commit: Fix the sstable-needs-cleanup check
Date Thu, 26 Nov 2015 07:51:18 GMT
Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.1 138f8db85 -> c2ea8c4fc


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/cassandra-3.1
Commit: 3d99418cfd641b30b393c9e61cc2b4f5e864d14d
Parents: 9768e57
Author: Jakub Janecek <janecek.jakub@gmail.com>
Authored: Wed Nov 25 13:51:14 2015 +0100
Committer: Marcus Eriksson <marcuse@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();


Mime
View raw message