You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ma...@apache.org on 2015/03/04 09:10:01 UTC

cassandra git commit: Remove cold_reads_to_omit from STCS

Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.1 d4e37869b -> 33a9adac8


Remove cold_reads_to_omit from STCS

Patch by marcuse; reviewed by thobbs for CASSANDRA-8860


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/33a9adac
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/33a9adac
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/33a9adac

Branch: refs/heads/cassandra-2.1
Commit: 33a9adac8e11cc4b01aa305868412b74048d3b34
Parents: d4e3786
Author: Marcus Eriksson <ma...@apache.org>
Authored: Tue Mar 3 11:49:08 2015 +0100
Committer: Marcus Eriksson <ma...@apache.org>
Committed: Wed Mar 4 09:09:18 2015 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 NEWS.txt                                        |   8 ++
 pylib/cqlshlib/cql3handling.py                  |   1 -
 .../SizeTieredCompactionStrategy.java           | 136 -------------------
 .../SizeTieredCompactionStrategyOptions.java    |  14 --
 .../SizeTieredCompactionStrategyTest.java       |  90 ------------
 6 files changed, 9 insertions(+), 241 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 748acf8..4992d85 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.1.4
+ * Remove cold_reads_to_omit from STCS (CASSANDRA-8860)
  * Make EstimatedHistogram#percentile() use ceil instead of floor (CASSANDRA-8883)
  * Fix top partitions reporting wrong cardinality (CASSANDRA-8834)
  * Fix rare NPE in KeyCacheSerializer (CASSANDRA-8067)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/NEWS.txt
----------------------------------------------------------------------
diff --git a/NEWS.txt b/NEWS.txt
index 602770c..06013b8 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -13,6 +13,14 @@ restore snapshots created with the previous major version using the
 'sstableloader' tool. You can upgrade the file format of your snapshots
 using the provided 'sstableupgrade' tool.
 
+2.1.4
+=====
+Upgrading
+---------
+    - The option to omit cold sstables with size tiered compaction has been
+      removed - it is almost always better to use date tiered compaction for
+      workloads that have cold data. 
+
 2.1.3
 =====
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/pylib/cqlshlib/cql3handling.py
----------------------------------------------------------------------
diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py
index f089cd7..88f042e 100644
--- a/pylib/cqlshlib/cql3handling.py
+++ b/pylib/cqlshlib/cql3handling.py
@@ -468,7 +468,6 @@ def cf_prop_val_mapkey_completer(ctxt, cass):
             opts.add('min_threshold')
             opts.add('bucket_high')
             opts.add('bucket_low')
-            opts.add('cold_reads_to_omit')
         elif csc == 'LeveledCompactionStrategy':
             opts.add('sstable_size_in_mb')
         elif csc == 'DateTieredCompactionStrategy':

http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
index 08102c1..19abd9c 100644
--- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
+++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
@@ -82,7 +82,6 @@ public class SizeTieredCompactionStrategy extends AbstractCompactionStrategy
         int maxThreshold = cfs.getMaximumCompactionThreshold();
 
         Iterable<SSTableReader> candidates = filterSuspectSSTables(Sets.intersection(cfs.getUncompactingSSTables(), sstables));
-        candidates = filterColdSSTables(Lists.newArrayList(candidates), options.coldReadsToOmit, cfs.getMinimumCompactionThreshold());
 
         List<List<SSTableReader>> buckets = getBuckets(createSSTableAndLengthPairs(candidates), options.bucketHigh, options.bucketLow, options.minSSTableSize);
         logger.debug("Compaction buckets are {}", buckets);
@@ -106,141 +105,6 @@ public class SizeTieredCompactionStrategy extends AbstractCompactionStrategy
         return Collections.singletonList(sstablesWithTombstones.get(0));
     }
 
