Skip to content

Commit 989901e

Browse files
Matt Byrdsmiklosovic
Matt Byrd
authored and
smiklosovic
committed
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
1 parent 92c69ef commit 989901e

File tree

4 files changed

+70
-9
lines changed

4 files changed

+70
-9
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
5.0.8
2+
* Ensure schema created before 2.1 without tableId in folder name can be loaded in SnapshotLoader (CASSANDRA-21173)
23
Merged from 4.1:
34
Merged from 4.0:
45

‎src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,11 @@
4141
import org.slf4j.LoggerFactory;
4242

4343
import org.apache.cassandra.config.DatabaseDescriptor;
44+
import org.apache.cassandra.db.ColumnFamilyStore;
4445
import org.apache.cassandra.db.Directories;
46+
import org.apache.cassandra.db.Keyspace;
4547
import org.apache.cassandra.io.util.File;
48+
import org.apache.cassandra.schema.Schema;
4649

4750
import static org.apache.cassandra.db.Directories.SNAPSHOT_SUBDIR;
4851
import static org.apache.cassandra.service.snapshot.TableSnapshot.buildSnapshotId;
@@ -54,7 +57,9 @@ public class SnapshotLoader
5457
{
5558
private static final Logger logger = LoggerFactory.getLogger(SnapshotLoader.class);
5659

57-
static final Pattern SNAPSHOT_DIR_PATTERN = Pattern.compile("(?<keyspace>\\w+)/(?<tableName>\\w+)-(?<tableId>[0-9a-f]{32})/snapshots/(?<tag>.+)$");
60+
static final Pattern SNAPSHOT_DIR_PATTERN = Pattern.compile("(?<keyspace>\\w+)/(?<tableName>\\w+)" +
61+
"(-(?<tableId>[0-9a-f]{32}))?" +
62+
"/snapshots/(?<tag>.+)$");
5863

5964
private final Collection<Path> dataDirectories;
6065

@@ -149,12 +154,48 @@ private void loadSnapshotFromDir(Matcher snapshotDirMatcher, Path snapshotDir)
149154
{
150155
String keyspaceName = snapshotDirMatcher.group("keyspace");
151156
String tableName = snapshotDirMatcher.group("tableName");
152-
UUID tableId = parseUUID(snapshotDirMatcher.group("tableId"));
157+
final UUID tableId = maybeDetermineTableId(snapshotDirMatcher, snapshotDir, keyspaceName, tableName);
153158
String tag = snapshotDirMatcher.group("tag");
154159
String snapshotId = buildSnapshotId(keyspaceName, tableName, tableId, tag);
155160
TableSnapshot.Builder builder = snapshots.computeIfAbsent(snapshotId, k -> new TableSnapshot.Builder(keyspaceName, tableName, tableId, tag));
156161
builder.addSnapshotDir(new File(snapshotDir));
157162
}
163+
164+
private UUID maybeDetermineTableId(Matcher snapshotDirMatcher, Path snapshotDir, String keyspaceName, String tableName)
165+
{
166+
final UUID tableId;
167+
if (snapshotDirMatcher.group("tableId") == null)
168+
{
169+
logger.debug("Snapshot directory without tableId found (pre-2.1 format): {}", snapshotDir);
170+
// If we don't have a tableId in folder name (e.g pre 2.1 created table)
171+
// Then attempt to get tableId from CFS on startup
172+
// falling back to null is fine as it still yields a unique result in buildSnapshotId for pre-2.1 table
173+
if (Keyspace.isInitialized() && Schema.instance.getKeyspaceMetadata(keyspaceName) != null)
174+
{
175+
ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(keyspaceName, tableName);
176+
tableId = cfs != null && cfs.metadata.id != null
177+
? cfs.metadata.id.asUUID()
178+
: null;
179+
180+
if (tableId == null)
181+
{
182+
logger.warn("Snapshot directory without tableId found (pre-2.1 format), " +
183+
"unable to resolve table id from column family, defaulting to null, snapshot dir: {}", snapshotDir);
184+
}
185+
}
186+
else
187+
{
188+
logger.warn("Snapshot directory without tableId found (pre-2.1 format), " +
189+
"keyspace is not initialized or there is a schema missing, defaulting to null, snapshot dir: {}", snapshotDir);
190+
tableId = null;
191+
}
192+
}
193+
else
194+
{
195+
tableId = parseUUID(snapshotDirMatcher.group("tableId"));
196+
}
197+
return tableId;
198+
}
158199
}
159200

160201
public Set<TableSnapshot> loadSnapshots(String keyspace)

‎src/java/org/apache/cassandra/service/snapshot/TableSnapshot.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.util.UUID;
3030
import java.util.function.Predicate;
3131

32+
import javax.annotation.Nullable;
33+
3234
import org.slf4j.Logger;
3335
import org.slf4j.LoggerFactory;
3436

@@ -43,6 +45,9 @@ public class TableSnapshot
4345

