diff --git a/CHANGES.txt b/CHANGES.txt index 19cd8ab3c163..9a3263a78293 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.0.10 + * Accept data storage units in the min_sstable_size compaction option of STCS, LCS and TWCS (CASSANDRA-21617) * Force repair should ignore min_repair_interval (CASSANDRA-21552) * Render SubnetGroups as JSON in system_views.settings (CASSANDRA-21579) * Avoid rebuilding per-SSTable SAI components unless missing or corrupted (CASSANDRA-21515) diff --git a/doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc b/doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc index 4de16fbb713e..6e9a2941b641 100644 --- a/doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc +++ b/doc/modules/cassandra/pages/developing/cql/cql_singlefile.adoc @@ -668,12 +668,13 @@ enables more aggressive tombstone compactions - single sstable tombstone compactions will run without checking how likely it is that they will be successful. -|`min_sstable_size` |SizeTieredCompactionStrategy |50MB |The size tiered +|`min_sstable_size` |SizeTieredCompactionStrategy |`52428800` (50MiB) |The size tiered strategy groups SSTables to compact in buckets. A bucket groups SSTables that differs from less than 50% in size. However, for small sizes, this would result in a bucketing that is too fine grained. `min_sstable_size` -defines a size threshold (in bytes) below which all SSTables belong to -one unique bucket +defines a size threshold below which all SSTables belong to one unique +bucket. Accepts either a plain byte count (`52428800`) or a size with a +unit suffix (`50MiB`) |`min_threshold` |SizeTieredCompactionStrategy |4 |Minimum number of SSTables needed to start a minor compaction. diff --git a/doc/modules/cassandra/pages/managing/operating/compaction/stcs.adoc b/doc/modules/cassandra/pages/managing/operating/compaction/stcs.adoc index c31635ae0dad..382f29129bd7 100644 --- a/doc/modules/cassandra/pages/managing/operating/compaction/stcs.adoc +++ b/doc/modules/cassandra/pages/managing/operating/compaction/stcs.adoc @@ -7,15 +7,15 @@ include::cassandra:partial$ucs-recommend.adoc[] The `SizeTieredCompactionStrategy (STCS)` is recommended for write-intensive workloads, and is the legacy recommended compaction strategy. It is the default compaction strategy if no other strategy is specified. -STCS initiates compaction when {cassandra} has accumulated a set number (default: 4) of similar-sized SSTables. -STCS merges these SSTables into one larger SSTable. -As these larger SSTables accumulate, STCS merges them into even larger SSTables. +STCS initiates compaction when {cassandra} has accumulated a set number (default: 4) of similar-sized SSTables. +STCS merges these SSTables into one larger SSTable. +As these larger SSTables accumulate, STCS merges them into even larger SSTables. At any given time, several SSTables of varying sizes are present. -While STCS works well to compact a write-intensive workload, it makes reads slower because the merge-by-size process does not group data by rows. -This fact makes it more likely that versions of a particular row may be spread over many SSTables. +While STCS works well to compact a write-intensive workload, it makes reads slower because the merge-by-size process does not group data by rows. +This fact makes it more likely that versions of a particular row may be spread over many SSTables. Also, STCS does not evict deleted data predictably, because its trigger for compaction is SSTable size. -However, SSTables may not grow quickly enough to merge and evict old data regularly. +However, SSTables may not grow quickly enough to merge and evict old data regularly. Most STCS compactions are minor compactions, which merge a few SSTables into one. In contrast, when executing a major compaction with STCS, two SSTables per data directory, one for repaired data and one for unrepaired data, will exist during the compaction. @@ -35,62 +35,63 @@ include::cassandra:partial$default-compaction-strategy.adoc[] == STCS options SizeTieredCompactionStrategy (STCS) options are set per table using table options. -The `min_threshold` option of a table is the main value that triggers a minor compaction. -Minor compactions do not involve all the tables in a keyspace. +The `min_threshold` option of a table is the main value that triggers a minor compaction. +Minor compactions do not involve all the tables in a keyspace. [cols="1,2"] |=== | Subproperty | Description -| enabled -| Enables background compaction. +| enabled +| Enables background compaction. Default value: true // See Enabling and disabling background compaction. -| tombstone_compaction_interval -| The minimum number of seconds after which an SSTable is created before {cassandra} considers the SSTable for tombstone compaction. -An SSTable is eligible for tombstone compaction if the table exceeds the `tombstone_threshold` ratio. +| tombstone_compaction_interval +| The minimum number of seconds after which an SSTable is created before {cassandra} considers the SSTable for tombstone compaction. +An SSTable is eligible for tombstone compaction if the table exceeds the `tombstone_threshold` ratio. Default value: 86400 -| tombstone_threshold -| The ratio of garbage-collectable tombstones to all contained columns. -If the ratio exceeds this limit, {cassandra} starts compaction on that table alone, to purge the tombstones. +| tombstone_threshold +| The ratio of garbage-collectable tombstones to all contained columns. +If the ratio exceeds this limit, {cassandra} starts compaction on that table alone, to purge the tombstones. Default value: 0.2 -| unchecked_tombstone_compaction -| If set to `true`, allows {cassandra} to run tombstone compaction without pre-checking which tables are eligible for this operation. +| unchecked_tombstone_compaction +| If set to `true`, allows {cassandra} to run tombstone compaction without pre-checking which tables are eligible for this operation. Even without this pre-check, {cassandra} checks an SSTable to make sure it is safe to drop tombstones. Default value: false -| log_all +| log_all | Activates advanced logging for the entire cluster. Default value: false -| max_threshold +| max_threshold | The maximum number of SSTables to allow in a minor compaction. Default value: 32 -| min_threshold -| The minimum number of SSTables to trigger a minor compaction. +| min_threshold +| The minimum number of SSTables to trigger a minor compaction. Default value: 4 -| bucket_high +| bucket_high | An SSTable is added to a bucket if its size is less than 150% of the average size of that bucket. For example, if the SSTable size is 13 MB, and the bucket average size is 10 MB, then the SSTable will be added to that bucket and the new average size will be computed for that bucket. Default value: 1.5 -| bucket_low +| bucket_low | An SSTable is added to a bucket if the SSTable size is greater than 50% of the average size of that bucket. For example, if the SSTable size is 6 MB, and the bucket average size is 10 MB, then the SSTable will be added to that bucket and the new average size will be computed for that bucket. Default value: 0.5 -| min_sstable_size -| SSTables smaller than this value will be grouped into one bucket where the average size is less than this setting. -Default value: 50MB +| min_sstable_size +| SSTables smaller than this value will be grouped into one bucket where the average size is less than this setting. +Accepts either a plain byte count (`52428800`) or a size with a unit suffix (`50MiB`). +Default value: `52428800` (50MiB) -| only_purge_repaired_tombstones -| If set to `true`, allows purging tombstones only from repaired SSTables. -The purpose is to prevent data from resurrecting if repair is not run within `gc_grace_seconds`. -If you do not run repair for a long time, {cassandra} keeps all tombstones — this may cause problems. +| only_purge_repaired_tombstones +| If set to `true`, allows purging tombstones only from repaired SSTables. +The purpose is to prevent data from resurrecting if repair is not run within `gc_grace_seconds`. +If you do not run repair for a long time, {cassandra} keeps all tombstones — this may cause problems. Default value: false -|=== \ No newline at end of file +|=== diff --git a/doc/modules/cassandra/pages/reference/cql-commands/compact-subproperties.adoc b/doc/modules/cassandra/pages/reference/cql-commands/compact-subproperties.adoc index 91763fd0e790..292bc1c259ed 100644 --- a/doc/modules/cassandra/pages/reference/cql-commands/compact-subproperties.adoc +++ b/doc/modules/cassandra/pages/reference/cql-commands/compact-subproperties.adoc @@ -153,7 +153,7 @@ compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_high' : , 'bucket_low' : , - 'min_sstable_size' : , + 'min_sstable_size' : , 'min_threshold' : , 'max_threshold' : } ---- @@ -190,9 +190,10 @@ min_sstable_size:: STCS groups SSTables into buckets. The bucketing process groups SSTables that differ in size by less than 50%. This bucketing process is too fine-grained for small SSTables. -If your SSTables are small, use this option to define a size threshold in MB below which all SSTables belong to one unique bucket. +If your SSTables are small, use this option to define a size threshold below which all SSTables belong to one unique bucket. +Accepts either a plain byte count (`52428800`) or a size with a unit suffix (`50MiB`). + -Default: `50` (MB) +Default: `52428800` (50MiB) [NOTE] ==== diff --git a/doc/modules/cassandra/partials/compact-subproperties.adoc b/doc/modules/cassandra/partials/compact-subproperties.adoc index 1ca766211385..0a5b14dabd95 100644 --- a/doc/modules/cassandra/partials/compact-subproperties.adoc +++ b/doc/modules/cassandra/partials/compact-subproperties.adoc @@ -123,7 +123,7 @@ compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_high' : , 'bucket_low' : , - 'min_sstable_size' : } + 'min_sstable_size' : } ---- *bucket_high* :: @@ -144,9 +144,10 @@ Default: `0.5` STCS groups SSTables into buckets. The bucketing process groups SSTables that differ in size by less than 50%. This bucketing process is too fine-grained for small SSTables. -If your SSTables are small, use this option to define a size threshold in MB below which all SSTables belong to one unique bucket. +If your SSTables are small, use this option to define a size threshold below which all SSTables belong to one unique bucket. +Accepts either a plain byte count (`52428800`) or a size with a unit suffix (`50MiB`). + -Default: `50` (MB) +Default: `52428800` (50MiB) [NOTE] ==== diff --git a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java index 37c6e95f0381..e212f25f3710 100644 --- a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java @@ -630,4 +630,9 @@ public static Map validateOptions(Map options) t return uncheckedOptions; } + + public static Map normalizeOptions(Map options) + { + return SizeTieredCompactionStrategyOptions.normalizeOptions(options); + } } diff --git a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java index 74a96ca211ab..9bb84d1c7bc7 100644 --- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java @@ -320,6 +320,11 @@ public static Map validateOptions(Map options) t return uncheckedOptions; } + public static Map normalizeOptions(Map options) + { + return SizeTieredCompactionStrategyOptions.normalizeOptions(options); + } + @Override public synchronized void addSSTable(SSTableReader added) { diff --git a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java index eb1d8f97afe2..5e17cb938ce0 100644 --- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java +++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyOptions.java @@ -17,9 +17,11 @@ */ package org.apache.cassandra.db.compaction; +import java.util.HashMap; import java.util.Map; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.FBUtilities; public final class SizeTieredCompactionStrategyOptions { @@ -37,7 +39,7 @@ public final class SizeTieredCompactionStrategyOptions public SizeTieredCompactionStrategyOptions(Map options) { String optionValue = options.get(MIN_SSTABLE_SIZE_KEY); - minSSTableSize = optionValue == null ? DEFAULT_MIN_SSTABLE_SIZE : Long.parseLong(optionValue); + minSSTableSize = parseMinSSTableSize(optionValue); optionValue = options.get(BUCKET_LOW_KEY); bucketLow = optionValue == null ? DEFAULT_BUCKET_LOW : Double.parseDouble(optionValue); optionValue = options.get(BUCKET_HIGH_KEY); @@ -51,6 +53,46 @@ public SizeTieredCompactionStrategyOptions() bucketHigh = DEFAULT_BUCKET_HIGH; } + /** + * Parses {@link #MIN_SSTABLE_SIZE_KEY}, which accepts either a plain byte count (e.g. {@code 52428800}) or a + * human-readable size that must be suffixed with the unit (e.g. {@code 50MiB}). + */ + private static long parseMinSSTableSize(String optionValue) throws NumberFormatException + { + if (optionValue == null) + return DEFAULT_MIN_SSTABLE_SIZE; + + return optionValue.endsWith("B") ? FBUtilities.parseHumanReadableBytes(optionValue) + : Long.parseLong(optionValue); + } + + /** + * Rewrites {@link #MIN_SSTABLE_SIZE_KEY} to its canonical plain byte count so that the + * value we persist in the schema is always parsable by nodes that predate unit support. + *