-    /**
-     * Removes as many cold sstables as possible while retaining at least 1-coldReadsToOmit of the total reads/sec
-     * across all sstables
-     * @param sstables all sstables to consider
-     * @param coldReadsToOmit the proportion of total reads/sec that will be omitted (0=omit nothing, 1=omit everything)
-     * @param minThreshold min compaction threshold
-     * @return a list of sstables with the coldest sstables excluded until the reads they represent reaches coldReadsToOmit
-     */
-    @VisibleForTesting
-    static List<SSTableReader> filterColdSSTables(List<SSTableReader> sstables, double coldReadsToOmit, int minThreshold)
-    {
-        if (coldReadsToOmit == 0.0)
-            return sstables;
-
-        // Sort the sstables by hotness (coldest-first). We first build a map because the hotness may change during the sort.
-        final Map<SSTableReader, Double> hotnessSnapshot = getHotnessMap(sstables);
-        Collections.sort(sstables, new Comparator<SSTableReader>()
-        {
-            public int compare(SSTableReader o1, SSTableReader o2)
-            {
-                int comparison = Double.compare(hotnessSnapshot.get(o1), hotnessSnapshot.get(o2));
-                if (comparison != 0)
-                    return comparison;
-
-                // break ties with size on disk (mainly for system tables and cold tables)
-                comparison = Long.compare(o1.bytesOnDisk(), o2.bytesOnDisk());
-                if (comparison != 0)
-                    return comparison;
-
-                // if there's still a tie, use generation, which is guaranteed to be unique.  this ensures that
-                // our filtering is deterministic, which can be useful when debugging.
-                return o1.descriptor.generation - o2.descriptor.generation;
-            }
-        });
-
-        // calculate the total reads/sec across all sstables
-        double totalReads = 0.0;
-        for (SSTableReader sstr : sstables)
-            if (sstr.getReadMeter() != null)
-                totalReads += sstr.getReadMeter().twoHourRate();
-
-        // if this is a system table with no read meters or we don't have any read rates yet, just return them all
-        if (totalReads == 0.0)
-            return sstables;
-
-        // iteratively ignore the coldest sstables until ignoring one more would put us over the coldReadsToOmit threshold
-        double maxColdReads = coldReadsToOmit * totalReads;
-
-        double totalColdReads = 0.0;
-        int cutoffIndex = 0;
-        while (cutoffIndex < sstables.size())
-        {
-            SSTableReader sstable = sstables.get(cutoffIndex);
-            if (sstable.getReadMeter() == null)
-            {
-                throw new AssertionError("If you're seeing this exception, please attach your logs to CASSANDRA-8238 to help us debug. "+sstable);
-            }
-            double reads = sstable.getReadMeter().twoHourRate();
-            if (totalColdReads + reads > maxColdReads)
-                break;
-
-            totalColdReads += reads;
-            cutoffIndex++;
-        }
-        List<SSTableReader> hotSSTables = new ArrayList<>(sstables.subList(cutoffIndex, sstables.size()));
-        List<SSTableReader> coldSSTables = sstables.subList(0, cutoffIndex);
-        logger.debug("hotSSTables={}, coldSSTables={}", hotSSTables.size(), coldSSTables.size());
-        if (hotSSTables.size() >= minThreshold)
-            return hotSSTables;
-        if (coldSSTables.size() < minThreshold)
-            return Collections.emptyList();
-
-        Map<SSTableReader, Set<SSTableReader>> overlapMap = new HashMap<>();
-        for (int i = 0; i < coldSSTables.size(); i++)
-        {
-            SSTableReader sstable = coldSSTables.get(i);
-            Set<SSTableReader> overlaps = new HashSet<>();
-            for (int j = 0; j < coldSSTables.size(); j++)
-            {
-                SSTableReader innerSSTable = coldSSTables.get(j);
-                if (ColumnNameHelper.overlaps(sstable.getSSTableMetadata().minColumnNames,
-                                              sstable.getSSTableMetadata().maxColumnNames,
-                                              innerSSTable.getSSTableMetadata().minColumnNames,
-                                              innerSSTable.getSSTableMetadata().maxColumnNames,
-                                              sstable.metadata.comparator))
-                {
-                    overlaps.add(innerSSTable);
-                }
-            }
-            overlapMap.put(sstable, overlaps);
-        }
-        List<Set<SSTableReader>> overlapChains = new ArrayList<>();
-        for (SSTableReader sstable : overlapMap.keySet())
-            overlapChains.add(createOverlapChain(sstable, overlapMap));
-
-        Collections.sort(overlapChains, new Comparator<Set<SSTableReader>>()
-        {
-            @Override
-            public int compare(Set<SSTableReader> o1, Set<SSTableReader> o2)
-            {
-                return Longs.compare(SSTableReader.getTotalBytes(o2), SSTableReader.getTotalBytes(o1));
-            }
-        });
-        for (Set<SSTableReader> overlapping : overlapChains)
-        {
-            // if we are expecting to only keep 70% of the keys after a compaction, run a compaction on these cold sstables:
-            if (SSTableReader.estimateCompactionGain(overlapping) < 0.7)
-                return new ArrayList<>(overlapping);
-        }
-        return Collections.emptyList();
-    }
-
-    /**
-     * returns a set with all overlapping sstables starting with s.
-     * if we have 3 sstables, a, b, c where a overlaps with b, but not c and b overlaps with c, all sstables would be returned.
-     *
-     * m contains an sstable -> all overlapping mapping
-     */
-    private static Set<SSTableReader> createOverlapChain(SSTableReader s, Map<SSTableReader, Set<SSTableReader>> m)
-    {
-        Deque<SSTableReader> sstables = new ArrayDeque<>();
-        Set<SSTableReader> overlapChain = new HashSet<>();
-        sstables.push(s);
-        while (!sstables.isEmpty())
-        {
-            SSTableReader sstable = sstables.pop();
-            if (overlapChain.add(sstable))
-            {
-                if (m.containsKey(sstable))
-                    sstables.addAll(m.get(sstable));
-            }
-        }
-        return overlapChain;
-    }
-
 
     /**
      * @param buckets list of buckets from which to return the most interesting, where "interesting" is the total hotness for reads

http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java
index 84e7d61..911bb9f 100644
--- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java
+++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java
@@ -26,16 +26,13 @@ public final class SizeTieredCompactionStrategyOptions
     protected static final long DEFAULT_MIN_SSTABLE_SIZE = 50L * 1024L * 1024L;
     protected static final double DEFAULT_BUCKET_LOW = 0.5;
     protected static final double DEFAULT_BUCKET_HIGH = 1.5;
-    protected static final double DEFAULT_COLD_READS_TO_OMIT = 0.05;
     protected static final String MIN_SSTABLE_SIZE_KEY = "min_sstable_size";
     protected static final String BUCKET_LOW_KEY = "bucket_low";
     protected static final String BUCKET_HIGH_KEY = "bucket_high";
-    protected static final String COLD_READS_TO_OMIT_KEY = "cold_reads_to_omit";
 
     protected long minSSTableSize;
     protected double bucketLow;
     protected double bucketHigh;
-    protected double coldReadsToOmit;
 
     public SizeTieredCompactionStrategyOptions(Map<String, String> options)
     {
@@ -45,8 +42,6 @@ public final class SizeTieredCompactionStrategyOptions
         bucketLow = optionValue == null ? DEFAULT_BUCKET_LOW : Double.parseDouble(optionValue);
         optionValue = options.get(BUCKET_HIGH_KEY);
         bucketHigh = optionValue == null ? DEFAULT_BUCKET_HIGH : Double.parseDouble(optionValue);
-        optionValue = options.get(COLD_READS_TO_OMIT_KEY);
-        coldReadsToOmit = optionValue == null ? DEFAULT_COLD_READS_TO_OMIT : Double.parseDouble(optionValue);
     }
 
     public SizeTieredCompactionStrategyOptions()
@@ -54,7 +49,6 @@ public final class SizeTieredCompactionStrategyOptions
         minSSTableSize = DEFAULT_MIN_SSTABLE_SIZE;
         bucketLow = DEFAULT_BUCKET_LOW;
         bucketHigh = DEFAULT_BUCKET_HIGH;
-        coldReadsToOmit = DEFAULT_COLD_READS_TO_OMIT;
     }
 
     private static double parseDouble(Map<String, String> options, String key, double defaultValue) throws ConfigurationException
@@ -94,17 +88,9 @@ public final class SizeTieredCompactionStrategyOptions
                                                            BUCKET_HIGH_KEY, bucketHigh, BUCKET_LOW_KEY, bucketLow));
         }
 
-        double maxColdReadsRatio = parseDouble(options, COLD_READS_TO_OMIT_KEY, DEFAULT_COLD_READS_TO_OMIT);
-        if (maxColdReadsRatio < 0.0 || maxColdReadsRatio > 1.0)
-        {
-            throw new ConfigurationException(String.format("%s value (%s) should be between between 0.0 and 1.0",
-                                                           COLD_READS_TO_OMIT_KEY, optionValue));
-        }
-
         uncheckedOptions.remove(MIN_SSTABLE_SIZE_KEY);
         uncheckedOptions.remove(BUCKET_LOW_KEY);
         uncheckedOptions.remove(BUCKET_HIGH_KEY);
-        uncheckedOptions.remove(COLD_READS_TO_OMIT_KEY);
 
         return uncheckedOptions;
     }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/33a9adac/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java b/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java
index 87b284e..1591f03 100644
--- a/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java
+++ b/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java
@@ -33,7 +33,6 @@ import org.apache.cassandra.utils.Pair;
 import static org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy.getBuckets;
 import static org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy.mostInterestingBucket;
 import static org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy.trimToThresholdWithHotness;
-import static org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy.filterColdSSTables;
 import static org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy.validateOptions;
 
 import static org.junit.Assert.*;
@@ -45,7 +44,6 @@ public class SizeTieredCompactionStrategyTest extends SchemaLoader
     public void testOptionsValidation() throws ConfigurationException
     {
         Map<String, String> options = new HashMap<>();
-        options.put(SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY, "0.35");
         options.put(SizeTieredCompactionStrategyOptions.BUCKET_LOW_KEY, "0.5");
         options.put(SizeTieredCompactionStrategyOptions.BUCKET_HIGH_KEY, "1.5");
         options.put(SizeTieredCompactionStrategyOptions.MIN_SSTABLE_SIZE_KEY, "10000");
@@ -54,25 +52,6 @@ public class SizeTieredCompactionStrategyTest extends SchemaLoader
 
         try
         {
-            options.put(SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY, "-0.5");
-            validateOptions(options);
-            fail(String.format("Negative %s should be rejected", SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY));
-        }
-        catch (ConfigurationException e) {}
-
-        try
-        {
-            options.put(SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY, "10.0");
-            validateOptions(options);
-            fail(String.format("%s > 1.0 should be rejected", SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY));
-        }
-        catch (ConfigurationException e)
-        {
-            options.put(SizeTieredCompactionStrategyOptions.COLD_READS_TO_OMIT_KEY, "0.25");
-        }
-
-        try
-        {
             options.put(SizeTieredCompactionStrategyOptions.BUCKET_LOW_KEY, "1000.0");
             validateOptions(options);
             fail("bucket_low greater than bucket_high should be rejected");
@@ -186,73 +165,4 @@ public class SizeTieredCompactionStrategyTest extends SchemaLoader
         assertEquals(String.format("bucket hotness (%f) should be close to %f", bucket.right, expectedBucketHotness),
                      expectedBucketHotness, bucket.right, 1.0);
     }
-
-    @Test
-    public void testFilterColdSSTables() throws Exception
-    {
-        String ksname = "Keyspace1";
-        String cfname = "Standard1";
-        Keyspace keyspace = Keyspace.open(ksname);
-        ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(cfname);
-        cfs.truncateBlocking();
-        cfs.disableAutoCompaction();
-
-        ByteBuffer value = ByteBuffer.wrap(new byte[100]);
-
-        // create 10 sstables
-        int numSSTables = 10;
-        for (int r = 0; r < numSSTables; r++)
-        {
-            DecoratedKey key = Util.dk(String.valueOf(r));
-            Mutation rm = new Mutation(ksname, key.getKey());
-            rm.add(cfname, Util.cellname("column"), value, 0);
-            rm.apply();
-            cfs.forceBlockingFlush();
-        }
-        cfs.forceBlockingFlush();
-
-        List<SSTableReader> filtered;
-        List<SSTableReader> sstrs = new ArrayList<>(cfs.getSSTables());
-
-        for (SSTableReader sstr : sstrs)
-            sstr.overrideReadMeter(null);
-        filtered = filterColdSSTables(sstrs, 0.05, 0);
-        assertEquals("when there are no read meters, no sstables should be filtered", sstrs.size(), filtered.size());
-
-        for (SSTableReader sstr : sstrs)
-            sstr.overrideReadMeter(new RestorableMeter(0.0, 0.0));
-        filtered = filterColdSSTables(sstrs, 0.05, 0);
-        assertEquals("when all read meters are zero, no sstables should be filtered", sstrs.size(), filtered.size());
-
-        // leave all read rates at 0 besides one
-        sstrs.get(0).overrideReadMeter(new RestorableMeter(1000.0, 1000.0));
-        filtered = filterColdSSTables(sstrs, 0.05, 0);
-        assertEquals("there should only be one hot sstable", 1, filtered.size());
-        assertEquals(1000.0, filtered.get(0).getReadMeter().twoHourRate(), 0.5);
-
-        // the total read rate is 100, and we'll set a threshold of 2.5%, so two of the sstables with read
-        // rate 1.0 should be ignored, but not the third
-        for (SSTableReader sstr : sstrs)
-            sstr.overrideReadMeter(new RestorableMeter(0.0, 0.0));
-        sstrs.get(0).overrideReadMeter(new RestorableMeter(97.0, 97.0));
-        sstrs.get(1).overrideReadMeter(new RestorableMeter(1.0, 1.0));
-        sstrs.get(2).overrideReadMeter(new RestorableMeter(1.0, 1.0));
-        sstrs.get(3).overrideReadMeter(new RestorableMeter(1.0, 1.0));
-
-        filtered = filterColdSSTables(sstrs, 0.025, 0);
-        assertEquals(2, filtered.size());
-        assertEquals(98.0, filtered.get(0).getReadMeter().twoHourRate() + filtered.get(1).getReadMeter().twoHourRate(), 0.5);
-
-        // make sure a threshold of 0.0 doesn't result in any sstables being filtered
-        for (SSTableReader sstr : sstrs)
-            sstr.overrideReadMeter(new RestorableMeter(1.0, 1.0));
-        filtered = filterColdSSTables(sstrs, 0.0, 0);
-        assertEquals(sstrs.size(), filtered.size());
-
-        // just for fun, set a threshold where all sstables are considered cold
-        for (SSTableReader sstr : sstrs)
-            sstr.overrideReadMeter(new RestorableMeter(1.0, 1.0));
-        filtered = filterColdSSTables(sstrs, 1.0, 0);
-        assertTrue(filtered.isEmpty());
-    }
 }