This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch cassandra-4.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-4.0 by this push:
new d0851786a5 Delete SSTable components oldest-first so a partially
deleted SSTable's transaction log is not reported as corrupt
d0851786a5 is described below
commit d0851786a556b9334c33194c1f6fb2f40616fb20
Author: nivy <[email protected]>
AuthorDate: Tue Nov 3 11:18:29 2026 -0800
Delete SSTable components oldest-first so a partially deleted SSTable's
transaction log is not reported as corrupt
A REMOVE record in a transaction log stores the newest last-modified time
among the SSTable's files. On startup, LogFile compares that value against
the files still on disk. If they differ, the log is reported as corrupt.
SSTableTidier deleted the Data component first and then the remaining
components in arbitrary order. If the node stopped partway through, the
newest remaining file could be older than the recorded time, and startup
failed.
Sort the components by last-modified time and delete the oldest first,
so after any partial delete the newest remaining file still matches
the REMOVE record and the leftover files are cleaned up normally.
patch by Nivy Kani; reviewed by Caleb Rackliffe and Jon Meredith for
CASSANDRA-21649
---
CHANGES.txt | 1 +
.../cassandra/db/lifecycle/LogTransaction.java | 10 +-
.../org/apache/cassandra/io/sstable/SSTable.java | 24 ++--
.../lifecycle/SSTableTidierPartialDeleteTest.java | 133 +++++++++++++++++++++
4 files changed, 150 insertions(+), 18 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 9ec6355e54..506e707cd4 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0.22
+ * Delete SSTable files in order of last-updated timestamp (CASSANDRA-21649)
* Prevent negative BufferPool overflow memory accounting (CASSANDRA-21815)
* Respect cassandra.skip_sync in LogReplica, SSTable.appendTOC and
ChecksumWriter (CASSANDRA-21835)
* Fix cassandra-env.sh treating Xmx/Xms/Xmn inside paths as heap flags
(CASSANDRA-21804)
diff --git a/src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java
b/src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java
index 43b9e1f771..0a1804324b 100644
--- a/src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java
+++ b/src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java
@@ -386,18 +386,14 @@ class LogTransaction extends
Transactional.AbstractTransactional implements Tran
{
try
{
- // If we can't successfully delete the DATA component, set
the task to be retried later: see TransactionTidier
- File datafile = new File(desc.filenameFor(Component.DATA));
-
+ // If we can't successfully delete all components, set the
task to be retried later: see TransactionTidier
if (logger.isTraceEnabled())
logger.trace("Tidier running for old sstable {}",
desc.baseFilename());
- if (datafile.exists())
- delete(datafile);
- else if (!wasNew)
+ if (!new File(desc.filenameFor(Component.DATA)).exists()
&& !wasNew)
logger.error("SSTableTidier ran with no existing data
file for an sstable that was not new");
- // let the remainder be cleaned up by delete
+ // delete all components, DATA included, oldest first so
the REMOVE record stays valid on partial deletes
SSTable.delete(desc, SSTable.discoverComponentsFor(desc));
}
catch (Throwable t)
diff --git a/src/java/org/apache/cassandra/io/sstable/SSTable.java
b/src/java/org/apache/cassandra/io/sstable/SSTable.java
index 7bbcedc9fa..f80b66cc71 100644
--- a/src/java/org/apache/cassandra/io/sstable/SSTable.java
+++ b/src/java/org/apache/cassandra/io/sstable/SSTable.java
@@ -110,19 +110,21 @@ public abstract class SSTable
public static boolean delete(Descriptor desc, Set<Component> components)
{
logger.info("Deleting sstable: {}", desc);
- // remove the DATA component first if it exists
- if (components.contains(Component.DATA))
- FileUtils.deleteWithConfirm(desc.filenameFor(Component.DATA));
- for (Component component : components)
- {
- if (component.equals(Component.DATA) ||
component.equals(Component.SUMMARY))
- continue;
- FileUtils.deleteWithConfirm(desc.filenameFor(component));
- }
+ // delete older files first so the overall SSTable timestamp stays the
same on partial deletes
+ List<Component> sorted = new ArrayList<>(components);
+ Map<Component, Long> lastModified = new HashMap<>();
+ for (Component component : sorted)
+ lastModified.put(component, new
File(desc.filenameFor(component)).lastModified());
+ sorted.sort(Comparator.comparingLong(lastModified::get));
- if (components.contains(Component.SUMMARY))
- FileUtils.delete(desc.filenameFor(Component.SUMMARY));
+ for (Component component : sorted)
+ {
+ if (component.equals(Component.SUMMARY))
+ FileUtils.delete(desc.filenameFor(component));
+ else
+ FileUtils.deleteWithConfirm(desc.filenameFor(component));
+ }
return true;
}
diff --git
a/test/unit/org/apache/cassandra/db/lifecycle/SSTableTidierPartialDeleteTest.java
b/test/unit/org/apache/cassandra/db/lifecycle/SSTableTidierPartialDeleteTest.java
new file mode 100644
index 0000000000..573464d9b2
--- /dev/null
+++
b/test/unit/org/apache/cassandra/db/lifecycle/SSTableTidierPartialDeleteTest.java
@@ -0,0 +1,133 @@
+/*
+ * 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.lifecycle;
+
+import java.io.File;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.jboss.byteman.contrib.bmunit.BMRule;
+import org.jboss.byteman.contrib.bmunit.BMRules;
+import org.jboss.byteman.contrib.bmunit.BMUnitRunner;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Directories;
+import org.apache.cassandra.db.commitlog.CommitLog;
+import org.apache.cassandra.db.compaction.OperationType;
+import org.apache.cassandra.io.sstable.Component;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.schema.MockSchema;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Verifies that when the {@link LogTransaction.SSTableTidier} fails partway
through deleting an sstable,
+ * the transaction log is kept and still matches the files left on disk, so
both the tidier's retry
+ * and the startup cleanup finish the deletion.
+ */
+@RunWith(BMUnitRunner.class)
+@BMRules(rules = {
+ @BMRule(name = "Arm a delete failure after the given number of successful
deletes",
+ targetClass =
"org.apache.cassandra.db.lifecycle.SSTableTidierPartialDeleteTest",
+ targetMethod = "runTidier",
+ targetLocation = "AT ENTRY",
+ action = "createCountDown(\"deletes\", $1);"),
+ @BMRule(name = "Fail the delete once the countdown runs out",
+ targetClass = "org.apache.cassandra.io.util.FileUtils",
+ targetMethod = "deleteWithConfirm(java.lang.String)",
+ targetLocation = "AT ENTRY",
+ condition = "countDown(\"deletes\")",
+ action = "throw new org.apache.cassandra.io.FSWriteError(new
java.io.IOException(\"simulated delete failure\"), $1);")
+})
+public class SSTableTidierPartialDeleteTest
+{
+ @BeforeClass
+ public static void setUp()
+ {
+ DatabaseDescriptor.daemonInitialization();
+ CommitLog.instance.start();
+ MockSchema.cleanup();
+ }
+
+ @Test
+ public void testPartialDeleteIsFinishedByRetryOrStartupCleanup()
+ {
+ // fail after each number of successful deletes that leaves files
behind (there are 4 components in MockSchema)
+ for (int deletes = 1; deletes < 4; deletes++)
+ {
+ assertPartialDeleteFinished(deletes, false);
+ assertPartialDeleteFinished(deletes, true);
+ }
+ }
+
+ private static void assertPartialDeleteFinished(int deletes, boolean
restart)
+ {
+ ColumnFamilyStore cfs = MockSchema.newCFS();
+ SSTableReader sstableOld = MockSchema.sstable(0, true, cfs);
+ SSTableReader sstableNew = MockSchema.sstable(1, true, cfs);
+ List<File> files =
sstableOld.getAllFilePaths().stream().map(File::new).collect(Collectors.toList());
+
+ // make DATA the newest file, so deleting it early would cause the
REMOVE record to have a different timestamp
+ File data = new
File(sstableOld.descriptor.filenameFor(Component.DATA));
+ long newest =
files.stream().mapToLong(File::lastModified).max().getAsLong();
+ assertTrue(data.setLastModified(newest + 1000));
+
+ LogTransaction log = new LogTransaction(OperationType.COMPACTION);
+ log.trackNew(sstableNew);
+ LogTransaction.SSTableTidier tidier = log.obsoleted(sstableOld);
+ List<File> logFiles = log.txnFile().getFiles();
+ log.finish();
+
+ runTidier(deletes, tidier, sstableOld);
+
+ // the tidier failed partway, but DATA is deleted last so the txn log
still matches the files on disk
+ assertEquals(files.size() - deletes,
files.stream().filter(File::exists).count());
+ assertTrue("DATA should be deleted last", data.exists());
+ for (File logFile : logFiles)
+ assertTrue("txn log invalid after " + deletes + " deletes",
LogFile.make(logFile).verify());
+
+ if (restart)
+ {
+ // the startup txn log replay finishes the deletion, and the old
sstable isn't picked up as live
+
assertTrue(LogTransaction.removeUnfinishedLeftovers(cfs.metadata()));
+ assertEquals(Collections.singleton(sstableNew.descriptor),
+ new
Directories(cfs.metadata()).sstableLister(Directories.OnTxnErr.THROW).list().keySet());
+ }
+
+ // the tidier's retry finishes the deletion (or finds nothing left
after a restart) and releases the txn log
+ sstableNew.selfRef().release();
+ LogTransaction.rescheduleFailedDeletions();
+ LogTransaction.waitForDeletions();
+ assertTrue(Stream.concat(files.stream(),
logFiles.stream()).noneMatch(File::exists));
+ }
+
+ // the BMRules above use deletesBeforeFailure to fail the next delete
+ private static void runTidier(int deletesBeforeFailure,
LogTransaction.SSTableTidier tidier, SSTableReader sstable)
+ {
+ sstable.markObsolete(tidier);
+ sstable.selfRef().release();
+ LogTransaction.waitForDeletions();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
