Skip to content

Commit 99f33bc

Browse files
nikhilkumawat03nikhilkumawat03
committed
CASSANDRA-20570: Fixed Leveled Compaction doesn't validate maxBytesForLevel when the table is altered/created
1 parent 88ca4f8 commit 99f33bc

File tree

3 files changed

+65
-16
lines changed

3 files changed

+65
-16
lines changed

‎src/java/org/apache/cassandra/db/compaction/LeveledCompactionStrategy.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.cassandra.db.compaction;
1919

2020
import java.util.*;
21+
import java.math.BigInteger;
2122

2223

2324
import com.google.common.annotations.VisibleForTesting;
@@ -43,6 +44,8 @@
4344
import org.apache.cassandra.io.sstable.ISSTableScanner;
4445
import org.apache.cassandra.io.sstable.format.SSTableReader;
4546

47+
import static org.apache.cassandra.db.compaction.LeveledGenerations.MAX_LEVEL_COUNT;
48+
4649
public class LeveledCompactionStrategy extends AbstractCompactionStrategy
4750
{
4851
private static final Logger logger = LoggerFactory.getLogger(LeveledCompactionStrategy.class);
@@ -565,10 +568,14 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
565568
{
566569
Map<String, String> uncheckedOptions = AbstractCompactionStrategy.validateOptions(options);
567570

571+
int ssSize;
572+
int fanoutSize;
573+
574+
// Validate the sstable_size option
568575
String size = options.containsKey(SSTABLE_SIZE_OPTION) ? options.get(SSTABLE_SIZE_OPTION) : "1";
569576
try
570577
{
571-
int ssSize = Integer.parseInt(size);
578+
ssSize = Integer.parseInt(size);
572579
if (ssSize < 1)
573580
{
574581
throw new ConfigurationException(String.format("%s must be larger than 0, but was %s", SSTABLE_SIZE_OPTION, ssSize));
@@ -585,15 +592,31 @@ public static Map<String, String> validateOptions(Map<String, String> options) t
585592
String levelFanoutSize = options.containsKey(LEVEL_FANOUT_SIZE_OPTION) ? options.get(LEVEL_FANOUT_SIZE_OPTION) : String.valueOf(DEFAULT_LEVEL_FANOUT_SIZE);
586593
try
587594
{
588-
int fanoutSize = Integer.parseInt(levelFanoutSize);
595+
fanoutSize = Integer.parseInt(levelFanoutSize);
589596
if (fanoutSize < 1)
590597
{
591598
throw new ConfigurationException(String.format("%s must be larger than 0, but was %s", LEVEL_FANOUT_SIZE_OPTION, fanoutSize));
592599
}
593600
}
594601
catch (NumberFormatException ex)
595602
{
596-
throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", size, LEVEL_FANOUT_SIZE_OPTION), ex);
603+
throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", levelFanoutSize, LEVEL_FANOUT_SIZE_OPTION), ex);
604+
}
605+
606+
// Validate max Bytes for a level
607+
try
608+
{
609+
long maxSSTableSizeInBytes = Math.multiplyExact(ssSize, 1024L * 1024L); // Convert MB to Bytes
610+
BigInteger fanoutPower = BigInteger.valueOf(fanoutSize).pow(MAX_LEVEL_COUNT);
611+
BigInteger maxBytes = fanoutPower.multiply(BigInteger.valueOf(maxSSTableSizeInBytes));
612+
BigInteger longMaxValue = BigInteger.valueOf(Long.MAX_VALUE);
613+
if (maxBytes.compareTo(longMaxValue) > 0)
614+
throw new ConfigurationException(String.format("At most %s bytes may be in a compaction level; " +
615+
"your maxSSTableSize must be absurdly high to compute %s", Long.MAX_VALUE, maxBytes));
616+
}
617+
catch (ArithmeticException ex)
618+
{
619+
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);
597620
}
598621

599622
uncheckedOptions.remove(LEVEL_FANOUT_SIZE_OPTION);

‎test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,24 @@ public void testReduceScopeL0L1() throws IOException
955955
}
956956
}
957957

958+
@Test()
959+
public void testInvalidFanoutAndSSTableSize()
960+
{
961+
try
962+
{
963+
Map<String, String> options = new HashMap<>();
964+
options.put("class", "LeveledCompactionStrategy");
965+
options.put("fanout_size", "90");
966+
options.put("sstable_size_in_mb", "1089");
967+
LeveledCompactionStrategy.validateOptions(options);
968+
Assert.fail("fanout_sizeed and sstable_size_in_mb are invalid, but did not throw ConfigurationException");
969+
}
970+
catch (ConfigurationException e)
971+
{
972+
assertTrue(e.getMessage().contains("your maxSSTableSize must be absurdly high to compute"));
973+
}
974+
}
975+
958976
@Test
959977
public void testReduceScopeL0()
960978
{

‎test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.cassandra.cql3.QueryOptions;
2727
import org.apache.cassandra.exceptions.ConfigurationException;
2828
import org.apache.cassandra.exceptions.InvalidRequestException;
29+
import org.apache.cassandra.exceptions.RequestValidationException;
2930
import org.apache.cassandra.transport.Message;
3031
import org.apache.cassandra.transport.ProtocolVersion;
3132
import org.apache.cassandra.transport.SimpleClient;
@@ -123,44 +124,51 @@ public void testCreateTableOnAllClusteringColumns()
123124
public void testCreateTableErrorOnNonClusteringKey()
124125
{
125126
String expectedMessage = "Only clustering key columns can be defined in CLUSTERING ORDER directive";
126-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, v ASC);",
127+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, v ASC);",
127128
expectedMessage+": [v]");
128-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC);",
129+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC);",
129130
expectedMessage+": [v]");
130-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC);",
131+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC);",
131132
expectedMessage+": [pk]");
132-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC, ck1 DESC);",
133+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk ASC, ck1 DESC);",
133134
expectedMessage+": [pk]");
134-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, pk DESC);",
135+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, ck2 DESC, pk DESC);",
135136
expectedMessage+": [pk]");
136-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC);",
137+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC);",
137138
expectedMessage+": [pk, v]");
138-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC, ck1 DESC);",
139+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (pk DESC, v DESC, ck1 DESC);",
139140
expectedMessage+": [pk, v]");
140-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, v ASC);",
141+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck1 ASC, v ASC);",
141142
expectedMessage+": [v]");
142-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC, ck1 DESC);",
143+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (v ASC, ck1 DESC);",
143144
expectedMessage+": [v]");
144145
}
145146

