From 95a622305722889c321204c4bca68a3517a29aab Mon Sep 17 00:00:00 2001 From: Paulo Motta Date: Mon, 14 Mar 2022 17:17:37 -0300 Subject: [PATCH] Fix snapshot true size calculation Patch by Paulo Motta; Reviewed by Brandon Williams and Benjamin Lerer for CASSANDRA-17267 --- CHANGES.txt | 1 + .../org/apache/cassandra/db/Directories.java | 7 +-- .../cassandra/db/ColumnFamilyStoreTest.java | 46 +++++++++++++++++ .../cassandra/index/sasi/SASIIndexTest.java | 51 +++++++++++-------- 4 files changed, 81 insertions(+), 24 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a8954b7a64..df9d4e1e20 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.11.13 + * Fix snapshot true size calculation (CASSANDRA-17267) * Validate existence of DCs when repairing (CASSANDRA-17407) * dropping of a materialized view creates a snapshot with dropped- prefix (CASSANDRA-17415) Merged from 3.0: diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java index b5be69be23..b37afa50fe 100644 --- a/src/java/org/apache/cassandra/db/Directories.java +++ b/src/java/org/apache/cassandra/db/Directories.java @@ -27,6 +27,7 @@ import java.nio.file.Paths; import java.util.*; import java.util.concurrent.ThreadLocalRandom; import java.util.function.BiFunction; +import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; @@ -1053,11 +1054,11 @@ public class Directories private class SSTableSizeSummer extends DirectorySizeCalculator { - private final HashSet toSkip; + private final Set toSkip; SSTableSizeSummer(File path, List files) { super(path); - toSkip = new HashSet<>(files); + toSkip = files.stream().map(f -> f.getName()).collect(Collectors.toSet()); } @Override @@ -1068,7 +1069,7 @@ public class Directories return pair != null && pair.left.ksname.equals(metadata.ksName) && pair.left.cfname.equals(metadata.cfName) - && !toSkip.contains(file); + && !toSkip.contains(file.getName()); } } } diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index a3564bb6a1..3987a29f2d 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -30,10 +30,12 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.json.simple.JSONArray; import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -346,6 +348,50 @@ public class ColumnFamilyStoreTest cfs.clearSnapshot(""); } + @Test + public void testSnapshotSize() + { + // cleanup any previous test gargbage + ColumnFamilyStore cfs = Keyspace.open(KEYSPACE1).getColumnFamilyStore(CF_STANDARD1); + cfs.clearSnapshot(""); + + // Add row + new RowUpdateBuilder(cfs.metadata, 0, "key1") + .clustering("Column1") + .add("val", "asdf") + .build() + .applyUnsafe(); + cfs.forceBlockingFlush(); + + // snapshot + cfs.snapshot("basic", null, false, false); + + // check snapshot was created + Map> snapshotDetails = cfs.getSnapshotDetails(); + assertThat(snapshotDetails).hasSize(1); + assertThat(snapshotDetails).containsKey("basic"); + + // check that sizeOnDisk > trueSize = 0 + Pair details = snapshotDetails.get("basic"); + long sizeOnDisk = details.left; + long trueSize = details.right; + assertThat(sizeOnDisk).isGreaterThan(trueSize); + assertThat(trueSize).isZero(); + + // compact base table to make trueSize > 0 + cfs.forceMajorCompaction(); + LifecycleTransaction.waitForDeletions(); + + // sizeOnDisk > trueSize because trueSize does not include manifest.json + // Check that truesize now is > 0 + snapshotDetails = cfs.getSnapshotDetails(); + details = snapshotDetails.get("basic"); + sizeOnDisk = details.left; + trueSize = details.right; + assertThat(sizeOnDisk).isGreaterThan(trueSize); + assertThat(trueSize).isPositive(); + } + @Test public void testBackupAfterFlush() throws Throwable { diff --git a/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java b/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java index 6ee53f8715..5fbcd8f90f 100644 --- a/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java +++ b/test/unit/org/apache/cassandra/index/sasi/SASIIndexTest.java @@ -40,6 +40,7 @@ import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; import org.apache.cassandra.config.Schema; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.index.Index; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.*; @@ -156,7 +157,6 @@ public class SASIIndexTest data.put(UUID.randomUUID().toString(), Pair.create(UUID.randomUUID().toString(), r.nextInt())); ColumnFamilyStore store = loadData(data, true); - store.forceMajorCompaction(); Set ssTableReaders = store.getLiveSSTables(); Set sasiComponents = new HashSet<>(); @@ -170,6 +170,10 @@ public class SASIIndexTest try { store.snapshot(snapshotName); + // Compact to make true snapshot size != 0 + store.forceMajorCompaction(); + LifecycleTransaction.waitForDeletions(); + FileReader reader = new FileReader(store.getDirectories().getSnapshotManifestFile(snapshotName)); JSONObject manifest = (JSONObject) new JSONParser().parse(reader); JSONArray files = (JSONArray) manifest.get("files"); @@ -202,8 +206,7 @@ public class SASIIndexTest for (Component c : components) { - Path componentPath = Paths.get(sstable.descriptor + "-" + c.name); - long componentSize = Files.size(componentPath); + long componentSize = Files.size(Paths.get(snapshotSSTable.filenameFor(c))); if (Component.Type.fromRepresentation(c.name) == Component.Type.SECONDARY_INDEX) indexSize += componentSize; else @@ -214,7 +217,7 @@ public class SASIIndexTest Map> details = store.getSnapshotDetails(); // check that SASI components are included in the computation of snapshot size - Assert.assertEquals((long) details.get(snapshotName).right, tableSize + indexSize); + Assert.assertEquals(tableSize + indexSize, (long) details.get(snapshotName).right); } finally { @@ -2636,12 +2639,16 @@ public class SASIIndexTest private static Set getIndexed(ColumnFamilyStore store, ColumnFilter columnFilter, int maxResults, Expression... expressions) { - return getKeys(getIndexed(store, columnFilter, null, maxResults, expressions)); + ReadCommand command = getIndexReadCommand(store, columnFilter, null, maxResults, expressions); + try (ReadExecutionController controller = command.executionController(); + UnfilteredPartitionIterator rows = command.executeLocally(controller)) + { + return getKeys(rows); + } } private static Set getPaged(ColumnFamilyStore store, int pageSize, Expression... expressions) { - UnfilteredPartitionIterator currentPage; Set uniqueKeys = new TreeSet<>(); DecoratedKey lastKey = null; @@ -2650,28 +2657,32 @@ public class SASIIndexTest do { count = 0; - currentPage = getIndexed(store, ColumnFilter.all(store.metadata), lastKey, pageSize, expressions); - if (currentPage == null) - break; + ReadCommand command = getIndexReadCommand(store, ColumnFilter.all(store.metadata), lastKey, pageSize, expressions); - while (currentPage.hasNext()) + try (ReadExecutionController controller = command.executionController(); + UnfilteredPartitionIterator currentPage = command.executeLocally(controller)) { - try (UnfilteredRowIterator row = currentPage.next()) + if (currentPage == null) + break; + + while (currentPage.hasNext()) { - uniqueKeys.add(row.partitionKey()); - lastKey = row.partitionKey(); - count++; + try (UnfilteredRowIterator row = currentPage.next()) + { + uniqueKeys.add(row.partitionKey()); + lastKey = row.partitionKey(); + count++; + } } } - currentPage.close(); } while (count == pageSize); return uniqueKeys; } - private static UnfilteredPartitionIterator getIndexed(ColumnFamilyStore store, ColumnFilter columnFilter, DecoratedKey startKey, int maxResults, Expression... expressions) + private static ReadCommand getIndexReadCommand(ColumnFamilyStore store, ColumnFilter columnFilter, DecoratedKey startKey, int maxResults, Expression[] expressions) { DataRange range = (startKey == null) ? DataRange.allData(PARTITIONER) @@ -2682,15 +2693,13 @@ public class SASIIndexTest filter.add(store.metadata.getColumnDefinition(e.name), e.op, e.value); ReadCommand command = - PartitionRangeReadCommand.create(false, - store.metadata, + PartitionRangeReadCommand.create(false, store.metadata, FBUtilities.nowInSeconds(), columnFilter, filter, - DataLimits.thriftLimits(maxResults, DataLimits.NO_LIMIT), + DataLimits.cqlLimits(maxResults), range); - - return command.executeLocally(command.executionController()); + return command; } private static Mutation newMutation(String key, String firstName, String lastName, int age, long timestamp)