diff --git a/CHANGES.txt b/CHANGES.txt index b26f2ae9a0..10fe1993a8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -208,6 +208,7 @@ * Add the ability to disable bulk loading of SSTables (CASSANDRA-18781) * Clean up obsolete functions and simplify cql_version handling in cqlsh (CASSANDRA-18787) Merged from 5.0: + * Sort SSTable TOC entries for determinism (CASSANDRA-20494) * Do not source cassandra-env.sh unnecessarily in nodetool and other tooling (CASSANDRA-20745) * Make source distribution buildable by ant artifacts as doc/scripts/process-native-protocol-specs-in-docker.sh was not executable (CASSANDRA-20802) * SSTableIndexWriter#abort() should log more quietly in cases where an exception is not provided (CASSANDRA-20695) diff --git a/src/java/org/apache/cassandra/io/sstable/SSTable.java b/src/java/org/apache/cassandra/io/sstable/SSTable.java index 475f92beeb..14c7af6cd5 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTable.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTable.java @@ -320,7 +320,7 @@ public abstract class SSTable public synchronized void addComponents(Collection newComponents) { Collection componentsToAdd = Collections2.filter(newComponents, Predicates.not(Predicates.in(components))); - TOCComponent.appendTOC(descriptor, componentsToAdd); + TOCComponent.updateTOC(descriptor, componentsToAdd); components.addAll(componentsToAdd); } @@ -332,7 +332,7 @@ public abstract class SSTable public synchronized void registerComponents(Collection newComponents, Tracker tracker) { Collection componentsToAdd = new HashSet<>(Collections2.filter(newComponents, x -> !components.contains(x))); - TOCComponent.appendTOC(descriptor, componentsToAdd); + TOCComponent.updateTOC(descriptor, componentsToAdd); components.addAll(componentsToAdd); for (Component component : componentsToAdd) @@ -345,7 +345,7 @@ public abstract class SSTable /** * Unregisters custom components from sstable and update size tracking - * @param removeComponents collection of components to be remove + * @param removeComponents collection of components to be removed * @param tracker used to update on-disk size metrics */ public synchronized void unregisterComponents(Collection removeComponents, Tracker tracker) diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java index a3f24c0e5b..113332b102 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java @@ -381,7 +381,7 @@ public abstract class SSTableWriter extends SSTable implements Transactional new StatsComponent(finalizeMetadata()).save(descriptor); // save the table of components - TOCComponent.appendTOC(descriptor, components); + TOCComponent.updateTOC(descriptor, components); if (openResult) finalReader = openFinal(SSTableReader.OpenReason.NORMAL); diff --git a/src/java/org/apache/cassandra/io/sstable/format/TOCComponent.java b/src/java/org/apache/cassandra/io/sstable/format/TOCComponent.java index 15d81d0ba1..629350d2fb 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/TOCComponent.java +++ b/src/java/org/apache/cassandra/io/sstable/format/TOCComponent.java @@ -21,13 +21,15 @@ package org.apache.cassandra.io.sstable.format; import java.io.FileNotFoundException; import java.io.IOError; import java.io.IOException; -import java.io.PrintWriter; import java.nio.file.Files; import java.nio.file.NoSuchFileException; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Set; +import java.util.TreeSet; +import com.google.common.collect.Collections2; import com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,9 +39,11 @@ import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.io.sstable.Descriptor; import org.apache.cassandra.io.sstable.format.SSTableFormat.Components; import org.apache.cassandra.io.util.File; -import org.apache.cassandra.io.util.FileOutputStreamPlus; +import org.apache.cassandra.io.util.FileUtils; -import static org.apache.cassandra.io.util.File.WriteMode.APPEND; +import static java.nio.file.StandardOpenOption.CREATE; +import static java.nio.file.StandardOpenOption.SYNC; +import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; public class TOCComponent { @@ -78,20 +82,30 @@ public class TOCComponent } /** - * Appends new component names to the TOC component. + * Updates the TOC file by reading existing component entries, merging them with the given components, + * sorting the combined list in lexicographic order for deterministic output. + * + * @param descriptor the SSTable descriptor for which to update the TOC + * @param components new components to merge into the TOC (existing TOC entries are preserved) + * @throws FSWriteError if an I/O error occurs when creating or overwriting the TOC file */ - public static void appendTOC(Descriptor descriptor, Collection components) + public static void updateTOC(Descriptor descriptor, Collection components) { + if (components.isEmpty()) + return; + File tocFile = descriptor.fileFor(Components.TOC); - try (FileOutputStreamPlus out = tocFile.newOutputStream(APPEND); - PrintWriter w = new PrintWriter(out)) + + Set componentNames = new TreeSet<>(Collections2.transform(components, Component::name)); + + if (tocFile.exists()) + componentNames.addAll(FileUtils.readLines(tocFile)); + + try { - for (Component component : components) - w.println(component.name); - w.flush(); - out.sync(); + FileUtils.write(tocFile, new ArrayList<>(componentNames), CREATE, TRUNCATE_EXISTING, SYNC); } - catch (IOException e) + catch (RuntimeException e) { throw new FSWriteError(e, tocFile); } @@ -112,7 +126,7 @@ public class TOCComponent return components; // sstable doesn't exist yet components.add(Components.TOC); - TOCComponent.appendTOC(descriptor, components); + TOCComponent.updateTOC(descriptor, components); return components; } } @@ -123,13 +137,13 @@ public class TOCComponent } /** - * Rewrite TOC components by deleting existing TOC file and append new components + * Rewrites the TOC component by deleting and recreating it only with provided component names. */ public static void rewriteTOC(Descriptor descriptor, Collection components) { File tocFile = descriptor.fileFor(Components.TOC); if (!tocFile.tryDelete()) - logger.error("Failed to delete TOC component for " + descriptor); - appendTOC(descriptor, components); + logger.error("Failed to delete TOC component for {}", descriptor); + updateTOC(descriptor, components); } } diff --git a/test/unit/org/apache/cassandra/io/sstable/format/TOCComponentTest.java b/test/unit/org/apache/cassandra/io/sstable/format/TOCComponentTest.java new file mode 100644 index 0000000000..1dd79532ea --- /dev/null +++ b/test/unit/org/apache/cassandra/io/sstable/format/TOCComponentTest.java @@ -0,0 +1,177 @@ +/* + * 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.io.sstable.format; + +import java.nio.file.Files; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.io.FSWriteError; +import org.apache.cassandra.io.sstable.Component; +import org.apache.cassandra.io.sstable.Descriptor; +import org.apache.cassandra.io.sstable.format.SSTableFormat.Components; +import org.apache.cassandra.io.util.File; +import org.apache.cassandra.io.util.FileUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class TOCComponentTest +{ + + @Rule + public TemporaryFolder tempDir = new TemporaryFolder(); + + @Before + public void setup() + { + DatabaseDescriptor.daemonInitialization(); + } + + @Test + public void testAppendComponentsInSortedOrder() throws Exception + { + File dataFile = new File(tempDir.newFile("nb-1-big-Data.db")); + File tocFile = new File(tempDir.getRoot().toPath().resolve("nb-1-big-TOC.txt")); + Descriptor descriptor = Descriptor.fromFile(dataFile); + Set components = Set.of(Components.DATA, Components.STATS, Components.DIGEST, + Components.TOC, Components.COMPRESSION_INFO, Components.FILTER, Components.CRC); + TOCComponent.updateTOC(descriptor, components); + List lines = Files.readAllLines(tocFile.toPath()); + assertThat(lines).containsExactly( + "CRC.db", + "CompressionInfo.db", + "Data.db", + "Digest.crc32", + "Filter.db", + "Statistics.db", + "TOC.txt" + ); + } + + @Test + public void testUpdateTOCIdempotent() throws Exception + { + File dataFile = new File(tempDir.newFile("nb-1-big-Data.db")); + File tocFile = new File(tempDir.getRoot().toPath().resolve("nb-1-big-TOC.txt")); + Descriptor descriptor = Descriptor.fromFile(dataFile); + Set components = Set.of(Components.DATA, Components.STATS, Components.FILTER); + // call twice to check no duplication + TOCComponent.updateTOC(descriptor, components); + TOCComponent.updateTOC(descriptor, components); + List lines = Files.readAllLines(tocFile.toPath()); + assertThat(lines).containsExactly("Data.db", "Filter.db", "Statistics.db"); + } + + @Test(expected = FSWriteError.class) + public void testUpdateTOCNonExistentDirectory() + { + File dataFile = new File(tempDir.getRoot(), "nonexistent/nb-1-big-Data.db"); + Descriptor descriptor = Descriptor.fromFile(dataFile); + TOCComponent.updateTOC(descriptor, Set.of(Components.DATA)); + } + + @Test + public void testLoadOrCreateSSTableExistButNotToc() throws Exception + { + java.io.File dataFile = tempDir.newFile("nb-1-big-Data.db"); + Descriptor descriptor = Descriptor.fromFile(new File(dataFile)); + + Set components = TOCComponent.loadOrCreate(descriptor); + assertThat(components).containsExactlyInAnyOrder(Components.DATA, Components.TOC); + + File tocFile = descriptor.fileFor(Components.TOC); + assertTrue(tocFile.exists()); + } + + @Test + public void testLoadOrCreateWithDiscoveredComponents() throws Exception + { + java.io.File dataFile = tempDir.newFile("nb-1-big-Data.db"); + tempDir.newFile("nb-1-big-Filter.db"); + + Descriptor descriptor = Descriptor.fromFile(new File(dataFile)); + Set components = TOCComponent.loadOrCreate(descriptor); + assertThat(components).containsExactlyInAnyOrder(Components.DATA, Components.FILTER, Components.TOC); + + File tocFile = descriptor.fileFor(Components.TOC); + assertTrue(tocFile.exists()); + + List lines = Files.readAllLines(tocFile.toPath()); + assertThat(lines).containsExactly("Data.db", "Filter.db", "TOC.txt"); + } + + @Test + public void testLoadOrCreateWhenNoComponents() + { + java.io.File dataFile = new java.io.File(tempDir.getRoot(), "nb-1-big-Data.db"); + Descriptor descriptor = Descriptor.fromFile(new File(dataFile)); + Set components = TOCComponent.loadOrCreate(descriptor); + assertTrue(components.isEmpty()); + + File tocFile = descriptor.fileFor(Components.TOC); + assertFalse(tocFile.exists()); + } + + private Set loadTOC(boolean skipMissing) throws Exception + { + File dataFile = new File(tempDir.newFile("nb-1-big-Data.db")); + File tocFile = new File(tempDir.newFile("nb-1-big-TOC.txt")); + Descriptor desc = Descriptor.fromFile(dataFile); + Files.write(tocFile.toPath(), List.of("Data.db", "Statistics.db")); + return TOCComponent.loadTOC(desc, skipMissing); + } + + @Test + public void testSkipMissingFalse() throws Exception + { + assertThat(loadTOC(false)).containsExactlyInAnyOrder(Components.DATA, Components.STATS); + } + + @Test + public void testSkipMissingTrue() throws Exception + { + assertThat(loadTOC(true)).containsExactlyInAnyOrder(Components.DATA); + } + + @Test + public void testRewriteTOCReplacesFileWithSortedComponents() throws Exception + { + // Base file for descriptor + Descriptor descriptor = Descriptor.fromFile(new File(tempDir.newFile("nb-1-big-Data.db"))); + + File tocFile = descriptor.fileFor(Components.TOC); + assertFalse(tocFile.exists()); + FileUtils.write(tocFile, Arrays.asList("Data-OLD.db", "Filter-OLD.db", "TOC-OLD.txt")); + assertThat(FileUtils.readLines(tocFile)).containsExactly("Data-OLD.db", "Filter-OLD.db", "TOC-OLD.txt"); + + Set newComponents = Set.of(Components.FILTER, Components.DATA, Components.TOC); + TOCComponent.rewriteTOC(descriptor, newComponents); + + List lines = Files.readAllLines(tocFile.toPath()); + assertThat(lines).containsExactly("Data.db", "Filter.db", "TOC.txt"); + } +}