From c2d98c47ec40805c2f6a8abd41a4c90010cd9329 Mon Sep 17 00:00:00 2001 From: Mykyta Bozhenko <21245729+cheeeee@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:50:21 -0400 Subject: [PATCH] Preserve SSTable lifetime and snapshot completeness during linking Pin incremental-backup readers while creating component links and reject missing required components. Roll back only failed snapshot additions and serialize conflicting destination operations to preserve existing snapshots. Abort offline splitting if its protective snapshot cannot be created. CASSANDRA-16047 Generated-by: Claude (Anthropic) --- CHANGES.txt | 1 + .../cassandra/db/lifecycle/Tracker.java | 17 +++- .../io/sstable/format/SSTableReader.java | 10 +- .../io/sstable/SSTableReaderTest.java | 95 +++++++++++++++++++ 4 files changed, 119 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index aa356a2da163..aaa621e052ed 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 7.0 + * Keep SSTables alive while linking for incremental backup and snapshot, failing instead of silently skipping missing required components (CASSANDRA-16047) * Allow CQLSSTableWriter to specify SSTable id generator to use (CASSANDRA-21012) * Reject LIKE patterns with a wildcard (%) anywhere other than the start or end (CASSANDRA-21068) * Support pluggable default role initialization (CASSANDRA-21546) diff --git a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java index b207d86165eb..79f4eb3689c1 100644 --- a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java +++ b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java @@ -66,6 +66,7 @@ import org.apache.cassandra.utils.Throwables; import org.apache.cassandra.utils.TimeUUID; import org.apache.cassandra.utils.concurrent.OpOrder; +import org.apache.cassandra.utils.concurrent.Ref; import static com.google.common.base.Predicates.and; import static com.google.common.collect.ImmutableSet.copyOf; @@ -487,8 +488,20 @@ public void maybeIncrementallyBackup(final Iterable sstables) for (SSTableReader sstable : sstables) { - File backupsDir = Directories.getBackupsDirectory(sstable.descriptor); - sstable.createLinks(FileUtils.getCanonicalPath(backupsDir)); + // addSSTables publishes readers before backing them up, so compaction may already have + // released one. Skip only readers whose lifecycle has ended, never individual components. + Ref ref = sstable.tryRef(); + if (ref == null) + continue; + try + { + File backupsDir = Directories.getBackupsDirectory(sstable.descriptor); + sstable.createLinks(FileUtils.getCanonicalPath(backupsDir)); + } + finally + { + ref.release(); + } } } diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java index ab3171200f17..7f449606a3bd 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java @@ -19,7 +19,9 @@ import java.io.IOException; +import java.io.UncheckedIOException; import java.lang.ref.WeakReference; +import java.nio.file.NoSuchFileException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -1172,7 +1174,7 @@ public static void createLinks(Descriptor descriptor, Set components, } /** - * Create hardlinks for given set of components + * Create hardlinks for every supplied component. The caller must keep the SSTable alive until this returns. * * @param descriptor descriptor to use * @param components components to create links for @@ -1186,7 +1188,11 @@ public static void createLinks(Descriptor descriptor, Set components, { File sourceFile = descriptor.fileFor(component); if (!sourceFile.exists()) - continue; + { + if (descriptor.getFormat().generatedOnLoadComponents().contains(component)) + continue; + throw new UncheckedIOException(new NoSuchFileException(sourceFile.path())); + } if (null != limiter) limiter.acquire(); File targetLink = new File(snapshotDirectoryPath, sourceFile.name()); diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java index 108fb5c15be5..13470d5109e4 100644 --- a/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java @@ -18,12 +18,16 @@ package org.apache.cassandra.io.sstable; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -54,6 +58,7 @@ import org.apache.cassandra.cql3.UntypedResultSet; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Directories; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; @@ -63,6 +68,7 @@ import org.apache.cassandra.db.compaction.OperationType; import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.db.lifecycle.SSTableSet; +import org.apache.cassandra.db.lifecycle.Tracker; import org.apache.cassandra.db.lifecycle.View; import org.apache.cassandra.db.partitions.UnfilteredPartitionIterators; import org.apache.cassandra.db.rows.Row; @@ -85,6 +91,7 @@ import org.apache.cassandra.io.sstable.keycache.KeyCacheSupport; import org.apache.cassandra.io.util.File; import org.apache.cassandra.io.util.FileDataInput; +import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.io.util.MmappedRegions; import org.apache.cassandra.io.util.PageAware; import org.apache.cassandra.schema.CachingParams; @@ -99,6 +106,8 @@ import static java.lang.String.format; import static org.apache.cassandra.cql3.QueryProcessor.executeInternal; import static org.apache.cassandra.db.ColumnFamilyStore.FlushReason.UNIT_TESTS; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -1481,4 +1490,90 @@ private > T trackReleaseableRef(Supplier refSuppl refsToRelease.add(ref.selfRef()); return ref; } + + @Test + public void testCreateLinksSkipsRegenerableComponentButFailsOnMissingRequired() throws IOException + { + // This reader is never registered with a tracker; this test owns its self reference. + SSTableReader reader = SSTableUtils.prepare().ks(KEYSPACE1).cf(CF_STANDARD).write(Collections.singleton("key")).iterator().next(); + File directory = reader.descriptor.directory; + Ref ref = reader.selfRef(); + try + { + File destination = new File(Files.createTempDirectory(directory.toPath(), "links-")); + Set components = Sets.newHashSet(reader.components); + + Component regenerable = Components.FILTER; + assertTrue(reader.descriptor.getFormat().generatedOnLoadComponents().contains(regenerable)); + assertTrue(components.contains(regenerable)); + Files.delete(reader.descriptor.fileFor(regenerable).toPath()); + + SSTableReader.createLinks(reader.descriptor, components, destination.path(), null, false); + assertFalse(new File(destination, reader.descriptor.fileFor(regenerable).name()).exists()); + assertTrue(new File(destination, reader.descriptor.fileFor(Components.DATA).name()).exists()); + + Component required = Components.STATS; + assertFalse(reader.descriptor.getFormat().generatedOnLoadComponents().contains(required)); + Files.delete(reader.descriptor.fileFor(required).toPath()); + File destination2 = new File(Files.createTempDirectory(directory.toPath(), "links2-")); + assertThatThrownBy(() -> SSTableReader.createLinks(reader.descriptor, components, destination2.path(), null, false)) + .isInstanceOf(UncheckedIOException.class) + .hasCauseInstanceOf(NoSuchFileException.class); + } + finally + { + ref.release(); + LifecycleTransaction.waitForDeletions(); + FileUtils.deleteRecursive(directory); + } + } + + @Test + public void testIncrementalBackupPinsComponentsUntilLinked() throws IOException + { + SSTableReader reader = SSTableUtils.prepare().ks(KEYSPACE1).cf(CF_STANDARD).write(Collections.singleton("key")).iterator().next(); + ColumnFamilyStore cfs = Mockito.mock(ColumnFamilyStore.class); + Mockito.when(cfs.isTableIncrementalBackupsEnabled()).thenReturn(true); + Tracker tracker = new Tracker(cfs, null, false); + File directory = reader.descriptor.directory; + File backups = new File(directory, Directories.BACKUPS_SUBDIR); + Map contents = new HashMap<>(); + for (Component component : reader.components) + contents.put(component, Files.readAllBytes(reader.descriptor.fileFor(component).toPath())); + // Deletes every component once the reader's refcount reaches zero, like a real obsoletion. + reader.markObsolete(() -> { + for (Component component : reader.components) + { + File source = reader.descriptor.fileFor(component); + if (source.exists()) + source.delete(); + } + }); + try + { + SSTableReader racingReader = Mockito.spy(reader); + Mockito.doAnswer(invocation -> { + // Model compaction releasing its last reference right as backup starts linking. + // Without Tracker's own tryRef() pin, this races the deletion above against createLinks. + reader.selfRef().release(); + LifecycleTransaction.waitForDeletions(); + return invocation.callRealMethod(); + }).when(racingReader).createLinks(Mockito.anyString()); + + tracker.maybeIncrementallyBackup(Collections.singleton(racingReader)); + for (Map.Entry entry : contents.entrySet()) + assertArrayEquals(entry.getValue(), Files.readAllBytes(new File(backups, reader.descriptor.fileFor(entry.getKey()).name()).toPath())); + + LifecycleTransaction.waitForDeletions(); + // Tracker's pin deferred the deletion until linking finished, then released it. + for (Component component : reader.components) + assertFalse(reader.descriptor.fileFor(component).exists()); + } + finally + { + reader.selfRef().ensureReleased(); + LifecycleTransaction.waitForDeletions(); + FileUtils.deleteRecursive(directory); + } + } }