From 9b61905e4b2f20c2315c4460520a8554e0aa519a Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:20:54 -0600 Subject: [PATCH 01/10] CASSANDRA-21065: Fix ConcurrentModificationException in GC compaction by copying transaction.originals() --- .../org/apache/cassandra/db/compaction/CompactionManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java index 16079ff8883d..44270bbe0ea7 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -688,7 +688,9 @@ public Iterable filterSSTables(LifecycleTransaction transaction) List filteredSSTables = new ArrayList<>(); if (cfStore.getCompactionStrategyManager().onlyPurgeRepairedTombstones()) { - for (SSTableReader sstable : transaction.originals()) + // Copy originals to avoid ConcurrentModificationException when cancel() + // modifies the underlying collection + for (SSTableReader sstable : new ArrayList<>(transaction.originals())) { if (!sstable.isRepaired()) { From 46fda6be3c19884a0a021d61766c0cd2a9a23e7a Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:21:32 -0600 Subject: [PATCH 02/10] CASSANDRA-21065: Add tests for GC compaction with mixed repaired/unrepaired SSTables --- .../GarbageCollectRepairedSSTablesTest.java | 570 ++++++++++++++++++ 1 file changed, 570 insertions(+) create mode 100644 test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java diff --git a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java new file mode 100644 index 000000000000..9655d0376207 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java @@ -0,0 +1,570 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.db.compaction; + +import java.io.IOException; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.ConcurrentModificationException; +import java.util.Iterator; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.Test; +import org.awaitility.Awaitility; + +import org.apache.cassandra.Util; +import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.schema.CompactionParams; +import org.apache.cassandra.utils.concurrent.Ref; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Tests for garbage collection with only_purge_repaired_tombstones enabled. + * + * Specifically tests the fix for the ConcurrentModificationException that occurred + * when calling nodetool garbagecollect on a table with mixed repaired/unrepaired + * SSTables and only_purge_repaired_tombstones=true. + * + * The bug was in CompactionManager.performGarbageCollection() where the code + * iterated directly over transaction.originals() while calling transaction.cancel() + * inside the loop, causing the underlying collection to be modified during iteration. + */ +public class GarbageCollectRepairedSSTablesTest extends CQLTester +{ + /** + * Tests that garbage collection completes without ConcurrentModificationException + * when there are mixed repaired and unrepaired SSTables with only_purge_repaired_tombstones=true. + * + * Before the fix, this would fail with: + * java.util.ConcurrentModificationException + * at java.util.HashMap$HashIterator.nextNode(HashMap.java:1597) + * at org.apache.cassandra.db.compaction.CompactionManager$6.filterSSTables(CompactionManager.java:691) + */ + @Test + public void testGarbageCollectWithMixedRepairedUnrepairedSSTables() throws Throwable + { + // Create table with UnifiedCompactionStrategy and only_purge_repaired_tombstones=true + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create multiple SSTables + for (int i = 0; i < 5; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + Set sstables = cfs.getLiveSSTables(); + assertEquals("Should have 5 SSTables", 5, sstables.size()); + + // Mark some SSTables as repaired (alternate pattern to ensure mix) + int count = 0; + for (SSTableReader sstable : sstables) + { + if (count % 2 == 0) + { + repair(cfs, sstable); + } + count++; + } + + // Verify we have a mix of repaired and unrepaired + long repairedCount = cfs.getLiveSSTables().stream().filter(SSTableReader::isRepaired).count(); + long unrepairedCount = cfs.getLiveSSTables().stream().filter(s -> !s.isRepaired()).count(); + assertTrue("Should have at least 1 repaired SSTable", repairedCount >= 1); + assertTrue("Should have at least 1 unrepaired SSTable", unrepairedCount >= 1); + + // This should NOT throw ConcurrentModificationException + // Before the fix, this would fail when iterating over originals() while cancel() modifies it + try + { + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + catch (ConcurrentModificationException e) + { + fail("Garbage collection should not throw ConcurrentModificationException. " + + "This indicates the bug in filterSSTables() where transaction.originals() is " + + "iterated while transaction.cancel() modifies the underlying collection."); + } + } + + /** + * Tests that garbage collection works when ALL SSTables are unrepaired. + * In this case, all SSTables should be cancelled (not processed for GC) + * since only_purge_repaired_tombstones is true. + */ + @Test + public void testGarbageCollectAllUnrepaired() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create SSTables (all unrepaired by default) + for (int i = 0; i < 3; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Verify all are unrepaired + long unrepairedCount = cfs.getLiveSSTables().stream().filter(s -> !s.isRepaired()).count(); + assertEquals("All 3 SSTables should be unrepaired", 3, unrepairedCount); + + // Should complete without error - all SSTables will be cancelled + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + + /** + * Tests that garbage collection works when ALL SSTables are repaired. + * In this case, no cancellation happens, so no risk of ConcurrentModificationException. + */ + @Test + public void testGarbageCollectAllRepaired() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create and repair all SSTables + for (int i = 0; i < 3; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + repair(cfs, sstable); + } + + // Verify all are repaired + long repairedCount = cfs.getLiveSSTables().stream().filter(SSTableReader::isRepaired).count(); + assertEquals("All 3 SSTables should be repaired", 3, repairedCount); + + // Should complete without error + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + + /** + * Tests garbage collection without only_purge_repaired_tombstones (baseline test). + * This code path doesn't involve the problematic iteration with cancel(). + */ + @Test + public void testGarbageCollectWithoutOnlyPurgeRepaired() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy'}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + execute("INSERT INTO %s (id, data) VALUES (1, 'data1')"); + Util.flush(cfs); + + // Should complete without error + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + + /** + * Tests with SizeTieredCompactionStrategy to ensure the fix doesn't break + * the original behavior reported in CASSANDRA-14204. + */ + @Test + public void testGarbageCollectWithSTCS() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'SizeTieredCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create multiple SSTables + for (int i = 0; i < 4; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Mark half as repaired + Iterator iter = cfs.getLiveSSTables().iterator(); + if (iter.hasNext()) repair(cfs, iter.next()); + if (iter.hasNext()) repair(cfs, iter.next()); + + // Should complete without ConcurrentModificationException + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + + /** + * Tests with LeveledCompactionStrategy to ensure the fix works across + * all compaction strategies. + */ + @Test + public void testGarbageCollectWithLCS() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'LeveledCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create multiple SSTables + for (int i = 0; i < 4; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Mark half as repaired + Iterator iter = cfs.getLiveSSTables().iterator(); + if (iter.hasNext()) repair(cfs, iter.next()); + if (iter.hasNext()) repair(cfs, iter.next()); + + // Should complete without ConcurrentModificationException + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + + /** + * Tests that reference leaks are detected when ConcurrentModificationException + * prevents proper transaction cleanup. + * + * This test uses Cassandra's Ref.OnLeak callback to detect when SSTable references + * are not properly released. The bug in performGarbageCollection() causes a + * ConcurrentModificationException which prevents the transaction from completing + * properly, leaving SSTable references unreleased (leaked). + * + * The leak would show in logs as: + * ERROR [Reference-Reaper] - LEAK DETECTED: a reference to ... + */ + @Test + public void testLeakDetectionWithMixedRepairedUnrepaired() throws Throwable + { + // Track detected leaks + CopyOnWriteArrayList detectedLeaks = new CopyOnWriteArrayList<>(); + + try + { + // Install leak detector callback + Ref.setOnLeak(state -> { + detectedLeaks.add(state); + System.err.println("LEAK DETECTED in test: " + state); + }); + + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create multiple SSTables + for (int i = 0; i < 5; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Mark some as repaired to create mixed state + int count = 0; + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + if (count % 2 == 0) + { + repair(cfs, sstable); + } + count++; + } + + // Attempt garbage collection - this may throw ConcurrentModificationException + // if the bug is present, which would prevent proper reference cleanup + boolean exceptionThrown = false; + try + { + CompactionManager.instance.performGarbageCollection(cfs, CompactionParams.TombstoneOption.CELL, 1); + } + catch (ConcurrentModificationException e) + { + exceptionThrown = true; + System.err.println("ConcurrentModificationException caught (bug is present): " + e.getMessage()); + } + + // Force garbage collection to trigger leak detection + // The Ref cleanup happens via PhantomReference when objects are GC'd + for (int i = 0; i < 5; i++) + { + System.gc(); + Thread.sleep(100); + } + + // Give the Reference-Reaper thread time to process + Thread.sleep(500); + + // If exception was thrown, we expect leaks (bug is present) + // If no exception, we should have no leaks (fix is working) + if (exceptionThrown) + { + // Bug is present - we may see leaks due to improper cleanup + System.err.println("Bug detected: ConcurrentModificationException was thrown."); + System.err.println("Leaks detected: " + detectedLeaks.size()); + fail("ConcurrentModificationException was thrown, indicating the bug is present. " + + "This can cause reference leaks."); + } + else + { + // Fix is working - verify no leaks were detected + assertTrue("No leaks should be detected when garbage collection completes successfully. " + + "Detected leaks: " + detectedLeaks, + detectedLeaks.isEmpty()); + } + } + finally + { + // Clear leak handler + Ref.setOnLeak(null); + } + } + + /** + * Tests that reference leaks are reliably detected using Awaitility to wait + * for the asynchronous leak detection mechanism. + * + * This test directly simulates what happens when a LifecycleTransaction is not + * properly closed - references to SSTables are acquired but never released, + * causing the Reference-Reaper to detect and report the leak. + * + * The leak detection is asynchronous because it relies on: + * 1. JVM garbage collection to collect unreleased Ref objects + * 2. PhantomReference processing by the Reference-Reaper thread + * 3. The OnLeak callback being invoked + * + * Uses Awaitility to poll and wait for leak detection with proper timeouts. + */ + @Test + public void testLeakDetectionWithAwaitility() throws Throwable + { + AtomicBoolean leakDetected = new AtomicBoolean(false); + CopyOnWriteArrayList leakDetails = new CopyOnWriteArrayList<>(); + + try + { + // Install leak detector callback + Ref.setOnLeak(state -> { + leakDetected.set(true); + String detail = "LEAK DETECTED: " + state; + leakDetails.add(detail); + System.err.println(detail); + }); + + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create SSTables + for (int i = 0; i < 3; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Simulate what happens when ConcurrentModificationException prevents cleanup: + // Acquire references to SSTables but deliberately don't release them + // This simulates the leak that occurs when transaction.close() is not called + simulateLeakedReferences(cfs); + + // Wait for leak detection with Awaitility + Awaitility.await() + .atMost(Duration.ofSeconds(30)) + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted(() -> { + // Force GC to trigger leak detection + System.gc(); + System.gc(); + System.gc(); + assertThat(leakDetected.get()) + .as("Leak should be detected for unreleased SSTable references") + .isTrue(); + }); + + // Report findings + System.err.println("Leak detection confirmed!"); + System.err.println("Leaks detected: " + leakDetails.size()); + for (String detail : leakDetails) + { + System.err.println(" - " + detail); + } + + assertThat(leakDetails.size()) + .as("At least one leak should be detected") + .isGreaterThan(0); + } + finally + { + // Clear leak handler + Ref.setOnLeak(null); + } + } + + /** + * Tests that NO reference leaks occur when references are properly managed. + * This is the counterpart to testLeakDetectionWithAwaitility - it demonstrates + * that when code properly releases references (like the fix does), no leaks are detected. + * + * The fix pattern is: acquire reference -> do work -> release reference + * This mimics the correct behavior in performGarbageCollection() after the fix. + */ + @Test + public void testNoLeakDetectionWithAwaitility() throws Throwable + { + AtomicBoolean leakDetected = new AtomicBoolean(false); + CopyOnWriteArrayList leakDetails = new CopyOnWriteArrayList<>(); + + try + { + // Install leak detector callback + Ref.setOnLeak(state -> { + leakDetected.set(true); + String detail = "LEAK DETECTED: " + state; + leakDetails.add(detail); + System.err.println(detail); + }); + + createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + + "WITH gc_grace_seconds=0 " + + "AND compaction = {'class':'UnifiedCompactionStrategy', 'only_purge_repaired_tombstones':true}"); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.disableAutoCompaction(); + + // Create SSTables + for (int i = 0; i < 3; i++) + { + execute("INSERT INTO %s (id, data) VALUES (?, ?)", i, "data" + i); + Util.flush(cfs); + } + + // Simulate proper reference handling (like the fixed code does): + // Acquire references AND release them properly + simulateNoLeakedReferences(cfs); + + // Wait a reasonable time to confirm NO leaks are detected + // We poll for a bit to give the Reference-Reaper time to process any potential leaks + Awaitility.await() + .atMost(Duration.ofSeconds(10)) + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted(() -> { + // Force GC to give leak detection a chance to run + System.gc(); + System.gc(); + System.gc(); + // With proper reference management, no leaks should be detected + assertThat(leakDetected.get()) + .as("No leak should be detected when references are properly released") + .isFalse(); + }); + + // Final confirmation + System.err.println("No leak detection confirmed - references were properly managed!"); + System.err.println("Leaks detected: " + leakDetails.size()); + + assertThat(leakDetails.size()) + .as("No leaks should be detected when references are properly released") + .isEqualTo(0); + } + finally + { + // Clear leak handler + Ref.setOnLeak(null); + } + } + + /** + * Simulates leaked references by acquiring SSTable refs without releasing them. + * This is done in a separate method so local variables go out of scope and become GC-eligible. + */ + @SuppressWarnings("unused") + private void simulateLeakedReferences(ColumnFamilyStore cfs) + { + // Acquire references to SSTables - these will be "leaked" when we don't release them + // This simulates what happens when ConcurrentModificationException prevents proper cleanup + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + // Acquire a reference but deliberately don't release it + // This is equivalent to what happens when a transaction is interrupted mid-operation + Ref leakedRef = sstable.tryRef(); + // leakedRef goes out of scope without release() being called - THIS IS THE LEAK + // When leakedRef is garbage collected, the Reference-Reaper will detect the leak + } + } + + /** + * Simulates proper reference handling: acquire refs AND release them properly. + * This is the counterpart to simulateLeakedReferences which acquires but doesn't release. + * + * This demonstrates that when references are properly managed (acquired and released), + * no leaks are detected by the Reference-Reaper. + */ + private void simulateNoLeakedReferences(ColumnFamilyStore cfs) + { + // Simulate proper reference handling: acquire refs AND release them properly + // This is the counterpart to simulateLeakedReferences which acquires but doesn't release + for (SSTableReader sstable : new ArrayList<>(cfs.getLiveSSTables())) + { + Ref ref = sstable.tryRef(); + if (ref != null) + ref.release(); + } + } + + /** + * Helper method to mark an SSTable as repaired. + * Follows the pattern from RepairedDataTombstonesTest. + */ + private static void repair(ColumnFamilyStore cfs, SSTableReader sstable) throws IOException + { + sstable.descriptor.getMetadataSerializer().mutateRepairMetadata(sstable.descriptor, System.currentTimeMillis(), null, false); + sstable.reloadSSTableMetadata(); + cfs.getTracker().notifySSTableRepairedStatusChanged(Collections.singleton(sstable)); + } +} From 72875ad25d23fd925776346330409fd232566e71 Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:23:00 -0600 Subject: [PATCH 03/10] CASSANDRA-21065: Add Docker setup for running unit tests --- test/docker/Dockerfile | 26 +++++++++++++++++++ test/docker/README.md | 47 ++++++++++++++++++++++++++++++++++ test/docker/docker-compose.yml | 29 +++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 test/docker/Dockerfile create mode 100644 test/docker/README.md create mode 100644 test/docker/docker-compose.yml diff --git a/test/docker/Dockerfile b/test/docker/Dockerfile new file mode 100644 index 000000000000..56d0d0bcb7c7 --- /dev/null +++ b/test/docker/Dockerfile @@ -0,0 +1,26 @@ +# Dockerfile for running Cassandra unit tests +# Uses official Ubuntu image as base +FROM ubuntu:24.04 + +# Prevent interactive prompts during package installation +ENV DEBIAN_FRONTEND=noninteractive + +# Install OpenJDK 11 and required build tools +RUN apt-get update && apt-get install -y \ + openjdk-11-jdk \ + ant \ + ant-optional \ + git \ + python3 \ + && rm -rf /var/lib/apt/lists/* + +# Set working directory +WORKDIR /cassandra + +# Set environment variables for Cassandra build +ENV JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 +ENV ANT_HOME=/usr/share/ant +ENV PATH="${ANT_HOME}/bin:${PATH}" + +# Default command - run the full test suite +CMD [".build/run-tests.sh", "-a", "test"] diff --git a/test/docker/README.md b/test/docker/README.md new file mode 100644 index 000000000000..d984a58c8336 --- /dev/null +++ b/test/docker/README.md @@ -0,0 +1,47 @@ +# Cassandra Unit Test Docker Setup + +This Docker Compose setup allows running Cassandra unit tests in an isolated environment with all required dependencies (Java 11 and Ant). + +## Prerequisites + +- Docker +- Docker Compose + +## Usage + +### Run the full test suite + +```bash +cd test/docker +docker compose up --build +``` + +### Run a specific test + +```bash +cd test/docker +TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up --build +``` + +### Run multiple specific tests + +```bash +cd test/docker +TEST_NAME_REGEX="GarbageCollectRepairedSSTablesTest,NeverPurgeTest" docker compose up --build +``` + +### Clean up + +```bash +docker compose down +``` + +## Test for the ConcurrentModificationException Fix + +To verify the fix for the ConcurrentModificationException bug in `performGarbageCollection()`: + +```bash +TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up --build +``` + +This test validates that garbage collection works correctly with mixed repaired/unrepaired SSTables when `only_purge_repaired_tombstones=true`. diff --git a/test/docker/docker-compose.yml b/test/docker/docker-compose.yml new file mode 100644 index 000000000000..7a8878f6887e --- /dev/null +++ b/test/docker/docker-compose.yml @@ -0,0 +1,29 @@ +services: + cassandra-test: + build: + context: . + dockerfile: Dockerfile + volumes: + # Mount the Cassandra source code + - ../../:/cassandra:rw + # Cache Maven repository to avoid re-downloading dependencies + - ./.m2:/root/.m2 + working_dir: /cassandra + # Run full suite by default, or specific test if TEST_NAME_REGEX is set + command: > + sh -c 'if [ -n "$TEST_NAME_REGEX" ]; then + .build/run-tests.sh -a test -t "$TEST_NAME_REGEX"; + else + ant test; + fi' + environment: + # Set TEST_NAME_REGEX to run a specific test, e.g.: + # TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up + - TEST_NAME_REGEX=${TEST_NAME_REGEX:-} + # Allow container to use more resources + deploy: + resources: + limits: + memory: 8g + reservations: + memory: 4g From 0863c11e4f18938fddcb10322568e1c252f45cb0 Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:32:20 -0600 Subject: [PATCH 04/10] clear test output from previous runs --- test/docker/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/docker/docker-compose.yml b/test/docker/docker-compose.yml index 7a8878f6887e..b16a448e25e6 100644 --- a/test/docker/docker-compose.yml +++ b/test/docker/docker-compose.yml @@ -11,7 +11,7 @@ services: working_dir: /cassandra # Run full suite by default, or specific test if TEST_NAME_REGEX is set command: > - sh -c 'if [ -n "$TEST_NAME_REGEX" ]; then + sh -c 'rm -rf build/test/output/* && if [ -n "$TEST_NAME_REGEX" ]; then .build/run-tests.sh -a test -t "$TEST_NAME_REGEX"; else ant test; From 6fde361dcb404e91752feeba2c8a708c7558b024 Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:37:37 -0600 Subject: [PATCH 05/10] move extra text from README to PR description --- test/docker/README.md | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/docker/README.md b/test/docker/README.md index d984a58c8336..9ae8928c7710 100644 --- a/test/docker/README.md +++ b/test/docker/README.md @@ -35,13 +35,3 @@ TEST_NAME_REGEX="GarbageCollectRepairedSSTablesTest,NeverPurgeTest" docker compo ```bash docker compose down ``` - -## Test for the ConcurrentModificationException Fix - -To verify the fix for the ConcurrentModificationException bug in `performGarbageCollection()`: - -```bash -TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up --build -``` - -This test validates that garbage collection works correctly with mixed repaired/unrepaired SSTables when `only_purge_repaired_tombstones=true`. From 70d62f86410caf4931f58d9781818c8cb0ba2b27 Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Mon, 19 Jan 2026 17:46:31 -0600 Subject: [PATCH 06/10] add license text --- test/docker/Dockerfile | 17 +++++++++++++++++ test/docker/docker-compose.yml | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/test/docker/Dockerfile b/test/docker/Dockerfile index 56d0d0bcb7c7..fa0cffab24df 100644 --- a/test/docker/Dockerfile +++ b/test/docker/Dockerfile @@ -1,3 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# # Dockerfile for running Cassandra unit tests # Uses official Ubuntu image as base FROM ubuntu:24.04 diff --git a/test/docker/docker-compose.yml b/test/docker/docker-compose.yml index b16a448e25e6..278d76bca9e2 100644 --- a/test/docker/docker-compose.yml +++ b/test/docker/docker-compose.yml @@ -1,3 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# + services: cassandra-test: build: From 9a2e27536cf3d0d46981a278d610e6603998b40d Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Wed, 21 Jan 2026 01:53:17 -0600 Subject: [PATCH 07/10] Apply suggestions from Mick's code review Co-authored-by: mck --- .../cassandra/db/compaction/CompactionManager.java | 2 +- .../GarbageCollectRepairedSSTablesTest.java | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java index 44270bbe0ea7..e16e46c17c5b 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionManager.java @@ -689,7 +689,7 @@ public Iterable filterSSTables(LifecycleTransaction transaction) if (cfStore.getCompactionStrategyManager().onlyPurgeRepairedTombstones()) { // Copy originals to avoid ConcurrentModificationException when cancel() - // modifies the underlying collection + // modifies the underlying collection when calling `cancel(..)` for (SSTableReader sstable : new ArrayList<>(transaction.originals())) { if (!sstable.isRepaired()) diff --git a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java index 9655d0376207..be597cf419e2 100644 --- a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java @@ -53,7 +53,7 @@ * iterated directly over transaction.originals() while calling transaction.cancel() * inside the loop, causing the underlying collection to be modified during iteration. */ -public class GarbageCollectRepairedSSTablesTest extends CQLTester +public class CompactionGarbageCollectOnlyPurgeRepairedTest extends CQLTester { /** * Tests that garbage collection completes without ConcurrentModificationException @@ -63,9 +63,11 @@ public class GarbageCollectRepairedSSTablesTest extends CQLTester * java.util.ConcurrentModificationException * at java.util.HashMap$HashIterator.nextNode(HashMap.java:1597) * at org.apache.cassandra.db.compaction.CompactionManager$6.filterSSTables(CompactionManager.java:691) + * Tests that garbage collection completes + * when there are mixed repaired and unrepaired SSTables with only_purge_repaired_tombstones=true */ @Test - public void testGarbageCollectWithMixedRepairedUnrepairedSSTables() throws Throwable + public void testOnlyPurgeRepaired() throws Throwable { // Create table with UnifiedCompactionStrategy and only_purge_repaired_tombstones=true createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + @@ -151,7 +153,7 @@ public void testGarbageCollectAllUnrepaired() throws Throwable * In this case, no cancellation happens, so no risk of ConcurrentModificationException. */ @Test - public void testGarbageCollectAllRepaired() throws Throwable + public void testAllRepaired() throws Throwable { createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + "WITH gc_grace_seconds=0 " + @@ -185,7 +187,7 @@ public void testGarbageCollectAllRepaired() throws Throwable * This code path doesn't involve the problematic iteration with cancel(). */ @Test - public void testGarbageCollectWithoutOnlyPurgeRepaired() throws Throwable + public void testWithoutOnlyPurgeRepaired() throws Throwable { createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + "WITH gc_grace_seconds=0 " + @@ -206,7 +208,7 @@ public void testGarbageCollectWithoutOnlyPurgeRepaired() throws Throwable * the original behavior reported in CASSANDRA-14204. */ @Test - public void testGarbageCollectWithSTCS() throws Throwable + public void testSTCS() throws Throwable { createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + "WITH gc_grace_seconds=0 " + @@ -236,7 +238,7 @@ public void testGarbageCollectWithSTCS() throws Throwable * all compaction strategies. */ @Test - public void testGarbageCollectWithLCS() throws Throwable + public void testLCS() throws Throwable { createTable("CREATE TABLE %s (id int PRIMARY KEY, data text) " + "WITH gc_grace_seconds=0 " + From 33a6dbbbc026914e56b41078981289569e3c437d Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Wed, 21 Jan 2026 01:55:04 -0600 Subject: [PATCH 08/10] Clean up comments in GarbageCollectRepairedSSTablesTest Remove redundant comments in the garbage collection test. --- .../db/compaction/GarbageCollectRepairedSSTablesTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java index be597cf419e2..d027ed617703 100644 --- a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java @@ -56,15 +56,13 @@ public class CompactionGarbageCollectOnlyPurgeRepairedTest extends CQLTester { /** - * Tests that garbage collection completes without ConcurrentModificationException - * when there are mixed repaired and unrepaired SSTables with only_purge_repaired_tombstones=true. + * Tests that garbage collection completes + * when there are mixed repaired and unrepaired SSTables with only_purge_repaired_tombstones=true * * Before the fix, this would fail with: * java.util.ConcurrentModificationException * at java.util.HashMap$HashIterator.nextNode(HashMap.java:1597) * at org.apache.cassandra.db.compaction.CompactionManager$6.filterSSTables(CompactionManager.java:691) - * Tests that garbage collection completes - * when there are mixed repaired and unrepaired SSTables with only_purge_repaired_tombstones=true */ @Test public void testOnlyPurgeRepaired() throws Throwable From 9b35fd0bc5d3982e99dc4dd713d843af56b4f7c2 Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Wed, 21 Jan 2026 01:56:54 -0600 Subject: [PATCH 09/10] remove redundant docker testing container --- test/docker/Dockerfile | 43 ------------------------------- test/docker/README.md | 37 -------------------------- test/docker/docker-compose.yml | 47 ---------------------------------- 3 files changed, 127 deletions(-) delete mode 100644 test/docker/Dockerfile delete mode 100644 test/docker/README.md delete mode 100644 test/docker/docker-compose.yml diff --git a/test/docker/Dockerfile b/test/docker/Dockerfile deleted file mode 100644 index fa0cffab24df..000000000000 --- a/test/docker/Dockerfile +++ /dev/null @@ -1,43 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# -# Dockerfile for running Cassandra unit tests -# Uses official Ubuntu image as base -FROM ubuntu:24.04 - -# Prevent interactive prompts during package installation -ENV DEBIAN_FRONTEND=noninteractive - -# Install OpenJDK 11 and required build tools -RUN apt-get update && apt-get install -y \ - openjdk-11-jdk \ - ant \ - ant-optional \ - git \ - python3 \ - && rm -rf /var/lib/apt/lists/* - -# Set working directory -WORKDIR /cassandra - -# Set environment variables for Cassandra build -ENV JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 -ENV ANT_HOME=/usr/share/ant -ENV PATH="${ANT_HOME}/bin:${PATH}" - -# Default command - run the full test suite -CMD [".build/run-tests.sh", "-a", "test"] diff --git a/test/docker/README.md b/test/docker/README.md deleted file mode 100644 index 9ae8928c7710..000000000000 --- a/test/docker/README.md +++ /dev/null @@ -1,37 +0,0 @@ -# Cassandra Unit Test Docker Setup - -This Docker Compose setup allows running Cassandra unit tests in an isolated environment with all required dependencies (Java 11 and Ant). - -## Prerequisites - -- Docker -- Docker Compose - -## Usage - -### Run the full test suite - -```bash -cd test/docker -docker compose up --build -``` - -### Run a specific test - -```bash -cd test/docker -TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up --build -``` - -### Run multiple specific tests - -```bash -cd test/docker -TEST_NAME_REGEX="GarbageCollectRepairedSSTablesTest,NeverPurgeTest" docker compose up --build -``` - -### Clean up - -```bash -docker compose down -``` diff --git a/test/docker/docker-compose.yml b/test/docker/docker-compose.yml deleted file mode 100644 index 278d76bca9e2..000000000000 --- a/test/docker/docker-compose.yml +++ /dev/null @@ -1,47 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# - -services: - cassandra-test: - build: - context: . - dockerfile: Dockerfile - volumes: - # Mount the Cassandra source code - - ../../:/cassandra:rw - # Cache Maven repository to avoid re-downloading dependencies - - ./.m2:/root/.m2 - working_dir: /cassandra - # Run full suite by default, or specific test if TEST_NAME_REGEX is set - command: > - sh -c 'rm -rf build/test/output/* && if [ -n "$TEST_NAME_REGEX" ]; then - .build/run-tests.sh -a test -t "$TEST_NAME_REGEX"; - else - ant test; - fi' - environment: - # Set TEST_NAME_REGEX to run a specific test, e.g.: - # TEST_NAME_REGEX=GarbageCollectRepairedSSTablesTest docker compose up - - TEST_NAME_REGEX=${TEST_NAME_REGEX:-} - # Allow container to use more resources - deploy: - resources: - limits: - memory: 8g - reservations: - memory: 4g From b47afd742dba065fd54707aacfe7b836cdaa552b Mon Sep 17 00:00:00 2001 From: Joaquin Casares Date: Wed, 21 Jan 2026 02:00:54 -0600 Subject: [PATCH 10/10] finish rename --- ...st.java => CompactionGarbageCollectOnlyPurgeRepairedTest.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/unit/org/apache/cassandra/db/compaction/{GarbageCollectRepairedSSTablesTest.java => CompactionGarbageCollectOnlyPurgeRepairedTest.java} (100%) diff --git a/test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionGarbageCollectOnlyPurgeRepairedTest.java similarity index 100% rename from test/unit/org/apache/cassandra/db/compaction/GarbageCollectRepairedSSTablesTest.java rename to test/unit/org/apache/cassandra/db/compaction/CompactionGarbageCollectOnlyPurgeRepairedTest.java