+ * Unparsable values are left untouched for {@link #validateOptions} to reject. + */ + public static Map normalizeOptions(Map options) + { + String optionValue = options.get(MIN_SSTABLE_SIZE_KEY); + if (optionValue == null) + return options; + + long minSSTableSize; + try + { + minSSTableSize = parseMinSSTableSize(optionValue); + } + catch (NumberFormatException e) + { + return options; + } + + Map normalized = new HashMap<>(options); + normalized.put(MIN_SSTABLE_SIZE_KEY, Long.toString(minSSTableSize)); + return normalized; + } + private static double parseDouble(Map options, String key, double defaultValue) throws ConfigurationException { String optionValue = options.get(key); @@ -69,7 +111,7 @@ public static Map validateOptions(Map options, M String optionValue = options.get(MIN_SSTABLE_SIZE_KEY); try { - long minSSTableSize = optionValue == null ? DEFAULT_MIN_SSTABLE_SIZE : Long.parseLong(optionValue); + long minSSTableSize = parseMinSSTableSize(optionValue); if (minSSTableSize < 0) { throw new ConfigurationException(String.format("%s must be non negative: %d", MIN_SSTABLE_SIZE_KEY, minSSTableSize)); @@ -77,7 +119,7 @@ public static Map validateOptions(Map options, M } catch (NumberFormatException e) { - throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", optionValue, MIN_SSTABLE_SIZE_KEY), e); + throw new ConfigurationException(String.format("%s is not a valid size in bytes for %s", optionValue, MIN_SSTABLE_SIZE_KEY), e); } double bucketLow = parseDouble(options, BUCKET_LOW_KEY, DEFAULT_BUCKET_LOW); diff --git a/src/java/org/apache/cassandra/db/compaction/TimeWindowCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/TimeWindowCompactionStrategy.java index 2709d43ae56d..addf1c8b618e 100644 --- a/src/java/org/apache/cassandra/db/compaction/TimeWindowCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/TimeWindowCompactionStrategy.java @@ -445,6 +445,11 @@ public static Map validateOptions(Map options) t return uncheckedOptions; } + public static Map normalizeOptions(Map options) + { + return SizeTieredCompactionStrategyOptions.normalizeOptions(options); + } + public String toString() { return String.format("TimeWindowCompactionStrategy[%s/%s]", diff --git a/src/java/org/apache/cassandra/schema/CompactionParams.java b/src/java/org/apache/cassandra/schema/CompactionParams.java index 1194d621b7fc..441038d192e8 100644 --- a/src/java/org/apache/cassandra/schema/CompactionParams.java +++ b/src/java/org/apache/cassandra/schema/CompactionParams.java @@ -134,7 +134,7 @@ public static CompactionParams create(Class allOptions = new HashMap<>(options); + Map allOptions = new HashMap<>(normalizeOptions(klass, options)); if (supportsThresholdParams(klass)) { allOptions.putIfAbsent(Option.MIN_THRESHOLD.toString(), Integer.toString(DEFAULT_MIN_THRESHOLD)); @@ -144,6 +144,44 @@ public static CompactionParams create(Class + * The hook is opt-in: strategies that do not declare a static {@code normalizeOptions(Map)} - including + * {@link UnifiedCompactionStrategy}, whose size options require the unit suffix - are left alone. + */ + @SuppressWarnings("unchecked") + private static Map normalizeOptions(Class klass, + Map options) + { + try + { + return (Map) klass.getMethod("normalizeOptions", Map.class).invoke(null, options); + } + catch (NoSuchMethodException e) + { + return options; + } + catch (InvocationTargetException e) + { + if (e.getTargetException() instanceof ConfigurationException) + throw (ConfigurationException) e.getTargetException(); + + Throwable cause = e.getCause() == null ? e : e.getCause(); + throw new ConfigurationException(format("%s.normalizeOptions() threw an error: %s %s", + klass.getName(), + cause.getClass().getName(), + cause.getMessage()), + e); + } + catch (IllegalAccessException e) + { + throw new ConfigurationException("Cannot access method normalizeOptions in " + klass.getName(), e); + } + } + public static CompactionParams stcs(Map options) { return create(SizeTieredCompactionStrategy.class, options); diff --git a/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java b/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java index dac331e1170d..69015cfb5398 100644 --- a/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategyTest.java @@ -19,6 +19,7 @@ import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -91,6 +92,45 @@ public void testOptionsValidation() throws ConfigurationException assertTrue(unvalidated.containsKey("bad_option")); } + @Test + public void testMinSSTableSizeUnits() throws ConfigurationException + { + Map options = new HashMap<>(); + options.put(SizeTieredCompactionStrategyOptions.BUCKET_LOW_KEY, "0.5"); + options.put(SizeTieredCompactionStrategyOptions.BUCKET_HIGH_KEY, "1.5"); + + assertTrue(validateOptions(options).isEmpty()); + assertEquals(50L * 1024 * 1024, new SizeTieredCompactionStrategyOptions(options).minSSTableSize); + assertEquals(50L * 1024 * 1024, new SizeTieredCompactionStrategyOptions().minSSTableSize); + + List> accepted = Arrays.asList(Pair.create("52428800", 50L * 1024 * 1024), + Pair.create("50MiB", 50L * 1024 * 1024), + Pair.create("50 MiB", 50L * 1024 * 1024), + Pair.create("50MB", 49999999L), + Pair.create("1GiB", 1024L * 1024 * 1024), + Pair.create("0", 0L)); + for (Pair option : accepted) + { + options.put(SizeTieredCompactionStrategyOptions.MIN_SSTABLE_SIZE_KEY, option.left); + assertTrue(validateOptions(options).isEmpty()); + assertEquals((long) option.right, new SizeTieredCompactionStrategyOptions(options).minSSTableSize); + } + + for (String rejected : Arrays.asList("50Mi", "MiB", "fifty", "-1B", "-1")) + { + options.put(SizeTieredCompactionStrategyOptions.MIN_SSTABLE_SIZE_KEY, rejected); + try + { + validateOptions(options); + fail(String.format("min_sstable_size '%s' should be rejected", rejected)); + } + catch (ConfigurationException e) + { + // expected + } + } + } + @Test public void testGetBuckets() { diff --git a/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java b/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java index 3831c2db254c..f757abbe8130 100644 --- a/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java @@ -18,9 +18,17 @@ */ package org.apache.cassandra.schema; +import java.util.Map; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import org.junit.Test; import org.apache.cassandra.db.compaction.AbstractCompactionStrategy; +import org.apache.cassandra.db.compaction.LeveledCompactionStrategy; +import org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy; +import org.apache.cassandra.db.compaction.TimeWindowCompactionStrategy; +import org.apache.cassandra.db.compaction.UnifiedCompactionStrategy; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; import org.apache.cassandra.utils.ClassLoadingTestSupport; @@ -30,6 +38,48 @@ public class CompactionParamsTest { + private static final String MIN_SSTABLE_SIZE = "min_sstable_size"; + + /** + * We persist only the plain byte count in the schema to ensure we keep compatibility + * previous cassandra versions that didn't support storage units. + */ + @Test + public void testMinSSTableSizeIsNormalizedBeforeSerialization() + { + for (Class klass : ImmutableList.of(SizeTieredCompactionStrategy.class, + LeveledCompactionStrategy.class, + TimeWindowCompactionStrategy.class)) + { + for (String written : ImmutableList.of("50MiB", "50 MiB", "51200KiB", "52428800")) + { + CompactionParams params = CompactionParams.create(klass, ImmutableMap.of(MIN_SSTABLE_SIZE, written)); + params.validate(); + assertThat(params.options()).containsEntry(MIN_SSTABLE_SIZE, "52428800"); + assertThat(params.asMap()).containsEntry(MIN_SSTABLE_SIZE, "52428800"); + } + + // an unparsable value is left untouched so that validate() can reject it with a proper message + CompactionParams invalid = CompactionParams.create(klass, ImmutableMap.of(MIN_SSTABLE_SIZE, "50Mi")); + assertThat(invalid.options()).containsEntry(MIN_SSTABLE_SIZE, "50Mi"); + assertThatThrownBy(invalid::validate).isInstanceOf(ConfigurationException.class) + .hasMessageContaining(MIN_SSTABLE_SIZE); + } + } + + /** + * UCS parses its size options with {@link org.apache.cassandra.utils.FBUtilities#parseHumanReadableBytes}, which + * requires the unit suffix, so its options must not be rewritten into a bare byte count. + */ + @Test + public void testUnifiedCompactionStrategyOptionsAreNotNormalized() + { + Map options = ImmutableMap.of(MIN_SSTABLE_SIZE, "50MiB", "target_sstable_size", "1GiB"); + CompactionParams params = CompactionParams.create(UnifiedCompactionStrategy.class, options); + params.validate(); + assertThat(params.options()).containsEntry(MIN_SSTABLE_SIZE, "50MiB"); + } + @Test public void testRejectsNonCompactionStrategyWithoutInitializing() {