You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jm...@apache.org on 2022/09/19 16:09:13 UTC

[cassandra] branch trunk updated: Prevent NullPointerException when changing neverPurgeTombstones from true to false

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

jmckenzie pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new ff5f4833aa Prevent NullPointerException when changing neverPurgeTombstones from true to false
ff5f4833aa is described below

commit ff5f4833aa3e11fcffd6bff1b15597fd5a38b864
Author: Josh McKenzie <jm...@apache.org>
AuthorDate: Thu Sep 15 13:56:00 2022 -0400

    Prevent NullPointerException when changing neverPurgeTombstones from true to false
    
    Patch by Marcus Eriksson; reviewed by Caleb Rackliffe and Josh McKenzie for CASSANDRA-17897
    
    Co-authored-by: Marcus Eriksson <ma...@apache.org>
    Co-authored-by: Josh McKenzie <jm...@apache.org>
---
 CHANGES.txt                                        |  1 +
 .../db/compaction/CompactionController.java        | 16 ++++-------
 .../db/compaction/CompactionControllerTest.java    | 33 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 0439b08b82..10830cedb0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.2
+ * Prevent NullPointerException when changing neverPurgeTombstones from true to false (CASSANDRA-17897)
  * Add metrics around storage usage and compression (CASSANDRA-17898)
  * Remove usage of deprecated javax certificate classes (CASSANDRA-17867)
  * Make sure preview repairs don't optimise streams unless configured to (CASSANDRA-17865)
diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionController.java b/src/java/org/apache/cassandra/db/compaction/CompactionController.java
index 26dcdd39a6..31b097fc5b 100644
--- a/src/java/org/apache/cassandra/db/compaction/CompactionController.java
+++ b/src/java/org/apache/cassandra/db/compaction/CompactionController.java
@@ -105,14 +105,8 @@ public class CompactionController extends AbstractCompactionController
             return;
         }
 
-        for (SSTableReader reader : overlappingSSTables)
-        {
-            if (reader.isMarkedCompacted())
-            {
-                refreshOverlaps();
-                return;
-            }
-        }
+        if (overlappingSSTables == null || overlappingSSTables.stream().anyMatch(SSTableReader::isMarkedCompacted))
+            refreshOverlaps();
     }
 
     private void refreshOverlaps()
@@ -160,8 +154,8 @@ public class CompactionController extends AbstractCompactionController
     {
         logger.trace("Checking droppable sstables in {}", cfStore);
 
-        if (NEVER_PURGE_TOMBSTONES || compacting == null || cfStore.getNeverPurgeTombstones())
-            return Collections.<SSTableReader>emptySet();
+        if (NEVER_PURGE_TOMBSTONES || compacting == null || cfStore.getNeverPurgeTombstones() || overlapping == null)
+            return Collections.emptySet();
 
         if (cfStore.getCompactionStrategyManager().onlyPurgeRepairedTombstones() && !Iterables.all(compacting, SSTableReader::isRepaired))
             return Collections.emptySet();
@@ -243,7 +237,7 @@ public class CompactionController extends AbstractCompactionController
     @Override
     public LongPredicate getPurgeEvaluator(DecoratedKey key)
     {
-        if (NEVER_PURGE_TOMBSTONES || !compactingRepaired() || cfs.getNeverPurgeTombstones())
+        if (NEVER_PURGE_TOMBSTONES || !compactingRepaired() || cfs.getNeverPurgeTombstones() || overlapIterator == null)
             return time -> false;
 
         overlapIterator.update(key);
diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionControllerTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionControllerTest.java
index 9d81b61ed3..ce9b28ad9e 100644
--- a/test/unit/org/apache/cassandra/db/compaction/CompactionControllerTest.java
+++ b/test/unit/org/apache/cassandra/db/compaction/CompactionControllerTest.java
@@ -204,4 +204,37 @@ public class CompactionControllerTest extends SchemaLoader
         assertFalse(evaluator.test(boundary));
         assertTrue(evaluator.test(boundary - 1));
     }
+
+    @Test
+    public void testDisableNeverPurgeTombstones()
+    {
+        Keyspace keyspace = Keyspace.open(KEYSPACE);
+        ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF2);
+        cfs.truncateBlocking();
+
+        DecoratedKey key = Util.dk("k1");
+        long timestamp = System.currentTimeMillis();
+        applyMutation(cfs.metadata(), key, timestamp);
+        cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS);
+        Set<SSTableReader> toCompact = Sets.newHashSet(cfs.getLiveSSTables());
+        cfs.setNeverPurgeTombstones(true);
+        applyMutation(cfs.metadata(), key, timestamp + 1);
+
+        try (CompactionController cc = new CompactionController(cfs, toCompact, (int)(System.currentTimeMillis()/1000)))
+        {
+            assertFalse(cc.getPurgeEvaluator(key).test(timestamp));
+            assertFalse(cc.getPurgeEvaluator(key).test(timestamp + 1));
+            assertTrue(cc.getFullyExpiredSSTables().isEmpty());
+
+            cfs.setNeverPurgeTombstones(false);
+            assertFalse(cc.getPurgeEvaluator(key).test(timestamp));
+            assertFalse(cc.getPurgeEvaluator(key).test(timestamp + 1));
+            assertTrue(cc.getFullyExpiredSSTables().isEmpty());
+
+            cc.maybeRefreshOverlaps();
+            assertTrue(cc.getPurgeEvaluator(key).test(timestamp));
+            assertFalse(cc.getPurgeEvaluator(key).test(timestamp + 1));
+            assertTrue(cc.getFullyExpiredSSTables().isEmpty());
+        }
+    }
 }


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