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

marcuse pushed a commit to branch cassandra-4.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git

commit 41b43a46c13680168b45181e904a170717cd2514
Author: Marcus Eriksson <[email protected]>
AuthorDate: Fri Sep 17 17:50:41 2021 +0200

    Avoid rewriting all repaired sstables during cleanup when transient 
replication is disabled
    
    Patch by marcuse; reviewed by Alex Petrov for CASSANDRA-16966
---
 CHANGES.txt                                        |  1 +
 .../cassandra/db/compaction/CompactionManager.java | 11 ++++------
 test/unit/org/apache/cassandra/db/CleanupTest.java | 25 ++++++++++++++++++++++
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index dbe8411..b2f3369 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0.2
+ * Avoid rewriting all sstables during cleanup when transient replication is 
enabled (CASSANDRA-16966)
  * Prevent CQLSH from failure on Python 3.10 (CASSANDRA-16987)
  * Avoid trying to acquire 0 permits from the rate limiter when taking 
snapshot (CASSANDRA-16872)
  * Upgrade Caffeine to 2.5.6 (CASSANDRA-15153)
diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java 
b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java
index 8a0926d..8d4f136 100644
--- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java
+++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java
@@ -540,11 +540,11 @@ public class CompactionManager implements 
CompactionManagerMBean
                 {
                     SSTableReader sstable = sstableIter.next();
                     boolean needsCleanupFull = needsCleanup(sstable, 
fullRanges);
-                    boolean needsCleanupTransient = needsCleanup(sstable, 
transientRanges);
+                    boolean needsCleanupTransient = !transientRanges.isEmpty() 
&& sstable.isRepaired() && needsCleanup(sstable, transientRanges);
                     //If there are no ranges for which the table needs cleanup 
either due to lack of intersection or lack
                     //of the table being repaired.
                     totalSSTables++;
-                    if (!needsCleanupFull && (!needsCleanupTransient || 
!sstable.isRepaired()))
+                    if (!needsCleanupFull && !needsCleanupTransient)
                     {
                         logger.debug("Skipping {} ([{}, {}]) for cleanup; all 
rows should be kept. Needs cleanup full ranges: {} Needs cleanup transient 
ranges: {} Repaired: {}",
                                     sstable,
@@ -568,7 +568,7 @@ public class CompactionManager implements 
CompactionManagerMBean
             public void execute(LifecycleTransaction txn) throws IOException
             {
                 CleanupStrategy cleanupStrategy = CleanupStrategy.get(cfStore, 
allRanges, transientRanges, txn.onlyOne().isRepaired(), 
FBUtilities.nowInSeconds());
-                doCleanupOne(cfStore, txn, cleanupStrategy, replicas.ranges(), 
fullRanges, transientRanges, hasIndexes);
+                doCleanupOne(cfStore, txn, cleanupStrategy, replicas.ranges(), 
hasIndexes);
             }
         }, jobs, OperationType.CLEANUP);
     }
@@ -1011,7 +1011,6 @@ public class CompactionManager implements 
CompactionManagerMBean
             final RangesAtEndpoint replicas = 
StorageService.instance.getLocalReplicas(keyspace.getName());
             final Set<Range<Token>> allRanges = replicas.ranges();
             final Set<Range<Token>> transientRanges = 
replicas.onlyTransient().ranges();
-            final Set<Range<Token>> fullRanges = replicas.onlyFull().ranges();
             boolean hasIndexes = cfs.indexManager.hasIndexes();
             SSTableReader sstable = lookupSSTable(cfs, entry.getValue());
 
@@ -1024,7 +1023,7 @@ public class CompactionManager implements 
CompactionManagerMBean
                 CleanupStrategy cleanupStrategy = CleanupStrategy.get(cfs, 
allRanges, transientRanges, sstable.isRepaired(), FBUtilities.nowInSeconds());
                 try (LifecycleTransaction txn = 
cfs.getTracker().tryModify(sstable, OperationType.CLEANUP))
                 {
-                    doCleanupOne(cfs, txn, cleanupStrategy, allRanges, 
fullRanges, transientRanges, hasIndexes);
+                    doCleanupOne(cfs, txn, cleanupStrategy, allRanges, 
hasIndexes);
                 }
                 catch (IOException e)
                 {
@@ -1209,8 +1208,6 @@ public class CompactionManager implements 
CompactionManagerMBean
                               LifecycleTransaction txn,
                               CleanupStrategy cleanupStrategy,
                               Collection<Range<Token>> allRanges,
-                              Collection<Range<Token>> fullRanges,
-                              Collection<Range<Token>> transientRanges,
                               boolean hasIndexes) throws IOException
     {
         assert !cfs.isIndex();
diff --git a/test/unit/org/apache/cassandra/db/CleanupTest.java 
b/test/unit/org/apache/cassandra/db/CleanupTest.java
index 9965361..6bb6433 100644
--- a/test/unit/org/apache/cassandra/db/CleanupTest.java
+++ b/test/unit/org/apache/cassandra/db/CleanupTest.java
@@ -258,6 +258,17 @@ public class CleanupTest
     @Test
     public void testCleanupSkippingSSTables() throws UnknownHostException, 
ExecutionException, InterruptedException
     {
+        testCleanupSkippingSSTablesHelper(false);
+    }
+
+    @Test
+    public void testCleanupSkippingRepairedSSTables() throws 
UnknownHostException, ExecutionException, InterruptedException
+    {
+        testCleanupSkippingSSTablesHelper(true);
+    }
+
+    public void testCleanupSkippingSSTablesHelper(boolean repaired) throws 
UnknownHostException, ExecutionException, InterruptedException
+    {
         Keyspace keyspace = Keyspace.open(KEYSPACE3);
         ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_STANDARD3);
         cfs.disableAutoCompaction();
@@ -276,6 +287,20 @@ public class CleanupTest
         }
 
         Set<SSTableReader> beforeFirstCleanup = 
Sets.newHashSet(cfs.getLiveSSTables());
+        if (repaired)
+        {
+            beforeFirstCleanup.forEach((sstable) -> {
+                try
+                {
+                    
sstable.descriptor.getMetadataSerializer().mutateRepairMetadata(sstable.descriptor,
 System.currentTimeMillis(), null, false);
+                    sstable.reloadSSTableMetadata();
+                }
+                catch (Exception e)
+                {
+                    throw new RuntimeException(e);
+                }
+            });
+        }
         // single token - 127.0.0.1 owns everything, cleanup should be noop
         cfs.forceCleanup(2);
         assertEquals(beforeFirstCleanup, cfs.getLiveSSTables());

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to