diff --git a/CHANGES.txt b/CHANGES.txt index c2b3d693e0..60d1e22e8a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -82,6 +82,8 @@ Merged from 2.2: * Remove generated files from source artifact (CASSANDRA-15849) * Remove duplicated tools binaries from tarballs (CASSANDRA-15768) * Duplicate results with DISTINCT queries in mixed mode (CASSANDRA-15501) +Merged from 2.1: + * Fix writing of snapshot manifest when the table has table-backed secondary indexes (CASSANDRA-10968) 4.0-alpha4 * Add client request size server metrics (CASSANDRA-15704) diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 3be3706882..f7411b7de3 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1720,9 +1720,9 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean public Set snapshotWithoutFlush(String snapshotName, Predicate predicate, boolean ephemeral) { Set snapshottedSSTables = new HashSet<>(); + final JSONArray filesJSONArr = new JSONArray(); for (ColumnFamilyStore cfs : concatWithIndexes()) { - final JSONArray filesJSONArr = new JSONArray(); try (RefViewFragment currentView = cfs.selectAndReference(View.select(SSTableSet.CANONICAL, (x) -> predicate == null || predicate.apply(x)))) { for (SSTableReader ssTable : currentView.sstables) @@ -1735,12 +1735,13 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean logger.trace("Snapshot for {} keyspace data file {} created in {}", keyspace, ssTable.getFilename(), snapshotDirectory); snapshottedSSTables.add(ssTable); } - - writeSnapshotManifest(filesJSONArr, snapshotName); - if (!SchemaConstants.isLocalSystemKeyspace(metadata.keyspace) && !SchemaConstants.isReplicatedSystemKeyspace(metadata.keyspace)) - writeSnapshotSchema(snapshotName); } } + + writeSnapshotManifest(filesJSONArr, snapshotName); + if (!SchemaConstants.isLocalSystemKeyspace(metadata.keyspace) && !SchemaConstants.isReplicatedSystemKeyspace(metadata.keyspace)) + writeSnapshotSchema(snapshotName); + if (ephemeral) createEphemeralSnapshotMarkerFile(snapshotName); return snapshottedSSTables; diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index bd4081fa75..2ddeb645a0 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -540,7 +540,7 @@ public class Directories */ public static File getSnapshotDirectory(File location, String snapshotName) { - if (location.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR)) + if (isSecondaryIndexFolder(location)) { return getOrCreate(location.getParentFile(), SNAPSHOT_SUBDIR, snapshotName, location.getName()); } @@ -580,7 +580,7 @@ public class Directories public static File getBackupsDirectory(File location) { - if (location.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR)) + if (isSecondaryIndexFolder(location)) { return getOrCreate(location.getParentFile(), BACKUPS_SUBDIR, location.getName()); } @@ -876,9 +876,9 @@ public class Directories final List snapshots = new LinkedList<>(); for (final File dir : dataPaths) { - File snapshotDir = dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR) ? - new File(dir.getParent(), SNAPSHOT_SUBDIR) : - new File(dir, SNAPSHOT_SUBDIR); + File snapshotDir = isSecondaryIndexFolder(dir) + ? new File(dir.getParent(), SNAPSHOT_SUBDIR) + : new File(dir, SNAPSHOT_SUBDIR); if (snapshotDir.exists() && snapshotDir.isDirectory()) { final File[] snapshotDirs = snapshotDir.listFiles(); @@ -901,7 +901,7 @@ public class Directories for (File dir : dataPaths) { File snapshotDir; - if (dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR)) + if (isSecondaryIndexFolder(dir)) { snapshotDir = new File(dir.getParentFile(), join(SNAPSHOT_SUBDIR, snapshotName, dir.getName())); } @@ -960,9 +960,9 @@ public class Directories long result = 0L; for (File dir : dataPaths) { - File snapshotDir = dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR) ? - new File(dir.getParent(), SNAPSHOT_SUBDIR) : - new File(dir, SNAPSHOT_SUBDIR); + File snapshotDir = isSecondaryIndexFolder(dir) + ? new File(dir.getParent(), SNAPSHOT_SUBDIR) + : new File(dir, SNAPSHOT_SUBDIR); result += getTrueAllocatedSizeIn(snapshotDir); } return result; @@ -1024,6 +1024,11 @@ public class Directories return result; } + public static boolean isSecondaryIndexFolder(File dir) + { + return dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR); + } + public List getCFDirectories() { List result = new ArrayList<>(); diff --git a/src/java/org/apache/cassandra/io/sstable/Descriptor.java b/src/java/org/apache/cassandra/io/sstable/Descriptor.java index 3e4ba51c69..fca0f8db11 100644 --- a/src/java/org/apache/cassandra/io/sstable/Descriptor.java +++ b/src/java/org/apache/cassandra/io/sstable/Descriptor.java @@ -137,6 +137,11 @@ public class Descriptor public String relativeFilenameFor(Component component) { final StringBuilder buff = new StringBuilder(); + if (Directories.isSecondaryIndexFolder(directory)) + { + buff.append(directory.getName()).append(File.separator); + } + appendFileName(buff); buff.append(separator).append(component.name()); return buff.toString(); @@ -276,7 +281,7 @@ public class Descriptor // Check if it's a 2ndary index directory (not that it doesn't exclude it to be also a backup or snapshot) String indexName = ""; - if (tableDir.getName().startsWith(Directories.SECONDARY_INDEX_NAME_SEPARATOR)) + if (Directories.isSecondaryIndexFolder(tableDir)) { indexName = tableDir.getName(); tableDir = parentOf(name, tableDir); diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index 888cdc6c8e..c3ea0a0eec 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -19,6 +19,7 @@ package org.apache.cassandra.db; import java.io.File; +import java.io.FileReader; import java.io.IOException; import java.nio.ByteBuffer; import java.util.*; @@ -29,6 +30,10 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.json.simple.JSONArray; +import org.json.simple.JSONObject; +import org.json.simple.parser.JSONParser; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -38,7 +43,6 @@ import org.apache.cassandra.cql3.Operator; import org.apache.cassandra.db.lifecycle.SSTableSet; import org.apache.cassandra.db.rows.*; import org.apache.cassandra.db.partitions.*; -import org.apache.cassandra.db.marshal.*; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.io.sstable.Component; import org.apache.cassandra.io.sstable.Descriptor; @@ -51,6 +55,7 @@ import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.WrappedRunnable; import static junit.framework.Assert.assertNotNull; + @RunWith(OrderedJUnit4ClassRunner.class) public class ColumnFamilyStoreTest { @@ -427,6 +432,39 @@ public class ColumnFamilyStoreTest assertEquals(count, found); } + @Test + public void testSnapshotWithoutFlushWithSecondaryIndexes() throws Exception + { + Keyspace keyspace = Keyspace.open(KEYSPACE1); + ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_INDEX1); + cfs.truncateBlocking(); + + UpdateBuilder builder = UpdateBuilder.create(cfs.metadata.get(), "key") + .newRow() + .add("birthdate", 1L) + .add("notbirthdate", 2L); + new Mutation(builder.build()).applyUnsafe(); + cfs.forceBlockingFlush(); + + String snapshotName = "newSnapshot"; + cfs.snapshotWithoutFlush(snapshotName); + + File snapshotManifestFile = cfs.getDirectories().getSnapshotManifestFile(snapshotName); + JSONParser parser = new JSONParser(); + JSONObject manifest = (JSONObject) parser.parse(new FileReader(snapshotManifestFile)); + JSONArray files = (JSONArray) manifest.get("files"); + + // Keyspace1-Indexed1 and the corresponding index + assert files.size() == 2; + + // Snapshot of the secondary index is stored in the subfolder with the same file name + String baseTableFile = (String) files.get(0); + String indexTableFile = (String) files.get(1); + assert !baseTableFile.equals(indexTableFile); + assert Directories.isSecondaryIndexFolder(new File(indexTableFile).getParentFile()); + assert indexTableFile.endsWith(baseTableFile); + } + @Test public void testScrubDataDirectories() throws Throwable { diff --git a/test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java b/test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java index 5671e220d8..5f79f570b7 100644 --- a/test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java @@ -23,8 +23,10 @@ import java.util.UUID; import org.apache.commons.lang3.StringUtils; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.Directories; import org.apache.cassandra.io.sstable.format.SSTableFormat; import org.apache.cassandra.io.util.FileUtils; @@ -46,6 +48,12 @@ public class DescriptorTest tempDataDir = FileUtils.createTempFile("DescriptorTest", null).getParentFile(); } + @BeforeClass + public static void setup() + { + DatabaseDescriptor.daemonInitialization(); + } + @Test public void testFromFilename() throws Exception {