Merge branch 'cassandra-3.0' into cassandra-3.11

CHANGES.txt

@@ -1,4 +1,5 @@
 3.11.3
+ * Don't regenerate bloomfilter and summaries on startup (CASSANDRA-11163)
  * RateBasedBackPressure unnecessarily invokes a lock on the Guava RateLimiter (CASSANDRA-14163)
  * Fix wildcard GROUP BY queries (CASSANDRA-14209)
 Merged from 3.0:

NEWS.txt

@@ -49,6 +49,8 @@ Upgrading
 ---------
     - Materialized view users upgrading from 3.0.15 (3.0.X series) or 3.11.1 (3.11.X series) and  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.11.2
 ======

src/java/org/apache/cassandra/db/ColumnFamilyStore.java

@@ -1900,8 +1900,8 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
                 {
                     if (logger.isTraceEnabled())
                         logger.trace("using snapshot sstable {}", entries.getKey());
-                    // open without tracking hotness
+                    // open offline so we don't modify components or track hotness.
-                    sstable = SSTableReader.open(entries.getKey(), entries.getValue(), metadata, true, false);
+                    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();

src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java

@@ -382,19 +382,19 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
 
     public static SSTableReader open(Descriptor descriptor, Set<Component> 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<Component> 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);
     }
 
     /**
@@ -460,11 +460,22 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
         }
     }
 
+    /**
+     * Open an SSTable for reading
+     * @param descriptor SSTable to open
+     * @param components Components included with this SSTable
+     * @param metadata for this SSTables CF
+     * @param validate Check SSTable for corruption (limited)
+     * @param isOffline Whether we are opening this SSTable "offline", for example from an external tool or not for inclusion in queries (validations)
+     *                  This stops regenerating BF + Summaries and also disables tracking of hotness for the SSTable.
+     * @return {@link SSTableReader}
+     * @throws IOException
+     */
     public static SSTableReader open(Descriptor descriptor,
-                                      Set<Component> components,
+                                     Set<Component> components,
-                                      CFMetaData metadata,
+                                     CFMetaData metadata,
-                                      boolean validate,
+                                     boolean validate,
-                                      boolean trackHotness) throws IOException
+                                     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;
@@ -514,10 +525,10 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
         {
             // load index and filter
             long start = System.nanoTime();
-            sstable.load(validationMetadata);
+            sstable.load(validationMetadata, isOffline);
             logger.trace("INDEX LOAD TIME for {}: {} ms.", descriptor, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
 
-            sstable.setup(trackHotness);
+            sstable.setup(!isOffline); // Don't track hotness if we're offline.
             if (validate)
                 sstable.validate();
 
@@ -707,34 +718,37 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
         return keyCache != null;
     }
 
-    private void load(ValidationMetadata validation) throws IOException
+    /**
+     * See {@link #load(boolean, boolean)}
+     * @param validation Metadata for SSTable being loaded
+     * @param isOffline Whether the SSTable is being loaded by an offline tool (sstabledump, scrub, etc)
+     * @throws IOException
+     */
+    private void load(ValidationMetadata validation, boolean isOffline) throws IOException
     {
         if (metadata.params.bloomFilterFpChance == 1.0)
         {
             // bf is disabled.
-            load(false, true);
+            load(false, !isOffline);
             bf = FilterFactory.AlwaysPresent;
         }
-        else if (!components.contains(Component.PRIMARY_INDEX))
+        else if (!components.contains(Component.PRIMARY_INDEX)) // What happens if filter component and primary index is missing?
         {
             // avoid any reading of the missing primary index component.
             // this should only happen during StandaloneScrubber
-            load(false, false);
+            load(false, !isOffline);
         }
         else if (!components.contains(Component.FILTER) || validation == null)
         {
             // bf is enabled, but filter component is missing.
-            load(true, true);
+            load(!isOffline, !isOffline);
-        }
+            if (isOffline)
-        else if (validation.bloomFilterFPChance != metadata.params.bloomFilterFpChance)
+                bf = FilterFactory.AlwaysPresent;
-        {
-            // bf fp chance in sstable metadata and it has changed since compaction.
-            load(true, true);
         }
         else
         {
             // bf is enabled and fp chance matches the currently configured value.
-            load(false, true);
+            load(false, !isOffline);
             loadBloomFilter(descriptor.version.hasOldBfHashOrder());
         }
     }
@@ -753,7 +767,8 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
     }
 
     /**
-     * Loads ifile, dfile and indexSummary, and optionally recreates the bloom filter.
+     * Loads ifile, dfile and indexSummary, and optionally recreates and persists the bloom filter.
+     * @param recreateBloomFilter Recreate the bloomfilter.
      * @param saveSummaryIfCreated for bulk loading purposes, if the summary was absent and needed to be built, you can
      *                             avoid persisting it to disk by setting this to false
      */
