Claudenw commented on code in PR #2966:
URL: https://github.com/apache/cassandra/pull/2966#discussion_r1432614651
##########
src/java/org/apache/cassandra/db/compaction/unified/Controller.java:
##########
@@ -596,10 +598,15 @@ public static Map<String, String>
validateOptions(Map<String, String> options) t
try
{
long sizeInBytes = FBUtilities.parseHumanReadableBytes(s);
+ // zero is a valid option to disable featue
if (sizeInBytes < 0)
- throw new ConfigurationException(String.format("Invalid
configuration, %s should be positive: %s",
-
MIN_SSTABLE_SIZE_OPTION,
- s));
+ 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);
+ if (sizeInBytes >= limit )
+ throw new ConfigurationException(String.format("Invalid
configuration, %s (%sB) should be less than: %sB",
Review Comment:
If you take a value for targetSSTableSize and convert it to bytes, multiply
that by INVERSE_SQRT_2, subtract 1, and use FBUtilities.prettyPrintMemory() to
convert it to the human readable string. Then take that string and parse it
back to a value the value will often be too large for the minimum value. There
are a range of value where this will occur. The issue is that
FBUtilities.prettyPrintMemory() seems to round up which makes sense for most of
the values we use if for, but in this case we have an upper limit that we do
not want to exceed and so we run into problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
