Harden the possible range of values for max dictionary size and max total sample size for dictionary training

patch by Stefan Miklosovic; reviewed by Yifan Cai for CASSANDRA-21194

Author: smiklosovic

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Harden the possible range of values for max dictionary size and max total sample size for dictionary training (CASSANDRA-21194)
  * Implement a guardrail ensuring that minimum training frequency parameter is provided in ZstdDictionaryCompressor (CASSANDRA-21192)
  * Replace manual referencing with ColumnFamilyStore.selectAndReference when training a dictionary (CASSANDRA-21188)
  * Forbid nodes upgrading to a version which cannot read existing log entries (CASSANDRA-21174)

src/java/org/apache/cassandra/db/compression/CompressionDictionaryManager.java

@@ -471,7 +471,7 @@ private int internalTrainingParameterResolution(CompressionParams compressionPar
             else
                 resolvedValue = userSuppliedValue;
 
-            return new DataStorageSpec.IntKibibytesBound(resolvedValue).toBytes();
+            return new DataStorageSpec.IntBytesBound(resolvedValue).toBytes();
         }
         catch (Throwable t)
         {

src/java/org/apache/cassandra/io/compress/IDictionaryCompressor.java

@@ -59,7 +59,7 @@ static void validateSizeBasedTrainingParameter(String parameterName, String reso
     {
         try
         {
-            new DataStorageSpec.IntKibibytesBound(resolvedValue).toBytes();
+            new DataStorageSpec.IntBytesBound(resolvedValue).toBytes();
         }
         catch (Throwable t)
         {

src/java/org/apache/cassandra/tools/nodetool/CompressionDictionaryCommandGroup.java

@@ -182,7 +182,7 @@ private static void validateParameters(PrintStream err, String trainingMaxDictio
             {
                 try
                 {
-                    new DataStorageSpec.IntKibibytesBound(trainingMaxDictionarySize).toBytes();
+                    new DataStorageSpec.IntBytesBound(trainingMaxDictionarySize).toBytes();
                 }
                 catch (Throwable t)
                 {
@@ -195,7 +195,7 @@ private static void validateParameters(PrintStream err, String trainingMaxDictio
             {
                 try
                 {
-                    new DataStorageSpec.IntKibibytesBound(trainingMaxTotalSampleSize).toBytes();
+                    new DataStorageSpec.IntBytesBound(trainingMaxTotalSampleSize).toBytes();
                 }
                 catch (Throwable t)
                 {

test/unit/org/apache/cassandra/db/compression/CompressionDictionaryIntegrationTest.java

@@ -234,6 +234,33 @@ public void testSSTableBasedTrainingWithoutSSTables()
         .hasMessageContaining("No SSTables available for training");
     }
 
+    @Test
+    public void testMaxBoundForTrainingParameters()
+    {
+        String table = createTable(getTableCql());
+        ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(table);
+        CompressionDictionaryManager manager = cfs.compressionDictionaryManager();
+
+        assertThatThrownBy(() -> manager.train(false, Map.of(TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_NAME, "5GiB",
+                                                             TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME, TRAINING_MAX_TOTAL_SAMPLE_SIZE)))
+        .as("Should fail when " + TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_NAME + " is bigger than " + Integer.MAX_VALUE)
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Invalid value for training_max_dictionary_size: 5GiB");
+
+        assertThatThrownBy(() -> manager.train(false, Map.of(TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_NAME, TRAINING_MAX_DICTIONARY_SIZE,
+                                                             TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME, "5GiB")))
+        .as("Should fail when " + TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME + " is bigger than " + Integer.MAX_VALUE)
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Invalid value for " + TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME + ": 5GiB");
+    }
+
+    @Test
+    public void testInvalidTableCreation()
+    {
+        assertThatThrownBy(() -> createTable(getTableCqlWithInvalidTotalMaxSampleSize())).isInstanceOf(RuntimeException.class);
+        assertThatThrownBy(() -> createTable(getTableCqlWithInvalidMaxDictionarySize())).isInstanceOf(RuntimeException.class);
+    }
+
     private String getTableCqlWithChunkLength()
     {
         return "CREATE TABLE %s (pk text PRIMARY KEY, data text) " +
@@ -254,4 +281,24 @@ private String getTableCql()
                '\'' + TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME + "': '" + DEFAULT_TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_VALUE + '\'' +
                '}';
     }
+
+    private String getTableCqlWithInvalidTotalMaxSampleSize()
+    {
+        return "CREATE TABLE %s (pk text PRIMARY KEY, data text) " +
+               "WITH compression = {" +
+               "'class': 'ZstdDictionaryCompressor'," +
+               '\'' + TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_NAME + "': '" + DEFAULT_TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_VALUE + "'," +
+               '\'' + TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME + "': '5GiB'" +
+               '}';
+    }
+
+    private String getTableCqlWithInvalidMaxDictionarySize()
+    {
+        return "CREATE TABLE %s (pk text PRIMARY KEY, data text) " +
+               "WITH compression = {" +
+               "'class': 'ZstdDictionaryCompressor'," +
+               '\'' + TRAINING_MAX_DICTIONARY_SIZE_PARAMETER_NAME + "': '5GiB'," +
+               '\'' + TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_NAME + "': '" + DEFAULT_TRAINING_MAX_TOTAL_SAMPLE_SIZE_PARAMETER_VALUE + '\'' +
+               '}';
+    }
 }