@@ -767,12 +782,9 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
                                                      .withChunkCache(ChunkCache.instance))
         {
             boolean summaryLoaded = loadSummary();
-            boolean builtSummary = false;
+            boolean buildSummary = !summaryLoaded || recreateBloomFilter;
-            if (recreateBloomFilter || !summaryLoaded)
+            if (buildSummary)
-            {
                 buildSummary(recreateBloomFilter, summaryLoaded, Downsampling.BASE_SAMPLING_LEVEL);
-                builtSummary = true;
-            }
 
             int dataBufferSize = optimizationStrategy.bufferSize(sstableMetadata.estimatedPartitionSize.percentile(DatabaseDescriptor.getDiskOptimizationEstimatePercentile()));
 
@@ -785,8 +797,13 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
 
             dfile = dbuilder.bufferSize(dataBufferSize).complete();
 
-            if (saveSummaryIfCreated && builtSummary)
+            if (buildSummary)
-                saveSummary();
+            {
+                if (saveSummaryIfCreated)
+                    saveSummary();
+                if (recreateBloomFilter)
+                    saveBloomFilter();
+            }
         }
         catch (Throwable t)
         { // Because the tidier has not been set-up yet in SSTableReader.open(), we must release the files in case of error
@@ -951,6 +968,30 @@ public abstract class SSTableReader extends SSTable implements SelfRefCounted<SS
         }
     }
 
+    public void saveBloomFilter()
+    {
+        saveBloomFilter(this.descriptor, bf);
+    }
+
+    public static void saveBloomFilter(Descriptor descriptor, IFilter filter)
+    {
+        File filterFile = new File(descriptor.filenameFor(Component.FILTER));
+        try (DataOutputStreamPlus stream = new BufferedDataOutputStreamPlus(new FileOutputStream(filterFile)))
+        {
+            FilterFactory.serialize(filter, stream);
+            stream.flush();
+        }
+        catch (IOException e)
+        {
+            logger.trace("Cannot save SSTable bloomfilter: ", e);
+
+            // corrupted hence delete it and let it load it now.
+            if (filterFile.exists())
+                FileUtils.deleteWithConfirm(filterFile);
+        }
+
+    }
+
     public void setReplaced()
     {
         synchronized (tidy.global)

test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java

@@ -17,8 +17,11 @@
  */
 package org.apache.cassandra.io.sstable;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.*;
 import java.util.concurrent.*;
 
@@ -50,7 +53,9 @@ import org.apache.cassandra.schema.CachingParams;
 import org.apache.cassandra.schema.KeyspaceParams;
 import org.apache.cassandra.service.CacheService;
 import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.FilterFactory;
 import org.apache.cassandra.utils.Pair;
+import static org.apache.cassandra.cql3.QueryProcessor.executeInternal;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -351,10 +356,10 @@ public class SSTableReaderTest
 
 
             new RowUpdateBuilder(store.metadata, timestamp, key.getKey())
-                .clustering("col")
+            .clustering("col")
-                .add("val", ByteBufferUtil.EMPTY_BYTE_BUFFER)
+            .add("val", ByteBufferUtil.EMPTY_BYTE_BUFFER)
-                .build()
+            .build()
-                .applyUnsafe();
+            .applyUnsafe();
 
         }
         store.forceBlockingFlush();
@@ -368,6 +373,84 @@ public class SSTableReaderTest
         Assert.assertArrayEquals(ByteBufferUtil.getArray(firstKey.getKey()), target.getIndexSummaryKey(0));
         assert target.first.equals(firstKey);
         assert target.last.equals(lastKey);
+
+        executeInternal(String.format("ALTER TABLE \"%s\".\"%s\" WITH bloom_filter_fp_chance = 0.3", ks, cf));
+
+        File summaryFile = new File(desc.filenameFor(Component.SUMMARY));
+        Path bloomPath = new File(desc.filenameFor(Component.FILTER)).toPath();
+        Path summaryPath = summaryFile.toPath();
+
+        long bloomModified = Files.getLastModifiedTime(bloomPath).toMillis();
+        long summaryModified = Files.getLastModifiedTime(summaryPath).toMillis();
+
+        Thread.sleep(TimeUnit.MILLISECONDS.toMillis(10)); // sleep to ensure modified time will be different
+
+        // Offline tests
+        // check that bloomfilter/summary ARE NOT regenerated
+        target = SSTableReader.openNoValidation(desc, store.metadata);
+
+        assertEquals(bloomModified, Files.getLastModifiedTime(bloomPath).toMillis());
+        assertEquals(summaryModified, Files.getLastModifiedTime(summaryPath).toMillis());
+
+        target.selfRef().release();
+
+        // check that bloomfilter/summary ARE NOT regenerated and BF=AlwaysPresent when filter component is missing
+        Set<Component> 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();
     }