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