Repository: cassandra Updated Branches: refs/heads/trunk 89d31f3da -> b5b1af703
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/f6d0cf39 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f6d0cf39 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f6d0cf39 Branch: refs/heads/trunk Commit: f6d0cf395fe33bc3d13af75d5a34cbf1a9017276 Parents: d4e3786 Author: Marcus Eriksson <[email protected]> Authored: Tue Mar 3 11:49:08 2015 +0100 Committer: Marcus Eriksson <[email protected]> Committed: Wed Mar 4 08:42:34 2015 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + NEWS.txt | 8 ++ pylib/cqlshlib/cql3handling.py | 1 - .../SizeTieredCompactionStrategy.java | 136 ------------------- .../SizeTieredCompactionStrategyOptions.java | 13 +- .../SizeTieredCompactionStrategyTest.java | 90 ------------ 6 files changed, 10 insertions(+), 239 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f6d0cf39/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/f6d0cf39/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/f6d0cf39/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/f6d0cf39/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/f6d0cf39/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..9a840e1 100644 --- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java +++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java @@ -26,16 +26,15 @@ 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"; + @Deprecated 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 +44,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 +51,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,13 +90,6 @@ 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); http://git-wip-us.apache.org/repos/asf/cassandra/blob/f6d0cf39/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()); - } }
