From 989901e4e0953e7207f1cae3652fba278a48108c Mon Sep 17 00:00:00 2001 From: Matt Byrd Date: Fri, 13 Feb 2026 17:15:45 +0000 Subject: [PATCH] Ensure schema created before 2.1 without tableId in folder name can be loaded in SnapshotLoader Best effort attempt to get tableId from CFS, fallback to null which, still allows snapshot to be loaded End result is that nodetool clearsnapshot and listsnapshot function for such snapshots on restart. patch by Matt Byrd; reviewed by Stefan Miklosovic, Brandon Williams and Francisco Guerrero for CASSANDRA-21173 --- CHANGES.txt | 1 + .../service/snapshot/SnapshotLoader.java | 45 ++++++++++++++++++- .../service/snapshot/TableSnapshot.java | 15 +++++-- .../service/snapshot/SnapshotLoaderTest.java | 18 ++++++-- 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c87274ab21..4c79e1cd5a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.8 + * Ensure schema created before 2.1 without tableId in folder name can be loaded in SnapshotLoader (CASSANDRA-21173) Merged from 4.1: Merged from 4.0: diff --git a/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java b/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java index d31df361e5..7ffe199c89 100644 --- a/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java +++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java @@ -41,8 +41,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Directories; +import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.io.util.File; +import org.apache.cassandra.schema.Schema; import static org.apache.cassandra.db.Directories.SNAPSHOT_SUBDIR; import static org.apache.cassandra.service.snapshot.TableSnapshot.buildSnapshotId; @@ -54,7 +57,9 @@ public class SnapshotLoader { private static final Logger logger = LoggerFactory.getLogger(SnapshotLoader.class); - static final Pattern SNAPSHOT_DIR_PATTERN = Pattern.compile("(?\\w+)/(?\\w+)-(?[0-9a-f]{32})/snapshots/(?.+)$"); + static final Pattern SNAPSHOT_DIR_PATTERN = Pattern.compile("(?\\w+)/(?\\w+)" + + "(-(?[0-9a-f]{32}))?" + + "/snapshots/(?.+)$"); private final Collection dataDirectories; @@ -149,12 +154,48 @@ public class SnapshotLoader { String keyspaceName = snapshotDirMatcher.group("keyspace"); String tableName = snapshotDirMatcher.group("tableName"); - UUID tableId = parseUUID(snapshotDirMatcher.group("tableId")); + final UUID tableId = maybeDetermineTableId(snapshotDirMatcher, snapshotDir, keyspaceName, tableName); String tag = snapshotDirMatcher.group("tag"); String snapshotId = buildSnapshotId(keyspaceName, tableName, tableId, tag); TableSnapshot.Builder builder = snapshots.computeIfAbsent(snapshotId, k -> new TableSnapshot.Builder(keyspaceName, tableName, tableId, tag)); builder.addSnapshotDir(new File(snapshotDir)); } + + private UUID maybeDetermineTableId(Matcher snapshotDirMatcher, Path snapshotDir, String keyspaceName, String tableName) + { + final UUID tableId; + if (snapshotDirMatcher.group("tableId") == null) + { + logger.debug("Snapshot directory without tableId found (pre-2.1 format): {}", snapshotDir); + // If we don't have a tableId in folder name (e.g pre 2.1 created table) + // Then attempt to get tableId from CFS on startup + // falling back to null is fine as it still yields a unique result in buildSnapshotId for pre-2.1 table + if (Keyspace.isInitialized() && Schema.instance.getKeyspaceMetadata(keyspaceName) != null) + { + ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(keyspaceName, tableName); + tableId = cfs != null && cfs.metadata.id != null + ? cfs.metadata.id.asUUID() + : null; + + if (tableId == null) + { + logger.warn("Snapshot directory without tableId found (pre-2.1 format), " + + "unable to resolve table id from column family, defaulting to null, snapshot dir: {}", snapshotDir); + } + } + else + { + logger.warn("Snapshot directory without tableId found (pre-2.1 format), " + + "keyspace is not initialized or there is a schema missing, defaulting to null, snapshot dir: {}", snapshotDir); + tableId = null; + } + } + else + { + tableId = parseUUID(snapshotDirMatcher.group("tableId")); + } + return tableId; + } } public Set loadSnapshots(String keyspace) diff --git a/src/java/org/apache/cassandra/service/snapshot/TableSnapshot.java b/src/java/org/apache/cassandra/service/snapshot/TableSnapshot.java index d10092a41a..a048815c19 100644 --- a/src/java/org/apache/cassandra/service/snapshot/TableSnapshot.java +++ b/src/java/org/apache/cassandra/service/snapshot/TableSnapshot.java @@ -29,6 +29,8 @@ import java.util.Set; import java.util.UUID; import java.util.function.Predicate; +import javax.annotation.Nullable; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +45,9 @@ public class TableSnapshot private final String keyspaceName; private final String tableName; + // tableId may be null under some rare circumstance namely pre-2.1 table + // whose snapshot is loaded upon startup rather than created while the jvm is running + @Nullable private final UUID tableId; private final String tag; private final boolean ephemeral; @@ -70,7 +75,8 @@ public class TableSnapshot * Unique identifier of a snapshot. Used * only to deduplicate snapshots internally, * not exposed externally. - * + * table_id may be empty for tables created prior to 2.1 + *

* Format: "$ks:$table_name:$table_id:$tag" */ public String getId() @@ -224,7 +230,8 @@ public class TableSnapshot '}'; } - static class Builder { + static class Builder + { private final String keyspaceName; private final String tableName; private final UUID tableId; @@ -282,9 +289,9 @@ public class TableSnapshot } } - protected static String buildSnapshotId(String keyspaceName, String tableName, UUID tableId, String tag) + protected static String buildSnapshotId(String keyspaceName, String tableName, @Nullable UUID tableId, String tag) { - return String.format("%s:%s:%s:%s", keyspaceName, tableName, tableId, tag); + return String.format("%s:%s:%s:%s", keyspaceName, tableName, tableId == null ? "" : tableId, tag); } public static class SnapshotTrueSizeCalculator extends DirectorySizeCalculator diff --git a/test/unit/org/apache/cassandra/service/snapshot/SnapshotLoaderTest.java b/test/unit/org/apache/cassandra/service/snapshot/SnapshotLoaderTest.java index 9c2303f842..c39b2b9ec3 100644 --- a/test/unit/org/apache/cassandra/service/snapshot/SnapshotLoaderTest.java +++ b/test/unit/org/apache/cassandra/service/snapshot/SnapshotLoaderTest.java @@ -28,12 +28,15 @@ import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.DurationSpec; import org.apache.cassandra.db.Directories; +import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.io.util.File; import org.assertj.core.util.Lists; @@ -67,6 +70,13 @@ public class SnapshotLoaderTest @ClassRule public static TemporaryFolder tmpDir = new TemporaryFolder(); + @BeforeClass + public static void setup() + { + DatabaseDescriptor.daemonInitialization(); + Keyspace.setInitialized(); + } + @Test public void testMatcher() { @@ -144,6 +154,7 @@ public class SnapshotLoaderTest snapshots = loader.loadSnapshots(KEYSPACE_2); assertThat(snapshots).hasSize(1); assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_2, TABLE3_NAME, TABLE3_ID, TAG3, null, null, tag3Files, false)); + } @Test @@ -192,7 +203,8 @@ public class SnapshotLoaderTest for (String dataDir : DATA_DIRS) { tag1Files.add(createDir(baseDir, dataDir, KEYSPACE_1, tableDirName(TABLE1_NAME, TABLE1_ID), Directories.SNAPSHOT_SUBDIR, TAG1)); - tag2Files.add(createDir(baseDir, dataDir, KEYSPACE_1, tableDirName(TABLE2_NAME, TABLE2_ID), Directories.SNAPSHOT_SUBDIR, TAG2)); + // One table is an old pre 2.1 table folder structure + tag2Files.add(createDir(baseDir, dataDir, KEYSPACE_1, TABLE2_NAME, Directories.SNAPSHOT_SUBDIR, TAG2)); tag3Files.add(createDir(baseDir, dataDir, KEYSPACE_2, tableDirName(TABLE3_NAME, TABLE3_ID), Directories.SNAPSHOT_SUBDIR, TAG3)); } @@ -219,7 +231,7 @@ public class SnapshotLoaderTest Set snapshots = loader.loadSnapshots(); assertThat(snapshots).hasSize(3); assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE1_NAME, TABLE1_ID, TAG1, tag1Ts, null, tag1Files, false)); - assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, TABLE2_ID, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false)); + assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, null, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false)); assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_2, TABLE3_NAME, TABLE3_ID, TAG3, tag3Ts, null, tag3Files, false)); // Verify snapshot loading for a specific keyspace @@ -230,7 +242,7 @@ public class SnapshotLoaderTest snapshots = loader.loadSnapshots(KEYSPACE_1); assertThat(snapshots).hasSize(2); assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE1_NAME, TABLE1_ID, TAG1, tag1Ts, null, tag1Files, false)); - assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, TABLE2_ID, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false)); + assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, null, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false)); loader = new SnapshotLoader(Arrays.asList(Paths.get(baseDir.toString(), DATA_DIR_1), Paths.get(baseDir.toString(), DATA_DIR_2),