From 58e6c55e17d6b75328b065e093d51c08b8844004 Mon Sep 17 00:00:00 2001 From: kurt Date: Wed, 31 Jan 2018 04:50:24 +0000 Subject: [PATCH] Don't regenerate bloomfilter and summaries on startup Patch by Kurt Greaves; Reviewed by Chris Lohfink for CASSANDRA-11163, CASSANDRA-14166 --- CHANGES.txt | 1 + NEWS.txt | 2 + .../cassandra/db/ColumnFamilyStore.java | 4 +- .../io/sstable/format/SSTableReader.java | 97 +++++++++++++------ .../io/sstable/SSTableReaderTest.java | 92 +++++++++++++++++- 5 files changed, 161 insertions(+), 35 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 9c6a853d11..1d1a07a0f7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.17 + * Don't regenerate bloomfilter and summaries on startup (CASSANDRA-11163) * Fix NPE when performing comparison against a null frozen in LWT (CASSANDRA-14087) * Log when SSTables are deleted (CASSANDRA-14302) * Fix batch commitlog sync regression (CASSANDRA-14292) diff --git a/NEWS.txt b/NEWS.txt index 64de28a4a1..c06030e98a 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -50,6 +50,8 @@ Upgrading - Materialized view users upgrading from 3.0.15 or later that have performed range movements (join, decommission, move, etc), should run repair on the base tables, and subsequently on the views to ensure data affected by CASSANDRA-14251 is correctly propagated to all replicas. + - Changes to bloom_filter_fp_chance will no longer take effect on existing sstables when the node is restarted. Only + compactions/upgradesstables regenerates bloom filters and Summaries sstable components. See CASSANDRA-11163 3.0.16 ===== diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index b5946bbc1b..14e06b035c 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1752,8 +1752,8 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean { if (logger.isTraceEnabled()) logger.trace("using snapshot sstable {}", entries.getKey()); - // open without tracking hotness - sstable = SSTableReader.open(entries.getKey(), entries.getValue(), metadata, true, false); + // open offline so we don't modify components or track hotness. + sstable = SSTableReader.open(entries.getKey(), entries.getValue(), metadata, true, true); refs.tryRef(sstable); // release the self ref as we never add the snapshot sstable to DataTracker where it is otherwise released sstable.selfRef().release(); diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java index 2c94e45246..c66fd8c408 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java @@ -362,19 +362,19 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted components, CFMetaData metadata) throws IOException { - return open(descriptor, components, metadata, true, true); + return open(descriptor, components, metadata, true, false); } // use only for offline or "Standalone" operations public static SSTableReader openNoValidation(Descriptor descriptor, Set components, ColumnFamilyStore cfs) throws IOException { - return open(descriptor, components, cfs.metadata, false, false); // do not track hotness + return open(descriptor, components, cfs.metadata, false, true); } // use only for offline or "Standalone" operations public static SSTableReader openNoValidation(Descriptor descriptor, CFMetaData metadata) throws IOException { - return open(descriptor, componentsFor(descriptor), metadata, false, false); // do not track hotness + return open(descriptor, componentsFor(descriptor), metadata, false, true); } /** @@ -435,11 +435,22 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted components, - CFMetaData metadata, - boolean validate, - boolean trackHotness) throws IOException + Set components, + CFMetaData metadata, + boolean validate, + boolean isOffline) throws IOException { // Minimum components without which we can't do anything assert components.contains(Component.DATA) : "Data component is missing for sstable " + descriptor; @@ -488,10 +499,10 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted components = SSTable.discoverComponentsFor(desc); + components.remove(Component.FILTER); + target = SSTableReader.openNoValidation(desc, components, store); + + assertEquals(bloomModified, Files.getLastModifiedTime(bloomPath).toMillis()); + assertEquals(summaryModified, Files.getLastModifiedTime(summaryPath).toMillis()); + assertEquals(FilterFactory.AlwaysPresent, target.getBloomFilter()); + + target.selfRef().release(); + + // #### online tests #### + // check that summary & bloomfilter are not regenerated when SSTable is opened and BFFP has been changed + target = SSTableReader.open(desc, store.metadata); + + assertEquals(bloomModified, Files.getLastModifiedTime(bloomPath).toMillis()); + assertEquals(summaryModified, Files.getLastModifiedTime(summaryPath).toMillis()); + + target.selfRef().release(); + + // check that bloomfilter is recreated when it doesn't exist and this causes the summary to be recreated + components = SSTable.discoverComponentsFor(desc); + components.remove(Component.FILTER); + + target = SSTableReader.open(desc, components, store.metadata); + + assertTrue("Bloomfilter was not recreated", bloomModified < Files.getLastModifiedTime(bloomPath).toMillis()); + assertTrue("Summary was not recreated", summaryModified < Files.getLastModifiedTime(summaryPath).toMillis()); + + target.selfRef().release(); + + // check that only the summary is regenerated when it is deleted + components.add(Component.FILTER); + summaryModified = Files.getLastModifiedTime(summaryPath).toMillis(); + summaryFile.delete(); + + Thread.sleep(TimeUnit.MILLISECONDS.toMillis(10)); // sleep to ensure modified time will be different + bloomModified = Files.getLastModifiedTime(bloomPath).toMillis(); + + target = SSTableReader.open(desc, components, store.metadata); + + assertEquals(bloomModified, Files.getLastModifiedTime(bloomPath).toMillis()); + assertTrue("Summary was not recreated", summaryModified < Files.getLastModifiedTime(summaryPath).toMillis()); + + target.selfRef().release(); + + // check that summary and bloomfilter is not recreated when the INDEX is missing + components.add(Component.SUMMARY); + components.remove(Component.PRIMARY_INDEX); + + summaryModified = Files.getLastModifiedTime(summaryPath).toMillis(); + target = SSTableReader.open(desc, components, store.metadata, false, false); + + Thread.sleep(TimeUnit.MILLISECONDS.toMillis(10)); // sleep to ensure modified time will be different + assertEquals(bloomModified, Files.getLastModifiedTime(bloomPath).toMillis()); + assertEquals(summaryModified, Files.getLastModifiedTime(summaryPath).toMillis()); + target.selfRef().release(); }