From 2f836fa59687d79705c96d5836978c9266813780 Mon Sep 17 00:00:00 2001 From: Stefan Miklosovic Date: Fri, 8 Mar 2024 11:32:40 +0100 Subject: [PATCH] Set uuid_sstable_identifiers_enabled to true for cassandra-latest.yaml patch by Stefan Miklosovic; reviewed by Branimir Lambov, Jacek Lewandowski for CASSANDRA-19460 --- CHANGES.txt | 1 + conf/cassandra_latest.yaml | 2 +- .../org/apache/cassandra/db/Directories.java | 70 ++++++++++++++++++- test/conf/latest_diff.yaml | 2 + .../distributed/impl/InstanceConfig.java | 2 + .../TableLevelIncrementalBackupsTest.java | 20 ++++-- .../cassandra/index/CustomIndexTest.java | 2 +- 7 files changed, 90 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index cf542bbd58..2e543f1120 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0-beta2 + * Set uuid_sstable_identifiers_enabled to true in cassandra-latest.yaml (CASSANDRA-19460) * Revert switching to approxTime in Dispatcher (CASSANDRA-19454) * Add an optimized default configuration to tests and make it available for new users (CASSANDRA-18753) * Fix remote JMX under Java17 (CASSANDRA-19453) diff --git a/conf/cassandra_latest.yaml b/conf/cassandra_latest.yaml index d4f6c0f468..abac0d408c 100644 --- a/conf/cassandra_latest.yaml +++ b/conf/cassandra_latest.yaml @@ -1195,7 +1195,7 @@ sstable_preemptive_open_interval: 50MiB # set to true, each newly created sstable will have a UUID based generation identifier and such files are # not readable by previous Cassandra versions. At some point, this option will become true by default # and eventually get removed from the configuration. -uuid_sstable_identifiers_enabled: false +uuid_sstable_identifiers_enabled: true # When enabled, permits Cassandra to zero-copy stream entire eligible # SSTables between nodes, including every component. diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index 4e7347aa26..f6c77443ae 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -35,6 +35,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.Spliterator; import java.util.concurrent.ThreadLocalRandom; @@ -646,6 +647,29 @@ public class Directories } } + /** + * Returns directory to write a snapshot to. If directory does not exist, then it is NOT created. + * + * If given {@code location} indicates secondary index, this will return + * {@code /snapshots//.}. + * Otherwise, this will return {@code /snapshots/}. + * + * @param location base directory + * @param snapshotName snapshot name + * @return directory to write a snapshot + */ + public static Optional getSnapshotDirectoryIfExists(File location, String snapshotName) + { + if (isSecondaryIndexFolder(location)) + { + return get(location.parent(), SNAPSHOT_SUBDIR, snapshotName, location.name()); + } + else + { + return get(location, SNAPSHOT_SUBDIR, snapshotName); + } + } + public File getSnapshotManifestFile(String snapshotName) { File snapshotDir = getSnapshotDirectory(getDirectoryForNewSSTables(), snapshotName); @@ -673,6 +697,16 @@ public class Directories return getBackupsDirectory(desc.directory); } + /** + * Returns directory to write a backup to. If directory does not exist, then one is created. + * + * If given {@code location} indicates secondary index, this will return + * {@code /backups/.}. + * Otherwise, this will return {@code /backups/}. + * + * @param location base directory + * @return directory to write a backup + */ public static File getBackupsDirectory(File location) { if (isSecondaryIndexFolder(location)) @@ -685,6 +719,28 @@ public class Directories } } + /** + * Returns directory to write a backup to. If directory does not exist, then it is NOT created. + * + * If given {@code location} indicates secondary index, this will return + * {@code /backups/.}. + * Otherwise, this will return {@code /backups/}. + * + * @param location base directory + * @return directory to write a backup + */ + public static Optional getBackupsDirectoryIfExists(File location) + { + if (isSecondaryIndexFolder(location)) + { + return get(location.parent(), BACKUPS_SUBDIR, location.name()); + } + else + { + return get(location, BACKUPS_SUBDIR); + } + } + /** * Checks if the specified table should be stored with local system data. * @@ -1020,7 +1076,8 @@ public class Directories if (snapshotName != null) { - LifecycleTransaction.getFiles(getSnapshotDirectory(location, snapshotName).toPath(), getFilter(), onTxnErr); + Optional maybeSnapshotDir = getSnapshotDirectoryIfExists(location, snapshotName); + maybeSnapshotDir.ifPresent(dir -> LifecycleTransaction.getFiles(dir.toPath(), getFilter(), onTxnErr)); continue; } @@ -1028,7 +1085,10 @@ public class Directories LifecycleTransaction.getFiles(location.toPath(), getFilter(), onTxnErr); if (includeBackups) - LifecycleTransaction.getFiles(getBackupsDirectory(location).toPath(), getFilter(), onTxnErr); + { + Optional maybeBackupsDir = getBackupsDirectoryIfExists(location); + maybeBackupsDir.ifPresent(dir -> LifecycleTransaction.getFiles(dir.toPath(), getFilter(), onTxnErr)); + } } filtered = true; @@ -1326,6 +1386,12 @@ public class Directories return dir; } + public static Optional get(File base, String... subdirs) + { + File dir = subdirs == null || subdirs.length == 0 ? base : new File(base, join(subdirs)); + return dir.exists() ? Optional.of(dir) : Optional.empty(); + } + private static String join(String... s) { return StringUtils.join(s, File.pathSeparator()); diff --git a/test/conf/latest_diff.yaml b/test/conf/latest_diff.yaml index dc25a4139a..28cb76c971 100644 --- a/test/conf/latest_diff.yaml +++ b/test/conf/latest_diff.yaml @@ -53,6 +53,8 @@ default_compaction: concurrent_compactors: 8 +uuid_sstable_identifiers_enabled: true + stream_entire_sstables: true default_secondary_index: sai diff --git a/test/distributed/org/apache/cassandra/distributed/impl/InstanceConfig.java b/test/distributed/org/apache/cassandra/distributed/impl/InstanceConfig.java index a582731645..13889543d8 100644 --- a/test/distributed/org/apache/cassandra/distributed/impl/InstanceConfig.java +++ b/test/distributed/org/apache/cassandra/distributed/impl/InstanceConfig.java @@ -146,6 +146,8 @@ public class InstanceConfig implements IInstanceConfig .set("concurrent_compactors", "8") + .set("uuid_sstable_identifiers_enabled", "true") + .set("stream_entire_sstables", "true") .set("default_secondary_index", "sai") diff --git a/test/distributed/org/apache/cassandra/distributed/test/TableLevelIncrementalBackupsTest.java b/test/distributed/org/apache/cassandra/distributed/test/TableLevelIncrementalBackupsTest.java index 8e2f86622e..2bf6c5f350 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/TableLevelIncrementalBackupsTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/TableLevelIncrementalBackupsTest.java @@ -22,15 +22,18 @@ import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.junit.Test; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.Feature; import org.apache.cassandra.io.sstable.Descriptor; import org.apache.cassandra.io.sstable.SequenceBasedSSTableId; +import org.apache.cassandra.io.sstable.UUIDBasedSSTableId; import static org.apache.cassandra.Util.getBackups; import static org.apache.cassandra.distributed.Cluster.build; @@ -144,23 +147,30 @@ public class TableLevelIncrementalBackupsTest extends TestBaseImpl cluster.get(i).nodetool("disableautocompaction", keyspace, table); } - private static void assertBackupSSTablesCount(Cluster cluster, int expectedSeqGenIds, boolean enable, String ks, String... tableNames) + private static void assertBackupSSTablesCount(Cluster cluster, int expectedTablesCount, boolean enable, String ks, String... tableNames) { for (int i = 1; i < cluster.size() + 1; i++) { cluster.get(i).runOnInstance(rethrow(() -> Arrays.stream(tableNames).forEach(tableName -> assertTableMetaIncrementalBackupEnable(ks, tableName, enable)))); - cluster.get(i).runOnInstance(rethrow(() -> Arrays.stream(tableNames).forEach(tableName -> assertSSTablesCount(getBackups(ks, tableName), tableName, expectedSeqGenIds)))); + cluster.get(i).runOnInstance(rethrow(() -> Arrays.stream(tableNames).forEach(tableName -> assertSSTablesCount(getBackups(ks, tableName), tableName, expectedTablesCount)))); } } - private static void assertSSTablesCount(Set descs, String tableName, int expectedSeqGenIds) + private static void assertSSTablesCount(Set descs, String tableName, int expectedTablesCount) { + Predicate descriptorPredicate = descriptor -> { + if (DatabaseDescriptor.isUUIDSSTableIdentifiersEnabled()) + return descriptor.id instanceof UUIDBasedSSTableId; + else + return descriptor.id instanceof SequenceBasedSSTableId; + }; + List seqSSTables = descs.stream() - .filter(desc -> desc.id instanceof SequenceBasedSSTableId) + .filter(descriptorPredicate) .map(descriptor -> descriptor.baseFile().toString()) .sorted() .collect(Collectors.toList()); - assertThat(seqSSTables).describedAs("SSTables of %s with sequence based id", tableName).hasSize(expectedSeqGenIds); + assertThat(seqSSTables).describedAs("SSTables of %s with sequence based id", tableName).hasSize(expectedTablesCount); } private static void assertTableMetaIncrementalBackupEnable(String ks, String tableName, boolean enable) diff --git a/test/unit/org/apache/cassandra/index/CustomIndexTest.java b/test/unit/org/apache/cassandra/index/CustomIndexTest.java index a540c5d9e5..9f568a8773 100644 --- a/test/unit/org/apache/cassandra/index/CustomIndexTest.java +++ b/test/unit/org/apache/cassandra/index/CustomIndexTest.java @@ -691,7 +691,7 @@ public class CustomIndexTest extends CQLTester } // SSTables remain uncommitted. - assertEquals(1, getCurrentColumnFamilyStore().getDirectories().getDirectoryForNewSSTables().tryList().length); + assertEquals(0, getCurrentColumnFamilyStore().getDirectories().getDirectoryForNewSSTables().tryList().length); } @Test