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,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)

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.

src/java/org/apache/cassandra/db/Directories.java

@@ -35,6 +35,7 @@
 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 static File getSnapshotDirectory(File location, String snapshotName)
         }
     }
 
+    /**
+     * 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 <cf dir>/snapshots/<snapshot name>/.<index name>}.
+     * Otherwise, this will return {@code <cf dir>/snapshots/<snapshot name>}.
+     *
+     * @param location base directory
+     * @param snapshotName snapshot name
+     * @return directory to write a snapshot
+     */
+    public static Optional<File> 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 static File getBackupsDirectory(Descriptor desc)
         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 <cf dir>/backups/.<index name>}.
+     * Otherwise, this will return {@code <cf dir>/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 static File getBackupsDirectory(File location)
         }
     }
 
+    /**
+     * 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 <cf dir>/backups/.<index name>}.
+     * Otherwise, this will return {@code <cf dir>/backups/}.
+     *
+     * @param location base directory
+     * @return directory to write a backup
+     */
+    public static Optional<File> 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,15 +1076,19 @@ private void filter()
 
                 if (snapshotName != null)
                 {
-                    LifecycleTransaction.getFiles(getSnapshotDirectory(location, snapshotName).toPath(), getFilter(), onTxnErr);
+                    Optional<File> maybeSnapshotDir = getSnapshotDirectoryIfExists(location, snapshotName);
+                    maybeSnapshotDir.ifPresent(dir -> LifecycleTransaction.getFiles(dir.toPath(), getFilter(), onTxnErr));
                     continue;
                 }
 
                 if (!onlyBackups)
                     LifecycleTransaction.getFiles(location.toPath(), getFilter(), onTxnErr);
 
                 if (includeBackups)
-                    LifecycleTransaction.getFiles(getBackupsDirectory(location).toPath(), getFilter(), onTxnErr);
+                {
+                    Optional<File> maybeBackupsDir = getBackupsDirectoryIfExists(location);
+                    maybeBackupsDir.ifPresent(dir -> LifecycleTransaction.getFiles(dir.toPath(), getFilter(), onTxnErr));
+                }
             }
 
             filtered = true;
@@ -1326,6 +1386,12 @@ else if (!dir.tryCreateDirectories() && !(dir.exists() && dir.isDirectory()))
         return dir;
     }
 
+    public static Optional<File> 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());

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

test/distributed/org/apache/cassandra/distributed/impl/InstanceConfig.java

@@ -146,6 +146,8 @@ private InstanceConfig(int num,
 
                 .set("concurrent_compactors", "8")
 
+                .set("uuid_sstable_identifiers_enabled", "true")
+
                 .set("stream_entire_sstables", "true")
 
                 .set("default_secondary_index", "sai")

test/distributed/org/apache/cassandra/distributed/test/TableLevelIncrementalBackupsTest.java

@@ -22,15 +22,18 @@
 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 @@ private void disableCompaction(Cluster cluster, String keyspace, String table)
             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<Descriptor> descs, String tableName, int expectedSeqGenIds)
+    private static void assertSSTablesCount(Set<Descriptor> descs, String tableName, int expectedTablesCount)
     {
+        Predicate<Descriptor> descriptorPredicate = descriptor -> {
+            if (DatabaseDescriptor.isUUIDSSTableIdentifiersEnabled())
+                return descriptor.id instanceof UUIDBasedSSTableId;
+            else
+                return descriptor.id instanceof SequenceBasedSSTableId;
+        };
+
         List<String> 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)

test/unit/org/apache/cassandra/index/CustomIndexTest.java

@@ -691,7 +691,7 @@ public void testFailing2iFlush() throws Throwable
         }
 
         // SSTables remain uncommitted.
-        assertEquals(1, getCurrentColumnFamilyStore().getDirectories().getDirectoryForNewSSTables().tryList().length);
+        assertEquals(0, getCurrentColumnFamilyStore().getDirectories().getDirectoryForNewSSTables().tryList().length);
     }
 
     @Test