From 3386599193236ce30b65931bb2f8de9084d6a65a Mon Sep 17 00:00:00 2001 From: Pranav Shenoy Date: Fri, 11 Apr 2025 22:44:30 -0700 Subject: [PATCH] (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 --- .../db/compaction/unified/Controller.java | 16 +++-- .../db/compaction/unified/ControllerTest.java | 62 ++++++++++++++++++- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/java/org/apache/cassandra/db/compaction/unified/Controller.java b/src/java/org/apache/cassandra/db/compaction/unified/Controller.java index df2a66590c30..567b58552194 100644 --- a/src/java/org/apache/cassandra/db/compaction/unified/Controller.java +++ b/src/java/org/apache/cassandra/db/compaction/unified/Controller.java @@ -518,14 +518,20 @@ public static Map validateOptions(Map 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 validateOptions(Map 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) { diff --git a/test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java b/test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java index 7b8f0477e792..eaa4d6f77a27 100644 --- a/test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java +++ b/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 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 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 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 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))); } } \ No newline at end of file