diff --git a/CHANGES.txt b/CHANGES.txt index e4ed0d362f..b4996c26ba 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -329,6 +329,7 @@ Merged from 4.1: * Enforce CQL message size limit on multiframe messages (CASSANDRA-20052) * Fix race condition in DecayingEstimatedHistogramReservoir during rescale (CASSANDRA-19365) Merged from 4.0: + * Leveled Compaction doesn't validate maxBytesForLevel when the table is altered/created (CASSANDRA-20570) * Updated dtest-api to 0.0.18 and removed JMX-related classes that now live in the dtest-api (CASSANDRA-20884) * Fixed incorrect error message constant for keyspace name length validation (CASSANDRA-20915) * Prevent too long table names not fitting file names (CASSANDRA-20389) diff --git a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java index 1509aa2e03..e0664a9bc6 100644 --- a/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java +++ b/src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java @@ -18,6 +18,7 @@ package org.apache.cassandra.db.compaction; import java.util.*; +import java.math.BigInteger; import com.google.common.annotations.VisibleForTesting; @@ -43,6 +44,7 @@ import org.apache.cassandra.io.sstable.ISSTableScanner; import org.apache.cassandra.io.sstable.format.SSTableReader; import static org.apache.cassandra.config.CassandraRelevantProperties.TOLERATE_SSTABLE_SIZE; +import static org.apache.cassandra.db.compaction.LeveledGenerations.MAX_LEVEL_COUNT; public class LeveledCompactionStrategy extends AbstractCompactionStrategy { @@ -569,10 +571,14 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy { Map uncheckedOptions = AbstractCompactionStrategy.validateOptions(options); + int ssSize; + int fanoutSize; + + // Validate the sstable_size option String size = options.containsKey(SSTABLE_SIZE_OPTION) ? options.get(SSTABLE_SIZE_OPTION) : "1"; try { - int ssSize = Integer.parseInt(size); + ssSize = Integer.parseInt(size); if (ssSize < 1) { throw new ConfigurationException(String.format("%s must be larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize)); @@ -589,7 +595,7 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy String levelFanoutSize = options.containsKey(LEVEL_FANOUT_SIZE_OPTION) ? options.get(LEVEL_FANOUT_SIZE_OPTION) : String.valueOf(DEFAULT_LEVEL_FANOUT_SIZE); try { - int fanoutSize = Integer.parseInt(levelFanoutSize); + fanoutSize = Integer.parseInt(levelFanoutSize); if (fanoutSize < 1) { throw new ConfigurationException(String.format("%s must be larger than 0, but was %s", LEVEL_FANOUT_SIZE_OPTION, fanoutSize)); @@ -597,7 +603,23 @@ public class LeveledCompactionStrategy extends AbstractCompactionStrategy } catch (NumberFormatException ex) { - throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", size, LEVEL_FANOUT_SIZE_OPTION), ex); + throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", levelFanoutSize, LEVEL_FANOUT_SIZE_OPTION), ex); + } + + // Validate max Bytes for a level + try + { + long maxSSTableSizeInBytes = Math.multiplyExact(ssSize, 1024L * 1024L); // Convert MB to Bytes + BigInteger fanoutPower = BigInteger.valueOf(fanoutSize).pow(MAX_LEVEL_COUNT - 1); + BigInteger maxBytes = fanoutPower.multiply(BigInteger.valueOf(maxSSTableSizeInBytes)); + BigInteger longMaxValue = BigInteger.valueOf(Long.MAX_VALUE); + if (maxBytes.compareTo(longMaxValue) > 0) + throw new ConfigurationException(String.format("At most %s bytes may be in a compaction level; " + + "your maxSSTableSize must be absurdly high to compute %s", Long.MAX_VALUE, maxBytes)); + } + catch (ArithmeticException ex) + { + throw new ConfigurationException(String.format("sstable_size_in_mb=%d is too large; resulting bytes exceed Long.MAX_VALUE (%d)", ssSize, Long.MAX_VALUE), ex); } uncheckedOptions.remove(LEVEL_FANOUT_SIZE_OPTION); diff --git a/test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java b/test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java index 5951f2cabc..e000b9a94d 100644 --- a/test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java @@ -975,6 +975,24 @@ public class LeveledCompactionStrategyTest } } + @Test() + public void testInvalidFanoutAndSSTableSize() + { + try + { + Map options = new HashMap<>(); + options.put("class", "LeveledCompactionStrategy"); + options.put("fanout_size", "90"); + options.put("sstable_size_in_mb", "1089"); + LeveledCompactionStrategy.validateOptions(options); + Assert.fail("fanout_sizeed and sstable_size_in_mb are invalid, but did not throw ConfigurationException"); + } + catch (ConfigurationException e) + { + assertTrue(e.getMessage().contains("your maxSSTableSize must be absurdly high to compute")); + } + } + @Test public void testReduceScopeL0() { diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index 63d0f96381..0688b4a468 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -134,6 +134,13 @@ public class CreateTableValidationTest extends CQLTester String.format("CREATE TABLE %s.\" \" (key int PRIMARY KEY, val int)", KEYSPACE)); } + @Test + public void testInvalidCompactionOptions() + { + expectedFailure(ConfigurationException.class, "CREATE TABLE %s (k int PRIMARY KEY, v int) WITH compaction = {'class': 'LeveledCompactionStrategy', 'fanout_size': '90', 'sstable_size_in_mb': '1089'}", + "your maxSSTableSize must be absurdly high to compute"); + } + private void expectedFailure(final Class exceptionType, String statement, String errorMsg) { diff --git a/test/unit/org/apache/cassandra/utils/CassandraGenerators.java b/test/unit/org/apache/cassandra/utils/CassandraGenerators.java index b428e9acb9..edfbe88163 100644 --- a/test/unit/org/apache/cassandra/utils/CassandraGenerators.java +++ b/test/unit/org/apache/cassandra/utils/CassandraGenerators.java @@ -52,7 +52,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import accord.utils.SortedArrays.SortedArrayList; -import org.apache.cassandra.db.compaction.LeveledManifest; import org.apache.cassandra.schema.*; import org.apache.cassandra.service.consensus.migration.ConsensusMigrationState; import org.apache.cassandra.tcm.extensions.ExtensionKey; @@ -143,6 +142,7 @@ import static org.apache.cassandra.utils.Generators.SMALL_TIME_SPAN_NANOS; import static org.apache.cassandra.utils.Generators.TIMESTAMP_NANOS; import static org.apache.cassandra.utils.Generators.TINY_TIME_SPAN_NANOS; import static org.apache.cassandra.utils.Generators.directAndHeapBytes; +import static org.junit.Assert.assertTrue; public final class CassandraGenerators { @@ -585,11 +585,14 @@ public final class CassandraGenerators try { // see org.apache.cassandra.db.compaction.LeveledGenerations.MAX_LEVEL_COUNT for why 8 is hard coded here - LeveledManifest.maxBytesForLevel(8, value, maxSSTableSizeInBytes); + // LeveledManifest.maxBytesForLevel(8, value, maxSSTableSizeInBytes); + options.put(LeveledCompactionStrategy.LEVEL_FANOUT_SIZE_OPTION, value.toString()); + LeveledCompactionStrategy.validateOptions(options); break; // value is good, keep it } - catch (RuntimeException e) + catch (ConfigurationException e) { + assertTrue(e.getMessage().contains("your maxSSTableSize must be absurdly high to compute")); // this value is too large... lets shrink it if (value.intValue() == 1) throw new AssertionError("There is no possible fanout size that works with maxSSTableSizeInMB=" + maxSSTableSizeInMB);