CASSANDRA-21617: STCS min_sstable_size should accept data storage units

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)

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.

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
-|===
+|===

doc/modules/cassandra/pages/reference/cql-commands/compact-subproperties.adoc

@@ -153,7 +153,7 @@ compaction = {
      'class' : 'SizeTieredCompactionStrategy',
      'bucket_high' : <factor>,
      'bucket_low' : <factor>,
-     'min_sstable_size' : <int>,
+     'min_sstable_size' : <size>,
      'min_threshold' : <int>,
      'max_threshold' : <int> }
 ----
@@ -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]
 ==== 

doc/modules/cassandra/partials/compact-subproperties.adoc

@@ -123,7 +123,7 @@ compaction = {
      'class' : 'SizeTieredCompactionStrategy',
      'bucket_high' : <factor>,
      'bucket_low' : <factor>,
-     'min_sstable_size' : <int> }
+     'min_sstable_size' : <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]
 ====

src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java

@@ -630,4 +630,9 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
 
         return uncheckedOptions;
     }
+
+    public static Map<String, String> normalizeOptions(Map<String, String> options)
+    {
+        return SizeTieredCompactionStrategyOptions.normalizeOptions(options);
+    }
 }

src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java

@@ -320,6 +320,11 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
         return uncheckedOptions;
     }
 
+    public static Map<String, String> normalizeOptions(Map<String, String> options)
+    {
+        return SizeTieredCompactionStrategyOptions.normalizeOptions(options);
+    }
+
     @Override
     public synchronized void addSSTable(SSTableReader added)
     {

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<String, String> 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.
+     * <p>
+     * Unparsable values are left untouched for {@link #validateOptions} to reject.
+     */
+    public static Map<String, String> normalizeOptions(Map<String, String> 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<String, String> normalized = new HashMap<>(options);
+        normalized.put(MIN_SSTABLE_SIZE_KEY, Long.toString(minSSTableSize));
+        return normalized;
+    }
+
     private static double parseDouble(Map<String, String> options, String key, double defaultValue) throws ConfigurationException
     {
         String optionValue = options.get(key);
@@ -69,15 +111,15 @@ public static Map<String, String> validateOptions(Map<String, String> 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));
             }
         }
         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);

src/java/org/apache/cassandra/db/compaction/TimeWindowCompactionStrategy.java

@@ -445,6 +445,11 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
         return uncheckedOptions;
     }
 
+    public static Map<String, String> normalizeOptions(Map<String, String> options)
+    {
+        return SizeTieredCompactionStrategyOptions.normalizeOptions(options);
+    }
+
     public String toString()
     {
         return String.format("TimeWindowCompactionStrategy[%s/%s]",

src/java/org/apache/cassandra/schema/CompactionParams.java

@@ -134,7 +134,7 @@ public static CompactionParams create(Class<? extends AbstractCompactionStrategy
         }
         TombstoneOption tombstoneOption = tombstoneOptional.get();
 
-        Map<String, String> allOptions = new HashMap<>(options);
+        Map<String, String> 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<? extends AbstractCompactionStrategy
         return new CompactionParams(klass, allOptions, isEnabled, tombstoneOption);
     }
 
+    /**
+     * Gives the strategy a chance to canonicalize option values before we persist them in the schema and propagate
+     * them to the rest of the cluster. This keeps human-readable inputs such as {@code min_sstable_size = '50MiB'}
+     * from reaching nodes that only know how to parse a plain byte count.
+     * <p>
+     * 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<String, String> normalizeOptions(Class<? extends AbstractCompactionStrategy> klass,
+                                                        Map<String, String> options)
+    {
+        try
+        {
+            return (Map<String, String>) 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<String, String> options)
     {
         return create(SizeTieredCompactionStrategy.class, options);