146147
@Test
147148
public void testCreateTableInWrongOrder()
148149
{
149-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC, ck1 DESC);",
150+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC, ck1 DESC);",
150151
"The order of columns in the CLUSTERING ORDER directive must match that of the clustering columns");
151152
}
152153

153154
@Test
154155
public void testCreateTableWithMissingClusteringColumn()
155156
{
156-
expectedFailure("CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC);",
157+
expectedFailure(InvalidRequestException.class, "CREATE TABLE %s (pk int, ck1 int, ck2 int, v int, PRIMARY KEY ((pk),ck1, ck2)) WITH CLUSTERING ORDER BY (ck2 ASC);",
157158
"Missing CLUSTERING ORDER for column ck1");
158159
}
159160

160-
private void expectedFailure(String statement, String errorMsg)
161+
@Test
162+
public void testInvalidCompactionOptions()
163+
{
164+
expectedFailure(ConfigurationException.class, "CREATE TABLE %s (k int PRIMARY KEY, v int) WITH compaction = {'class': 'LeveledCompactionStrategy', 'fanout_size': '90', 'sstable_size_in_mb': '1089'}",
165+
"your maxSSTableSize must be absurdly high to compute");
166+
}
167+
168+
private void expectedFailure(final Class<? extends RequestValidationException> exceptionType, String statement, String errorMsg)
161169
{
162170

163-
assertThatExceptionOfType(InvalidRequestException.class)
171+
assertThatExceptionOfType(exceptionType)
164172
.isThrownBy(() -> createTableMayThrow(statement)) .withMessageContaining(errorMsg);
165173
}
166174
}

0 commit comments

Comments
 (0)