Nodetool cleanup on KS with no replicas should remove old data, not silently 
complete

Patch by Zhao Yang; Reviewed by Jeff Jirsa for CASSANDRA-13526


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

Branch: refs/heads/trunk
Commit: 090f418831be4e4dace861fda380ee4ec27cec35
Parents: 461af5b
Author: Zhao Yang <zhaoyangsingap...@gmail.com>
Authored: Thu Jul 6 00:10:49 2017 +0800
Committer: Jeff Jirsa <jji...@apple.com>
Committed: Wed Dec 6 21:40:54 2017 -0800

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../db/compaction/CompactionManager.java        | 12 ++--
 .../org/apache/cassandra/db/CleanupTest.java    | 63 ++++++++++++++++++++
 3 files changed, 70 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/090f4188/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 54a8538..9638886 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.16
+ * Fix cleanup on keyspace with no replicas (CASSANDRA-13526)
  * Fix updating base table rows with TTL not removing materialized view 
entries (CASSANDRA-14071)
  * Reduce garbage created by DynamicSnitch (CASSANDRA-14091)
  * More frequent commitlog chained markers (CASSANDRA-13987)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/090f4188/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 4483960..fdda562 100644
--- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java
+++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java
@@ -435,12 +435,8 @@ public class CompactionManager implements 
CompactionManagerMBean
             logger.info("Cleanup cannot run before a node has joined the 
ring");
             return AllSSTableOpStatus.ABORTED;
         }
+        // if local ranges is empty, it means no data should remain
         final Collection<Range<Token>> ranges = 
StorageService.instance.getLocalRanges(keyspace.getName());
-        if (ranges.isEmpty())
-        {
-            logger.info("Node owns no data for keyspace {}", 
keyspace.getName());
-            return AllSSTableOpStatus.SUCCESSFUL;
-        }
         final boolean hasIndexes = cfStore.indexManager.hasIndexes();
 
         return parallelAllSSTableOperation(cfStore, new OneSSTableOperation()
@@ -783,7 +779,10 @@ public class CompactionManager implements 
CompactionManagerMBean
     @VisibleForTesting
     public static boolean needsCleanup(SSTableReader sstable, 
Collection<Range<Token>> ownedRanges)
     {
-        assert !ownedRanges.isEmpty(); // cleanup checks for this
+        if (ownedRanges.isEmpty())
+        {
+            return true; // all data will be cleaned
+        }
 
         // unwrap and sort the ranges by LHS token
         List<Range<Token>> sortedRanges = Range.normalize(ownedRanges);
@@ -842,6 +841,7 @@ public class CompactionManager implements 
CompactionManagerMBean
 
         SSTableReader sstable = txn.onlyOne();
 
+        // if ranges is empty and no index, entire sstable is discarded
         if (!hasIndexes && !new Bounds<>(sstable.first.getToken(), 
sstable.last.getToken()).intersects(ranges))
         {
             txn.obsoleteOriginals();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/090f4188/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 b4ffe57..99030c5 100644
--- a/test/unit/org/apache/cassandra/db/CleanupTest.java
+++ b/test/unit/org/apache/cassandra/db/CleanupTest.java
@@ -24,9 +24,11 @@ import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
 import java.util.AbstractMap;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 
@@ -36,6 +38,8 @@ import org.junit.Test;
 import org.apache.cassandra.SchemaLoader;
 import org.apache.cassandra.Util;
 import org.apache.cassandra.config.ColumnDefinition;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.schema.KeyspaceMetadata;
 import org.apache.cassandra.cql3.Operator;
 import org.apache.cassandra.db.compaction.CompactionManager;
 import org.apache.cassandra.db.filter.RowFilter;
@@ -44,12 +48,14 @@ import org.apache.cassandra.dht.Range;
 import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.dht.Token;
+import org.apache.cassandra.locator.AbstractNetworkTopologySnitch;
 import org.apache.cassandra.locator.TokenMetadata;
 import org.apache.cassandra.schema.KeyspaceParams;
 import org.apache.cassandra.service.StorageService;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 public class CleanupTest
 {
@@ -57,6 +63,11 @@ public class CleanupTest
     public static final String KEYSPACE1 = "CleanupTest1";
     public static final String CF_INDEXED1 = "Indexed1";
     public static final String CF_STANDARD1 = "Standard1";
+
+    public static final String KEYSPACE2 = "CleanupTestMultiDc";
+    public static final String CF_INDEXED2 = "Indexed2";
+    public static final String CF_STANDARD2 = "Standard2";
+
     public static final ByteBuffer COLUMN = ByteBufferUtil.bytes("birthdate");
     public static final ByteBuffer VALUE = ByteBuffer.allocate(8);
     static
@@ -73,6 +84,27 @@ public class CleanupTest
                                     KeyspaceParams.simple(1),
                                     SchemaLoader.standardCFMD(KEYSPACE1, 
CF_STANDARD1),
                                     SchemaLoader.compositeIndexCFMD(KEYSPACE1, 
CF_INDEXED1, true));
+
+
+        DatabaseDescriptor.setEndpointSnitch(new 
AbstractNetworkTopologySnitch()
+        {
+            @Override
+            public String getRack(InetAddress endpoint)
+            {
+                return "RC1";
+            }
+
+            @Override
+            public String getDatacenter(InetAddress endpoint)
+            {
+                return "DC1";
+            }
+        });
+
+        SchemaLoader.createKeyspace(KEYSPACE2,
+                                    KeyspaceParams.nts("DC1", 1),
+                                    SchemaLoader.standardCFMD(KEYSPACE2, 
CF_STANDARD2),
+                                    SchemaLoader.compositeIndexCFMD(KEYSPACE2, 
CF_INDEXED2, true));
     }
 
     /*
@@ -174,6 +206,36 @@ public class CleanupTest
     }
 
     @Test
+    public void testCleanupWithNoTokenRange() throws Exception
+    {
+
+        TokenMetadata tmd = StorageService.instance.getTokenMetadata();
+        tmd.clearUnsafe();
+        tmd.updateHostId(UUID.randomUUID(), 
InetAddress.getByName("127.0.0.1"));
+        byte[] tk1 = {2};
+        tmd.updateNormalToken(new BytesToken(tk1), 
InetAddress.getByName("127.0.0.1"));
+
+
+        Keyspace keyspace = Keyspace.open(KEYSPACE2);
+        keyspace.setMetadata(KeyspaceMetadata.create(KEYSPACE2, 
KeyspaceParams.nts("DC1", 1)));
+        ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_STANDARD2);
+
+        // insert data and verify we get it back w/ range query
+        fillCF(cfs, "val", LOOPS);
+        assertEquals(LOOPS, Util.getAll(Util.cmd(cfs).build()).size());
+
+        // remove replication on DC1
+        keyspace.setMetadata(KeyspaceMetadata.create(KEYSPACE2, 
KeyspaceParams.nts("DC1", 0)));
+
+        // clear token range for localhost on DC1
+
+        CompactionManager.instance.performCleanup(cfs, 2);
+        assertEquals(0, Util.getAll(Util.cmd(cfs).build()).size());
+        assertTrue(cfs.getLiveSSTables().isEmpty());
+    }
+
+
+    @Test
     public void testNeedsCleanup() throws Exception
     {
         // setup
@@ -223,6 +285,7 @@ public class CleanupTest
                 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
+                add(entry(true, Collections.EMPTY_LIST)); // empty token range 
means discard entire sstable
             }
         };
 


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

Reply via email to