4446
private final String keyspaceName;
4547
private final String tableName;
48+
// tableId may be null under some rare circumstance namely pre-2.1 table
49+
// whose snapshot is loaded upon startup rather than created while the jvm is running
50+
@Nullable
4651
private final UUID tableId;
4752
private final String tag;
4853
private final boolean ephemeral;
@@ -70,7 +75,8 @@ public TableSnapshot(String keyspaceName, String tableName, UUID tableId,
7075
* Unique identifier of a snapshot. Used
7176
* only to deduplicate snapshots internally,
7277
* not exposed externally.
73-
*
78+
* table_id may be empty for tables created prior to 2.1
79+
* <p>
7480
* Format: "$ks:$table_name:$table_id:$tag"
7581
*/
7682
public String getId()
@@ -224,7 +230,8 @@ public String toString()
224230
'}';
225231
}
226232

227-
static class Builder {
233+
static class Builder
234+
{
228235
private final String keyspaceName;
229236
private final String tableName;
230237
private final UUID tableId;
@@ -282,9 +289,9 @@ TableSnapshot build()
282289
}
283290
}
284291

285-
protected static String buildSnapshotId(String keyspaceName, String tableName, UUID tableId, String tag)
292+
protected static String buildSnapshotId(String keyspaceName, String tableName, @Nullable UUID tableId, String tag)
286293
{
287-
return String.format("%s:%s:%s:%s", keyspaceName, tableName, tableId, tag);
294+
return String.format("%s:%s:%s:%s", keyspaceName, tableName, tableId == null ? "" : tableId, tag);
288295
}
289296

290297
public static class SnapshotTrueSizeCalculator extends DirectorySizeCalculator

‎test/unit/org/apache/cassandra/service/snapshot/SnapshotLoaderTest.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@
2828
import java.util.concurrent.ThreadLocalRandom;
2929

3030
import org.junit.Assert;
31+
import org.junit.BeforeClass;
3132
import org.junit.ClassRule;
3233
import org.junit.Test;
3334
import org.junit.rules.TemporaryFolder;
3435

36+
import org.apache.cassandra.config.DatabaseDescriptor;
3537
import org.apache.cassandra.config.DurationSpec;
3638
import org.apache.cassandra.db.Directories;
39+
import org.apache.cassandra.db.Keyspace;
3740
import org.apache.cassandra.io.util.File;
3841
import org.assertj.core.util.Lists;
3942

@@ -67,6 +70,13 @@ public class SnapshotLoaderTest
6770
@ClassRule
6871
public static TemporaryFolder tmpDir = new TemporaryFolder();
6972

73+
@BeforeClass
74+
public static void setup()
75+
{
76+
DatabaseDescriptor.daemonInitialization();
77+
Keyspace.setInitialized();
78+
}
79+
7080
@Test
7181
public void testMatcher()
7282
{
@@ -144,6 +154,7 @@ public void testSnapshotsWithoutManifests() throws IOException
144154
snapshots = loader.loadSnapshots(KEYSPACE_2);
145155
assertThat(snapshots).hasSize(1);
146156
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_2, TABLE3_NAME, TABLE3_ID, TAG3, null, null, tag3Files, false));
157+
147158
}
148159

149160
@Test
@@ -192,7 +203,8 @@ public void testSnapshotsWithManifests() throws IOException
192203
for (String dataDir : DATA_DIRS)
193204
{
194205
tag1Files.add(createDir(baseDir, dataDir, KEYSPACE_1, tableDirName(TABLE1_NAME, TABLE1_ID), Directories.SNAPSHOT_SUBDIR, TAG1));
195-
tag2Files.add(createDir(baseDir, dataDir, KEYSPACE_1, tableDirName(TABLE2_NAME, TABLE2_ID), Directories.SNAPSHOT_SUBDIR, TAG2));
206+
// One table is an old pre 2.1 table folder structure
207+
tag2Files.add(createDir(baseDir, dataDir, KEYSPACE_1, TABLE2_NAME, Directories.SNAPSHOT_SUBDIR, TAG2));
196208
tag3Files.add(createDir(baseDir, dataDir, KEYSPACE_2, tableDirName(TABLE3_NAME, TABLE3_ID), Directories.SNAPSHOT_SUBDIR, TAG3));
197209
}
198210

@@ -219,7 +231,7 @@ public void testSnapshotsWithManifests() throws IOException
219231
Set<TableSnapshot> snapshots = loader.loadSnapshots();
220232
assertThat(snapshots).hasSize(3);
221233
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE1_NAME, TABLE1_ID, TAG1, tag1Ts, null, tag1Files, false));
222-
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, TABLE2_ID, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false));
234+
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, null, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false));
223235
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_2, TABLE3_NAME, TABLE3_ID, TAG3, tag3Ts, null, tag3Files, false));
224236

225237
// Verify snapshot loading for a specific keyspace
@@ -230,7 +242,7 @@ public void testSnapshotsWithManifests() throws IOException
230242
snapshots = loader.loadSnapshots(KEYSPACE_1);
231243
assertThat(snapshots).hasSize(2);
232244
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE1_NAME, TABLE1_ID, TAG1, tag1Ts, null, tag1Files, false));
233-
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, TABLE2_ID, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false));
245+
assertThat(snapshots).contains(new TableSnapshot(KEYSPACE_1, TABLE2_NAME, null, TAG2, tag2Ts, tag2Ts.plusSeconds(tag2Ttl.toSeconds()), tag2Files, false));
234246

235247
loader = new SnapshotLoader(Arrays.asList(Paths.get(baseDir.toString(), DATA_DIR_1),
236248
Paths.get(baseDir.toString(), DATA_DIR_2),

0 commit comments

Comments
 (0)