(CASSANDRA-20398) Validating TARGET_SSTABLE_SIZE_OPTION and MIN_SSTABLE_SIZE_OPTION

The values were getting truncated resulting in unexpected output.

patch by Pranav Shenoy; reviewed by Claude Warren for CASSANDRA-20398

Author: pranavshenoy

src/java/org/apache/cassandra/db/compaction/unified/Controller.java

@@ -518,14 +518,20 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
         {
             try
             {
-                targetSSTableSize = FBUtilities.parseHumanReadableBytes(s);
-                if (targetSSTableSize < MIN_TARGET_SSTABLE_SIZE)
+                double targetSize = FBUtilities.parseHumanReadable(s, null, "B");
+                if (targetSize >= Long.MAX_VALUE) {
+                    throw new ConfigurationException(String.format("%s %s is out of range of Long.",
+                                                                    TARGET_SSTABLE_SIZE_OPTION,
+                                                                    s));
+                }
+                if (targetSize < MIN_TARGET_SSTABLE_SIZE)
                 {
                     throw new ConfigurationException(String.format("%s %s is not acceptable, size must be at least %s",
                                                                    TARGET_SSTABLE_SIZE_OPTION,
                                                                    s,
                                                                    FBUtilities.prettyPrintMemory(MIN_TARGET_SSTABLE_SIZE)));
                 }
+                targetSSTableSize = (long) Math.ceil(targetSize);
             }
             catch (NumberFormatException e)
             {
@@ -622,12 +628,12 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
                 if (sizeInBytes < 0)
                     throw new ConfigurationException(String.format("Invalid configuration, %s should be greater than or equal to 0 (zero)",
                                                                    MIN_SSTABLE_SIZE_OPTION));
-                int limit = (int) Math.ceil(targetSSTableSize * INVERSE_SQRT_2);
+                long limit = (long) Math.ceil(targetSSTableSize * INVERSE_SQRT_2);
                 if (sizeInBytes >= limit)
-                    throw new ConfigurationException(String.format("Invalid configuration, %s (%s) should be less than the target size minimum: %s",
+                    throw new ConfigurationException(String.format("Invalid configuration, %s (%s) should be less than 70%% of the targetSSTableSize (%s)",
                                                                    MIN_SSTABLE_SIZE_OPTION,
                                                                    FBUtilities.prettyPrintMemory(sizeInBytes),
-                                                                   FBUtilities.prettyPrintMemory(limit)));
+                                                                   FBUtilities.prettyPrintMemory(targetSSTableSize)));
             }
             catch (NumberFormatException e)
             {

test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java

@@ -121,6 +121,64 @@ public void testValidateOptionsIntegers()
         testValidateOptions(true);
     }
 
+    public void targetSSTableSizeValidator(String inputSize) 
+    {
+        Map<String, String> options = new HashMap<>();
+        options.putIfAbsent(Controller.TARGET_SSTABLE_SIZE_OPTION, inputSize);
+        assertThatExceptionOfType(ConfigurationException.class)
+        .describedAs("Should have thrown a ConfigurationException when target_sstable_size is greater than Long.MAX_VALUE")
+        .isThrownBy(() -> Controller.validateOptions(options))
+        .withMessageContaining(format("target_sstable_size %s is out of range of Long.", inputSize));
+    }
+
+    @Test
+    public void testCassandra20398Values()
+    {
+        //TARGET_SSTABLE_SIZE_OPTION = 12E899, the value reported in CASSANDRA-20398
+        String inputSize = "12E899 B";
+        targetSSTableSizeValidator(inputSize); 
+    }
+
+    @Test
+    public void testValidateOptionsTargetSSTableSizeGTLongMax()
+    {
+        //TARGET_SSTABLE_SIZE_OPTION > LONG.MAX_VALUE 
+        // the inputSize is Long.MAX_VALUE + 100
+        String inputSize = "9223372036854775907 B"; 
+        targetSSTableSizeValidator(inputSize); 
+    }
+
+    @Test
+    public void testValidateOptionsTargetSSTableSizeLTMinTargetSize()
+    {
+        // TARGET_SSTABLE_SIZE_OPTION < Default MIN_TARGET_SSTABLE_SIZE (1048576) 
+        Map<String, String> options = new HashMap<>();
+        String inputSize = "1048000 B";
+        options.putIfAbsent(Controller.TARGET_SSTABLE_SIZE_OPTION, inputSize);
+        assertThatExceptionOfType(ConfigurationException.class)
+        .describedAs("Should have thrown a ConfigurationException when target_sstable_size is less than default MIN_TARGET_SSTABLE_SIZE")
+        .isThrownBy(() -> Controller.validateOptions(options))
+        .withMessageContaining(format("target_sstable_size %s is not acceptable, size must be at least %s", inputSize, FBUtilities.prettyPrintMemory(Controller.MIN_TARGET_SSTABLE_SIZE)));
+    }
+
+    @Test
+    public void testValidateOptionsTargetSSTableSizeGTIntMax()
+    {
+        //TEST 4: Verifying if TARGET_SSTABLE_SIZE_OPTION (3650722199) < MIN_TARGET_SSTABLE_SIZE (2581450423)
+        // Previously, TARGET_SSTABLE_SIZE_OPTION * 0.7 was stored as Integer which would 3650722199 * 0.7 = 2147483647
+        // By storing it in a Long, 3650722199 * 0.7 = 2581450424. If TARGET_SSTABLE_SIZE_OPTION * 0.7 is truncated, 
+        //this test case will fail
+        try
+        {
+            Map<String, String> options = new HashMap<>();
+            options.putIfAbsent(Controller.TARGET_SSTABLE_SIZE_OPTION, "3650722199 B");
+            options.putIfAbsent(Controller.MIN_SSTABLE_SIZE_OPTION, "2581450423 B");
+            Controller.validateOptions(options);
+        } catch(ConfigurationException e) {
+            fail("3650722199 * 0.7 got truncated. " + e.getMessage());
+        }
+    }
+
     void testValidateOptions(boolean useIntegers)
     {
         Map<String, String> options = new HashMap<>();
@@ -579,7 +637,7 @@ public void testMinSSTableSize()
         assertThatExceptionOfType(ConfigurationException.class)
         .describedAs("Should have thrown a ConfigurationException when min_sstable_size is greater than target_sstable_size")
         .isThrownBy(() -> Controller.validateOptions(options))
-        .withMessageContaining(format("less than the target size minimum: %s", FBUtilities.prettyPrintMemory(limit)));
+        .withMessageContaining(format("Invalid configuration, %s (%s) should be less than 70%% of the targetSSTableSize (%s)", Controller.MIN_SSTABLE_SIZE_OPTION,  FBUtilities.prettyPrintMemory(limit+1), FBUtilities.prettyPrintMemory(Controller.DEFAULT_TARGET_SSTABLE_SIZE)));
 
         // test min < configured target table size * INV_SQRT_2
         limit = (int) Math.ceil(Controller.MIN_TARGET_SSTABLE_SIZE * 2 * Controller.INVERSE_SQRT_2);
@@ -589,6 +647,6 @@ public void testMinSSTableSize()
         assertThatExceptionOfType(ConfigurationException.class)
         .describedAs("Should have thrown a ConfigurationException when min_sstable_size is greater than target_sstable_size")
         .isThrownBy(() -> Controller.validateOptions(options))
-        .withMessageContaining(format("less than the target size minimum: %s", FBUtilities.prettyPrintMemory(limit)));
+        .withMessageContaining(format("Invalid configuration, %s (%s) should be less than 70%% of the targetSSTableSize (%s)", Controller.MIN_SSTABLE_SIZE_OPTION, FBUtilities.prettyPrintMemory(limit + 1), FBUtilities.prettyPrintMemory(Controller.MIN_TARGET_SSTABLE_SIZE * 2)));
     }
 }