From a14a954788e007ddaee7f215174e01a36b4dd35f Mon Sep 17 00:00:00 2001 From: Dmitry Konstantinov Date: Thu, 9 Jul 2026 18:14:14 +0100 Subject: [PATCH] Fix flakiness in CommitLogSegmentManagerCDCTest Restore deleteCDCRawFiles logic which is present in 6.0 and trunk but probably lost in 5.0 due to merges Files may be concurrently removed by the CDC management thread, so we tolerate a file that has already been deleted rather than failing the test. Fix IndexOutOfBoundsException at commitlog.AbstractCommitLogSegmentManager.forceRecycleAll - it is already fixed in 6.0/trunk. Cleanup test data after run to reduce disk usage. Patch by Dmitry Konstantinov; reviewed by Michael Semb Wever for CASSANDRA-20091 --- .../AbstractCommitLogSegmentManager.java | 5 +- .../CommitLogSegmentManagerCDCTest.java | 55 +++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogSegmentManager.java b/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogSegmentManager.java index 549955dd18..5a99dbc8c8 100644 --- a/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogSegmentManager.java +++ b/src/java/org/apache/cassandra/db/commitlog/AbstractCommitLogSegmentManager.java @@ -357,11 +357,12 @@ public abstract class AbstractCommitLogSegmentManager void forceRecycleAll(Collection droppedTables) { List segmentsToRecycle = new ArrayList<>(activeSegments); - CommitLogSegment last = segmentsToRecycle.get(segmentsToRecycle.size() - 1); + CommitLogSegment last = segmentsToRecycle.isEmpty() ? null : segmentsToRecycle.get(segmentsToRecycle.size() - 1); advanceAllocatingFrom(last); // wait for the commit log modifications - last.waitForModifications(); + if (last != null) + last.waitForModifications(); // make sure the writes have materialized inside of the memtables by waiting for all outstanding writes // to complete diff --git a/test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java b/test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java index e964efa2d8..6813f03142 100644 --- a/test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java +++ b/test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java @@ -30,6 +30,8 @@ import com.google.common.util.concurrent.Uninterruptibles; import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.io.util.File; import org.apache.cassandra.io.util.FileReader; + +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; @@ -42,7 +44,6 @@ import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.db.RowUpdateBuilder; import org.apache.cassandra.db.commitlog.CommitLogSegment.CDCState; import org.apache.cassandra.exceptions.CDCWriteException; -import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.schema.TableMetadata; public class CommitLogSegmentManagerCDCTest extends CQLTester @@ -62,13 +63,20 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester @Before public void beforeTest() throws Throwable { - super.beforeTest(); - // Need to clean out any files from previous test runs. Prevents flaky test failures. - CommitLog.instance.stopUnsafe(true); CommitLog.instance.start(); + super.beforeTest(); ((CommitLogSegmentManagerCDC)CommitLog.instance.segmentManager).updateCDCTotalSize(); } + @After + public void afterTest() throws Throwable + { + super.afterTest(); + // free up (a lot of) diskspace by deleting commitlog and cdc_raw after each test method + CommitLog.instance.stopUnsafe(true); + deleteCDCRawFiles(); + } + @Test public void testCDCWriteFailure() throws Throwable { @@ -87,11 +95,10 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester .forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); CommitLog.instance.forceRecycleAllSegments(); cdcMgr.awaitManagementTasksCompletion(); - Assert.assertTrue("Expected files to be moved to overflow.", getCDCRawCount() > 0); + Assert.assertTrue("Expected files to be moved to overflow.", getCDCRawFiles().length > 0); // Simulate a CDC consumer reading files then deleting them - for (File f : new File(DatabaseDescriptor.getCDCLogLocation()).tryList()) - FileUtils.deleteWithConfirm(f); + deleteCDCRawFiles(); // Update size tracker to reflect deleted files. Should flip flag on current allocatingFrom to allow. cdcMgr.updateCDCTotalSize(); @@ -272,13 +279,12 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester // Build up a list of expected index files after replay and then clear out cdc_raw List oldData = parseCDCIndexData(); - for (File f : new File(DatabaseDescriptor.getCDCLogLocation()).tryList()) - FileUtils.deleteWithConfirm(f.absolutePath()); + deleteCDCRawFiles(); try { Assert.assertEquals("Expected 0 files in CDC folder after deletion. ", - 0, new File(DatabaseDescriptor.getCDCLogLocation()).tryList().length); + 0, getCDCRawFiles().length); } finally { @@ -293,7 +299,7 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester // Rough sanity check -> should be files there now. Assert.assertTrue("Expected non-zero number of files in CDC folder after restart.", - new File(DatabaseDescriptor.getCDCLogLocation()).tryList().length > 0); + getCDCRawFiles().length > 0); // Confirm all the old indexes in old are present and >= the original offset, as we flag the entire segment // as cdc written on a replay. @@ -339,7 +345,7 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester List results = new ArrayList<>(); try { - for (File f : new File(DatabaseDescriptor.getCDCLogLocation()).tryList()) + for (File f : getCDCRawFiles()) { if (f.name().contains("_cdc.idx")) results.add(new CDCIndexData(f)); @@ -393,11 +399,6 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester return ByteBuffer.wrap(toWrap); } - private int getCDCRawCount() - { - return new File(DatabaseDescriptor.getCDCLogLocation()).tryList().length; - } - private void expectCurrentCDCState(CDCState expectedState) { CDCState currentState = CommitLog.instance.segmentManager.allocatingFrom().getCDCState(); @@ -495,20 +496,30 @@ public class CommitLogSegmentManagerCDCTest extends CQLTester cdcMgr.awaitManagementTasksCompletion(); // Delete all files in cdc_raw - for (File f : new File(DatabaseDescriptor.getCDCLogLocation()).tryList()) - f.delete(); + deleteCDCRawFiles(); cdcMgr.updateCDCTotalSize(); // Confirm cdc update process changes flag on active segment expectCurrentCDCState(CDCState.PERMITTED); } // Clear out archived CDC files - for (File f : new File(DatabaseDescriptor.getCDCLogLocation()).tryList()) { - FileUtils.deleteWithConfirm(f); - } + deleteCDCRawFiles(); }); } + private static File[] getCDCRawFiles() + { + return new File(DatabaseDescriptor.getCDCLogLocation()).tryList(); + } + + private static void deleteCDCRawFiles() + { + for (File f : getCDCRawFiles()) + { + f.deleteIfExists(); + } + } + private interface Testable { void run() throws Throwable;