From 3233c823116343cd95381790d736e239d800035a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20de=20la=20Pe=C3=B1a?= Date: Tue, 8 Mar 2022 11:08:29 +0000 Subject: [PATCH] Add guardrail for SELECT IN terms and their cartesian product MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Andrés de la Peña; reviewed by Ekaterina Dimitrova for CASSANDRA-17187 Co-authored-by: Aleksandr Sorokoumov Co-authored-by: Andrés de la Peña --- CHANGES.txt | 1 + conf/cassandra.yaml | 5 + .../org/apache/cassandra/config/Config.java | 3 + .../cassandra/config/GuardrailsOptions.java | 26 +++ .../ClusteringColumnRestrictions.java | 8 +- .../PartitionKeyRestrictions.java | 3 +- .../PartitionKeySingleRestrictionSet.java | 8 +- .../restrictions/StatementRestrictions.java | 11 +- .../cql3/restrictions/TokenFilter.java | 10 +- .../cql3/restrictions/TokenRestriction.java | 10 +- .../cql3/statements/BatchStatement.java | 6 +- .../statements/ModificationStatement.java | 17 +- .../cql3/statements/SelectStatement.java | 46 ++-- .../apache/cassandra/db/MultiCBuilder.java | 25 ++- .../cassandra/db/guardrails/Guardrails.java | 32 +++ .../db/guardrails/GuardrailsConfig.java | 12 + .../db/guardrails/GuardrailsMBean.java | 27 ++- .../io/sstable/CQLSSTableWriter.java | 5 +- .../GuardrailColumnsPerTableTest.java | 14 +- ...GuardrailInSelectCartesianProductTest.java | 209 ++++++++++++++++++ .../db/guardrails/GuardrailKeyspacesTest.java | 14 +- .../GuardrailPartitionKeysInSelectTest.java | 13 +- ...railReadBeforeWriteListOperationsTest.java | 2 +- .../GuardrailSecondaryIndexesPerTable.java | 19 +- .../db/guardrails/GuardrailTablesTest.java | 10 +- .../db/guardrails/GuardrailTester.java | 50 +++-- .../GuardrailUserTimestampsTest.java | 2 +- .../GuardrailViewsPerTableTest.java | 14 +- .../db/guardrails/ThresholdTester.java | 8 +- .../io/sstable/StressCQLSSTableWriter.java | 5 +- 30 files changed, 496 insertions(+), 119 deletions(-) create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 92c14eeba81f..b2cbe0f80b8d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add guardrail for SELECT IN terms and their cartesian product (CASSANDRA-17187) * remove unused imports in cqlsh.py and cqlshlib (CASSANDRA-17413) * deprecate property windows_timer_interval (CASSANDRA-17404) * Expose streaming as a vtable (CASSANDRA-17390) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index eecb4533f88c..ef03795526d5 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1620,6 +1620,11 @@ drop_compact_storage_enabled: false # The two thresholds default to -1 to disable. # partition_keys_in_select_warn_threshold: -1 # partition_keys_in_select_fail_threshold: -1 +# Guardrail to warn or fail when an IN query creates a cartesian product with a size exceeding threshold, +# eg. "a in (1,2,...10) and b in (1,2...10)" results in cartesian product of 100. +# The two thresholds default to -1 to disable. +# in_select_cartesian_product_warn_threshold: -1 +# in_select_cartesian_product_fail_threshold: -1 # Startup Checks are executed as part of Cassandra startup process, not all of them # are configurable (so you can disable them) but these which are enumerated bellow. diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 9dd164292509..09da88c810e2 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -767,10 +767,13 @@ public static void setClientMode(boolean clientMode) public volatile int page_size_fail_threshold = DISABLED_GUARDRAIL; public volatile int partition_keys_in_select_warn_threshold = DISABLED_GUARDRAIL; public volatile int partition_keys_in_select_fail_threshold = DISABLED_GUARDRAIL; + public volatile int in_select_cartesian_product_warn_threshold = DISABLED_GUARDRAIL; + public volatile int in_select_cartesian_product_fail_threshold = DISABLED_GUARDRAIL; public volatile Set table_properties_ignored = Collections.emptySet(); public volatile Set table_properties_disallowed = Collections.emptySet(); public volatile boolean user_timestamps_enabled = true; public volatile boolean read_before_write_list_operations_enabled = true; + public volatile DurationSpec streaming_state_expires = DurationSpec.inDays(3); public volatile DataStorageSpec streaming_state_size = DataStorageSpec.inMebibytes(40); diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index a36aa49714a1..d4de0f2c4a6e 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -69,6 +69,7 @@ public GuardrailsOptions(Config config) validateIntThreshold(config.page_size_warn_threshold, config.page_size_fail_threshold, "page_size"); validateIntThreshold(config.partition_keys_in_select_warn_threshold, config.partition_keys_in_select_fail_threshold, "partition_keys_in_select"); + validateIntThreshold(config.in_select_cartesian_product_warn_threshold, config.in_select_cartesian_product_fail_threshold, "in_select_cartesian_product"); } @Override @@ -321,6 +322,31 @@ public void setReadBeforeWriteListOperationsEnabled(boolean enabled) x -> config.read_before_write_list_operations_enabled = x); } + @Override + public int getInSelectCartesianProductWarnThreshold() + { + return config.in_select_cartesian_product_warn_threshold; + } + + @Override + public int getInSelectCartesianProductFailThreshold() + { + return config.in_select_cartesian_product_fail_threshold; + } + + public void setInSelectCartesianProductThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "in_select_cartesian_product"); + updatePropertyWithLogging("in_select_cartesian_product_warn_threshold", + warn, + () -> config.in_select_cartesian_product_warn_threshold, + x -> config.in_select_cartesian_product_warn_threshold = x); + updatePropertyWithLogging("in_select_cartesian_product_fail_threshold", + fail, + () -> config.in_select_cartesian_product_fail_threshold, + x -> config.in_select_cartesian_product_fail_threshold = x); + } + private static void updatePropertyWithLogging(String propertyName, T newValue, Supplier getter, Consumer setter) { T oldValue = getter.get(); diff --git a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java index 0a252ff557f0..bcf080ed5cb8 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java @@ -19,6 +19,7 @@ import java.util.*; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.cql3.QueryOptions; @@ -27,6 +28,7 @@ import org.apache.cassandra.db.filter.RowFilter; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.IndexRegistry; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.btree.BTreeSet; import static org.apache.cassandra.cql3.statements.RequestValidations.checkFalse; @@ -101,12 +103,16 @@ private boolean hasMultiColumnSlice() return false; } - public NavigableSet> valuesAsClustering(QueryOptions options) throws InvalidRequestException + public NavigableSet> valuesAsClustering(QueryOptions options, ClientState state) throws InvalidRequestException { MultiCBuilder builder = MultiCBuilder.create(comparator, hasIN()); for (SingleRestriction r : restrictions) { r.appendTo(builder, options); + + if (hasIN() && Guardrails.inSelectCartesianProduct.enabled(state)) + Guardrails.inSelectCartesianProduct.guard(builder.buildSize(), "clustering key", state); + if (builder.hasMissingElements()) break; } diff --git a/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeyRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeyRestrictions.java index b1edf947fa75..822452979ebf 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeyRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeyRestrictions.java @@ -23,6 +23,7 @@ import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.statements.Bound; +import org.apache.cassandra.service.ClientState; /** * A set of restrictions on the partition key. @@ -32,7 +33,7 @@ interface PartitionKeyRestrictions extends Restrictions { public PartitionKeyRestrictions mergeWith(Restriction restriction); - public List values(QueryOptions options); + public List values(QueryOptions options, ClientState state); public List bounds(Bound b, QueryOptions options); diff --git a/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeySingleRestrictionSet.java b/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeySingleRestrictionSet.java index fbe5673c05ba..a137175bd7ce 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeySingleRestrictionSet.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/PartitionKeySingleRestrictionSet.java @@ -20,6 +20,7 @@ import java.nio.ByteBuffer; import java.util.*; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.statements.Bound; @@ -28,6 +29,7 @@ import org.apache.cassandra.db.MultiCBuilder; import org.apache.cassandra.db.filter.RowFilter; import org.apache.cassandra.index.IndexRegistry; +import org.apache.cassandra.service.ClientState; /** * A set of single restrictions on the partition key. @@ -78,12 +80,16 @@ public PartitionKeyRestrictions mergeWith(Restriction restriction) } @Override - public List values(QueryOptions options) + public List values(QueryOptions options, ClientState state) { MultiCBuilder builder = MultiCBuilder.create(comparator, hasIN()); for (SingleRestriction r : restrictions) { r.appendTo(builder, options); + + if (hasIN() && Guardrails.inSelectCartesianProduct.enabled(state)) + Guardrails.inSelectCartesianProduct.guard(builder.buildSize(), "partition key", state); + if (builder.hasMissingElements()) break; } diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index b61cb4a30bde..9f87c93f710e 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -35,6 +35,7 @@ import org.apache.cassandra.index.IndexRegistry; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.utils.btree.BTreeSet; import org.apache.commons.lang3.builder.ToStringBuilder; @@ -619,11 +620,12 @@ public RowFilter getRowFilter(IndexRegistry indexRegistry, QueryOptions options) * Returns the partition keys for which the data is requested. * * @param options the query options + * @param state the client state * @return the partition keys for which the data is requested. */ - public List getPartitionKeys(final QueryOptions options) + public List getPartitionKeys(final QueryOptions options, ClientState state) { - return partitionKeyRestrictions.values(options); + return partitionKeyRestrictions.values(options, state); } /** @@ -741,9 +743,10 @@ public boolean hasClusteringColumnsRestrictions() * Returns the requested clustering columns. * * @param options the query options + * @param state the client state * @return the requested clustering columns */ - public NavigableSet> getClusteringColumns(QueryOptions options) + public NavigableSet> getClusteringColumns(QueryOptions options, ClientState state) { // If this is a names command and the table is a static compact one, then as far as CQL is concerned we have // only a single row which internally correspond to the static parts. In which case we want to return an empty @@ -751,7 +754,7 @@ public NavigableSet> getClusteringColumns(QueryOptions options) if (table.isStaticCompactTable()) return BTreeSet.empty(table.comparator); - return clusteringColumnsRestrictions.valuesAsClustering(options); + return clusteringColumnsRestrictions.valuesAsClustering(options, state); } /** diff --git a/src/java/org/apache/cassandra/cql3/restrictions/TokenFilter.java b/src/java/org/apache/cassandra/cql3/restrictions/TokenFilter.java index 437b17c617ab..9f67cc054750 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/TokenFilter.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/TokenFilter.java @@ -35,6 +35,7 @@ import org.apache.cassandra.dht.Token; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.IndexRegistry; +import org.apache.cassandra.service.ClientState; import static org.apache.cassandra.cql3.statements.Bound.END; import static org.apache.cassandra.cql3.statements.Bound.START; @@ -102,9 +103,9 @@ public TokenFilter(PartitionKeyRestrictions restrictions, TokenRestriction token } @Override - public List values(QueryOptions options) throws InvalidRequestException + public List values(QueryOptions options, ClientState state) throws InvalidRequestException { - return filter(restrictions.values(options), options); + return filter(restrictions.values(options, state), options, state); } @Override @@ -139,13 +140,14 @@ public List bounds(Bound bound, QueryOptions options) throws Invalid * * @param values the values returned by the decorated restriction * @param options the query options + * @param state the client state * @return the values matching the token restriction * @throws InvalidRequestException if the request is invalid */ - private List filter(List values, QueryOptions options) throws InvalidRequestException + private List filter(List values, QueryOptions options, ClientState state) throws InvalidRequestException { RangeSet rangeSet = tokenRestriction.hasSlice() ? toRangeSet(tokenRestriction, options) - : toRangeSet(tokenRestriction.values(options)); + : toRangeSet(tokenRestriction.values(options, state)); return filterWithRangeSet(rangeSet, values); } diff --git a/src/java/org/apache/cassandra/cql3/restrictions/TokenRestriction.java b/src/java/org/apache/cassandra/cql3/restrictions/TokenRestriction.java index e71b17782d15..d7477fbff9a3 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/TokenRestriction.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/TokenRestriction.java @@ -31,6 +31,7 @@ import org.apache.cassandra.db.filter.RowFilter; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.IndexRegistry; +import org.apache.cassandra.service.ClientState; import static org.apache.cassandra.cql3.statements.RequestValidations.invalidRequest; @@ -205,7 +206,10 @@ protected PartitionKeyRestrictions doMergeWith(TokenRestriction otherRestriction @Override public List bounds(Bound b, QueryOptions options) throws InvalidRequestException { - return values(options); + // ClientState is used by inSelectCartesianProduct guardrail to skip non-ordinary users. + // Passing null here to avoid polluting too many methods, because in case of EQ token restriction, + // it won't generate high cartesian product. + return values(options, null); } @Override @@ -221,7 +225,7 @@ public boolean isInclusive(Bound b) } @Override - public List values(QueryOptions options) throws InvalidRequestException + public List values(QueryOptions options, ClientState state) throws InvalidRequestException { return Collections.singletonList(value.bindAndGet(options)); } @@ -254,7 +258,7 @@ public boolean hasSlice() } @Override - public List values(QueryOptions options) throws InvalidRequestException + public List values(QueryOptions options, ClientState state) throws InvalidRequestException { throw new UnsupportedOperationException(); } diff --git a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java index 356e34706210..4f537ad910d2 100644 --- a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java @@ -282,7 +282,7 @@ public List getMutations(ClientState state, ModificationStatement stmt = statements.get(i); if (metadata != null && !stmt.metadata.id.equals(metadata.id)) metadata = null; - List stmtPartitionKeys = stmt.buildPartitionKeyNames(options.forStatement(i)); + List stmtPartitionKeys = stmt.buildPartitionKeyNames(options.forStatement(i), state); partitionKeys.add(stmtPartitionKeys); HashMultiset perKeyCountsForTable = partitionCounts.computeIfAbsent(stmt.metadata.id, k -> HashMultiset.create()); for (int stmtIdx = 0, stmtSize = stmtPartitionKeys.size(); stmtIdx < stmtSize; stmtIdx++) @@ -489,7 +489,7 @@ private Pair> makeCasRequest(BatchQueryOption ModificationStatement statement = statements.get(i); QueryOptions statementOptions = options.forStatement(i); long timestamp = attrs.getTimestamp(batchTimestamp, statementOptions); - List pks = statement.buildPartitionKeyNames(statementOptions); + List pks = statement.buildPartitionKeyNames(statementOptions, state.getClientState()); if (statement.getRestrictions().keyIsInRelation()) throw new IllegalArgumentException("Batch with conditions cannot span multiple partitions (you cannot use IN on the partition key)"); if (key == null) @@ -524,7 +524,7 @@ else if (!key.getKey().equals(pks.get(0))) } else { - Clustering clustering = Iterables.getOnlyElement(statement.createClustering(statementOptions)); + Clustering clustering = Iterables.getOnlyElement(statement.createClustering(statementOptions, state.getClientState())); if (statement.hasConditions()) { statement.addConditions(clustering, casRequest, statementOptions); diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index b6d274a9be6b..e5b99a9be8ef 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -330,23 +330,23 @@ public boolean hasIfExistCondition() return conditions.isIfExists(); } - public List buildPartitionKeyNames(QueryOptions options) + public List buildPartitionKeyNames(QueryOptions options, ClientState state) throws InvalidRequestException { - List partitionKeys = restrictions.getPartitionKeys(options); + List partitionKeys = restrictions.getPartitionKeys(options, state); for (ByteBuffer key : partitionKeys) QueryProcessor.validateKey(key); return partitionKeys; } - public NavigableSet> createClustering(QueryOptions options) + public NavigableSet> createClustering(QueryOptions options, ClientState state) throws InvalidRequestException { if (appliesOnlyToStaticColumns() && !restrictions.hasClusteringColumnsRestrictions()) return FBUtilities.singleton(CBuilder.STATIC_BUILDER.build(), metadata().comparator); - return restrictions.getClusteringColumns(options); + return restrictions.getClusteringColumns(options, state); } /** @@ -508,7 +508,8 @@ private ResultMessage executeWithCondition(QueryState queryState, QueryOptions o private CQL3CasRequest makeCasRequest(QueryState queryState, QueryOptions options) { - List keys = buildPartitionKeyNames(options); + ClientState clientState = queryState.getClientState(); + List keys = buildPartitionKeyNames(options, clientState); // We don't support IN for CAS operation so far checkFalse(restrictions.keyIsInRelation(), "IN on the partition key is not supported with conditional %s", @@ -522,7 +523,7 @@ private CQL3CasRequest makeCasRequest(QueryState queryState, QueryOptions option "IN on the clustering key columns is not supported with conditional %s", type.isUpdate()? "updates" : "deletions"); - Clustering clustering = Iterables.getOnlyElement(createClustering(options)); + Clustering clustering = Iterables.getOnlyElement(createClustering(options, clientState)); CQL3CasRequest request = new CQL3CasRequest(metadata(), key, conditionColumns(), updatesRegularRows(), updatesStaticRow()); addConditions(clustering, request, options); @@ -695,7 +696,7 @@ private List getMutations(ClientState state, int nowInSeconds, long queryStartNanoTime) { - List keys = buildPartitionKeyNames(options); + List keys = buildPartitionKeyNames(options, state); HashMultiset perPartitionKeyCounts = HashMultiset.create(keys); SingleTableUpdatesCollector collector = new SingleTableUpdatesCollector(metadata, updatedColumns, perPartitionKeyCounts); addUpdates(collector, keys, state, options, local, timestamp, nowInSeconds, queryStartNanoTime); @@ -741,7 +742,7 @@ final void addUpdates(UpdatesCollector collector, } else { - NavigableSet> clusterings = createClustering(options); + NavigableSet> clusterings = createClustering(options, state); // If some of the restrictions were unspecified (e.g. empty IN restrictions) we do not need to do anything. if (restrictions.hasClusteringColumnsRestrictions() && clusterings.isEmpty()) diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index a1fd1e533ed6..4d6bf00b75b0 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -293,7 +293,7 @@ public ReadQuery getQuery(QueryOptions options, DataLimits limit = getDataLimits(userLimit, perPartitionLimit, pageSize); if (isPartitionRangeQuery) - return getRangeCommand(options, columnFilter, limit, nowInSec); + return getRangeCommand(options, state, columnFilter, limit, nowInSec); return getSliceCommands(options, state, columnFilter, limit, nowInSec); } @@ -528,7 +528,7 @@ public StatementRestrictions getRestrictions() private ReadQuery getSliceCommands(QueryOptions options, ClientState state, ColumnFilter columnFilter, DataLimits limit, int nowInSec) { - Collection keys = restrictions.getPartitionKeys(options); + Collection keys = restrictions.getPartitionKeys(options, state); if (keys.isEmpty()) return ReadQuery.empty(table); @@ -537,7 +537,7 @@ private ReadQuery getSliceCommands(QueryOptions options, ClientState state, Colu Guardrails.partitionKeysInSelect.guard(keys.size(), table.name, state); } - ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter filter = makeClusteringIndexFilter(options, state, columnFilter); if (filter == null || filter.isEmpty(table.comparator)) return ReadQuery.empty(table); @@ -564,8 +564,9 @@ private ReadQuery getSliceCommands(QueryOptions options, ClientState state, Colu public Slices clusteringIndexFilterAsSlices() { QueryOptions options = QueryOptions.forInternalCalls(Collections.emptyList()); + ClientState state = ClientState.forInternalCalls(); ColumnFilter columnFilter = selection.newSelectors(options).getColumnFilter(); - ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter filter = makeClusteringIndexFilter(options, state, columnFilter); if (filter instanceof ClusteringIndexSliceFilter) return ((ClusteringIndexSliceFilter)filter).requestedSlices(); @@ -582,8 +583,9 @@ public Slices clusteringIndexFilterAsSlices() public SinglePartitionReadCommand internalReadForView(DecoratedKey key, int nowInSec) { QueryOptions options = QueryOptions.forInternalCalls(Collections.emptyList()); + ClientState state = ClientState.forInternalCalls(); ColumnFilter columnFilter = selection.newSelectors(options).getColumnFilter(); - ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter filter = makeClusteringIndexFilter(options, state, columnFilter); RowFilter rowFilter = getRowFilter(options); return SinglePartitionReadCommand.create(table, nowInSec, columnFilter, rowFilter, DataLimits.NONE, key, filter); } @@ -596,9 +598,9 @@ public RowFilter rowFilterForInternalCalls() return getRowFilter(QueryOptions.forInternalCalls(Collections.emptyList())); } - private ReadQuery getRangeCommand(QueryOptions options, ColumnFilter columnFilter, DataLimits limit, int nowInSec) + private ReadQuery getRangeCommand(QueryOptions options, ClientState state, ColumnFilter columnFilter, DataLimits limit, int nowInSec) { - ClusteringIndexFilter clusteringIndexFilter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter clusteringIndexFilter = makeClusteringIndexFilter(options, state, columnFilter); if (clusteringIndexFilter == null) return ReadQuery.empty(table); @@ -619,7 +621,7 @@ private ReadQuery getRangeCommand(QueryOptions options, ColumnFilter columnFilte return command; } - private ClusteringIndexFilter makeClusteringIndexFilter(QueryOptions options, ColumnFilter columnFilter) + private ClusteringIndexFilter makeClusteringIndexFilter(QueryOptions options, ClientState state, ColumnFilter columnFilter) { if (parameters.isDistinct) { @@ -642,7 +644,7 @@ private ClusteringIndexFilter makeClusteringIndexFilter(QueryOptions options, Co return new ClusteringIndexSliceFilter(slices, isReversed); } - NavigableSet> clusterings = getRequestedRows(options); + NavigableSet> clusterings = getRequestedRows(options, state); // We can have no clusterings if either we're only selecting the static columns, or if we have // a 'IN ()' for clusterings. In that case, we still want to query if some static columns are // queried. But we're fine otherwise. @@ -774,12 +776,12 @@ private int getLimit(Term limit, QueryOptions options) return userLimit; } - private NavigableSet> getRequestedRows(QueryOptions options) throws InvalidRequestException + private NavigableSet> getRequestedRows(QueryOptions options, ClientState state) throws InvalidRequestException { // Note: getRequestedColumns don't handle static columns, but due to CASSANDRA-5762 // we always do a slice for CQL3 tables, so it's ok to ignore them here assert !restrictions.isColumnRange(); - return restrictions.getClusteringColumns(options); + return restrictions.getClusteringColumns(options, state); } /** @@ -841,8 +843,9 @@ private void maybeWarn(ResultSetBuilder result, QueryOptions options) if (result.shouldWarn(options.getCoordinatorReadSizeWarnThresholdKB())) { String msg = String.format("Read on table %s has exceeded the size warning threshold of %,d kb", table, options.getCoordinatorReadSizeWarnThresholdKB()); - ClientWarn.instance.warn(msg + " with " + loggableTokens(options)); - logger.warn("{} with query {}", msg, asCQL(options)); + ClientState state = ClientState.forInternalCalls(); + ClientWarn.instance.warn(msg + " with " + loggableTokens(options, state)); + logger.warn("{} with query {}", msg, asCQL(options, state)); if (store != null) store.metric.coordinatorReadSizeWarnings.mark(); } @@ -855,9 +858,10 @@ private void maybeFail(ResultSetBuilder result, QueryOptions options) if (result.shouldReject(options.getCoordinatorReadSizeAbortThresholdKB())) { String msg = String.format("Read on table %s has exceeded the size failure threshold of %,d kb", table, options.getCoordinatorReadSizeAbortThresholdKB()); - String clientMsg = msg + " with " + loggableTokens(options); + ClientState state = ClientState.forInternalCalls(); + String clientMsg = msg + " with " + loggableTokens(options, state); ClientWarn.instance.warn(clientMsg); - logger.warn("{} with query {}", msg, asCQL(options)); + logger.warn("{} with query {}", msg, asCQL(options, state)); ColumnFamilyStore store = cfs(); if (store != null) { @@ -1442,7 +1446,7 @@ public String toString() return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); } - private String loggableTokens(QueryOptions options) + private String loggableTokens(QueryOptions options, ClientState state) { if (restrictions.isKeyRange() || restrictions.usesSecondaryIndexing()) { @@ -1454,7 +1458,7 @@ private String loggableTokens(QueryOptions options) } else { - Collection keys = restrictions.getPartitionKeys(options); + Collection keys = restrictions.getPartitionKeys(options, state); if (keys.size() == 1) { return "token: " + table.partitioner.getToken(Iterables.getOnlyElement(keys)).toString(); @@ -1474,7 +1478,7 @@ private String loggableTokens(QueryOptions options) } } - private String asCQL(QueryOptions options) + private String asCQL(QueryOptions options, ClientState state) { ColumnFilter columnFilter = selection.newSelectors(options).getColumnFilter(); StringBuilder sb = new StringBuilder(); @@ -1484,7 +1488,7 @@ private String asCQL(QueryOptions options) if (restrictions.isKeyRange() || restrictions.usesSecondaryIndexing()) { // partition range - ClusteringIndexFilter clusteringIndexFilter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter clusteringIndexFilter = makeClusteringIndexFilter(options, state, columnFilter); if (clusteringIndexFilter == null) return "EMPTY"; @@ -1515,10 +1519,10 @@ private String asCQL(QueryOptions options) else { // single partition - Collection keys = restrictions.getPartitionKeys(options); + Collection keys = restrictions.getPartitionKeys(options, state); if (keys.isEmpty()) return "EMPTY"; - ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter); + ClusteringIndexFilter filter = makeClusteringIndexFilter(options, state, columnFilter); if (filter == null) return "EMPTY"; diff --git a/src/java/org/apache/cassandra/db/MultiCBuilder.java b/src/java/org/apache/cassandra/db/MultiCBuilder.java index 0b5625b26f2a..435e418eb3a7 100644 --- a/src/java/org/apache/cassandra/db/MultiCBuilder.java +++ b/src/java/org/apache/cassandra/db/MultiCBuilder.java @@ -129,6 +129,13 @@ public int remainingCount() return comparator.size() - size; } + /** + * Returns the current number of results when {@link #build()} is called + * + * @return the current number of build results + */ + public abstract int buildSize(); + /** * Checks if the clusterings contains null elements. * @@ -252,6 +259,12 @@ public MultiCBuilder addAllElementsToAll(List> values) return addEachElementToAll(values.get(0)); } + @Override + public int buildSize() + { + return hasMissingElements ? 0 : 1; + } + public NavigableSet> build() { built = true; @@ -309,7 +322,7 @@ public MultiCBuilder addElementToAll(ByteBuffer value) checkUpdateable(); if (elementsList.isEmpty()) - elementsList.add(new ArrayList()); + elementsList.add(new ArrayList<>()); if (value == null) containsNull = true; @@ -328,7 +341,7 @@ public MultiCBuilder addEachElementToAll(List values) checkUpdateable(); if (elementsList.isEmpty()) - elementsList.add(new ArrayList()); + elementsList.add(new ArrayList<>()); if (values.isEmpty()) { @@ -365,7 +378,7 @@ public MultiCBuilder addAllElementsToAll(List> values) checkUpdateable(); if (elementsList.isEmpty()) - elementsList.add(new ArrayList()); + elementsList.add(new ArrayList<>()); if (values.isEmpty()) { @@ -397,6 +410,12 @@ public MultiCBuilder addAllElementsToAll(List> values) return this; } + @Override + public int buildSize() + { + return hasMissingElements ? 0 : elementsList.size(); + } + public NavigableSet> build() { built = true; diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index ecf801793a63..fa310e346d95 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -148,6 +148,20 @@ public final class Guardrails implements GuardrailsMBean new DisableFlag(state -> !CONFIG_PROVIDER.getOrCreate(state).getReadBeforeWriteListOperationsEnabled(), "List operation requiring read before write"); + /** + * Guardrail on the number of restrictions created by a cartesian product of a CQL's {@code IN} query. + */ + public static final Threshold inSelectCartesianProduct = + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("The cartesian product of the IN restrictions on %s produces %d values, " + + "this exceeds warning threshold of %s.", + what, value, threshold) + : format("Aborting query because the cartesian product of the IN restrictions on %s " + + "produces %d values, this exceeds fail threshold of %s.", + what, value, threshold)); + private Guardrails() { MBeanWrapper.instance.registerMBean(this, MBEAN_NAME); @@ -383,6 +397,24 @@ public int getPartitionKeysInSelectFailThreshold() return DEFAULT_CONFIG.getPartitionKeysInSelectFailThreshold(); } + @Override + public int getInSelectCartesianProductWarnThreshold() + { + return DEFAULT_CONFIG.getInSelectCartesianProductWarnThreshold(); + } + + @Override + public int getInSelectCartesianProductFailThreshold() + { + return DEFAULT_CONFIG.getInSelectCartesianProductFailThreshold(); + } + + @Override + public void setInSelectCartesianProductThreshold(int warn, int fail) + { + DEFAULT_CONFIG.setInSelectCartesianProductThreshold(warn, fail); + } + private static String toCSV(Set values) { return values == null ? "" : String.join(",", values); diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index dd30e4519b3b..2586436a0a10 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -144,4 +144,16 @@ public interface GuardrailsConfig * @return {@code true} if list operations that require read before write are allowed, {@code false} otherwise. */ boolean getReadBeforeWriteListOperationsEnabled(); + + /** + * @return The threshold to warn when an IN query creates a cartesian product with a size exceeding threshold. + * -1 means disabled. + */ + public int getInSelectCartesianProductWarnThreshold(); + + /** + * @return The threshold to prevent IN queries creating a cartesian product with a size exceeding threshold. + * -1 means disabled. + */ + public int getInSelectCartesianProductFailThreshold(); } diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 5becdec6ef99..b6ed551948c8 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -221,6 +221,17 @@ public interface GuardrailsMBean */ void setReadBeforeWriteListOperationsEnabled(boolean enabled); + /** + * @return The threshold to warn when the number of partition keys in a select statement greater than threshold. + * -1 means disabled. + */ + int getPartitionKeysInSelectWarnThreshold(); + + /** + * @return The threshold to fail when the number of partition keys in a select statement greater than threshold. + * -1 means disabled. + */ + int getPartitionKeysInSelectFailThreshold(); /** * @param warn The threshold to warn when the number of partition keys in a select statement is greater than @@ -231,14 +242,22 @@ public interface GuardrailsMBean void setPartitionKeysInSelectThreshold(int warn, int fail); /** - * @return The threshold to warn when the number of partition keys in a select statement greater than threshold. + * @return The threshold to warn when an IN query creates a cartesian product with a size exceeding threshold. * -1 means disabled. */ - int getPartitionKeysInSelectWarnThreshold(); + public int getInSelectCartesianProductWarnThreshold(); /** - * @return The threshold to fail when the number of partition keys in a select statement greater than threshold. + * @return The threshold to prevent IN queries creating a cartesian product with a size exceeding threshold. * -1 means disabled. */ - int getPartitionKeysInSelectFailThreshold(); + public int getInSelectCartesianProductFailThreshold(); + + /** + * @param warn The threshold to warn when an IN query creates a cartesian product with a size exceeding threshold. + * -1 means disabled. + * @param fail The threshold to prevent IN queries creating a cartesian product with a size exceeding threshold. + * -1 means disabled. + */ + public void setInSelectCartesianProductThreshold(int warn, int fail); } diff --git a/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java b/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java index 153f0d55d77f..6224cb7bfb8a 100644 --- a/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java @@ -239,8 +239,9 @@ public CQLSSTableWriter rawAddRow(List values) throw new InvalidRequestException(String.format("Invalid number of arguments, expecting %d values but got %d", boundNames.size(), values.size())); QueryOptions options = QueryOptions.forInternalCalls(null, values); - List keys = insert.buildPartitionKeyNames(options); - SortedSet> clusterings = insert.createClustering(options); + ClientState state = ClientState.forInternalCalls(); + List keys = insert.buildPartitionKeyNames(options, state); + SortedSet> clusterings = insert.createClustering(options, state); long now = currentTimeMillis(); // Note that we asks indexes to not validate values (the last 'false' arg below) because that triggers a 'Keyspace.open' diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java index 84f416fc5ea9..0c32f4914b88 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java @@ -160,11 +160,12 @@ private void assertAddColumnWarns(String query) throws Throwable private void assertWarns(long numColumns, String query, String tableName) throws Throwable { - assertThresholdWarns(format("The table %s has %s columns, this exceeds the warning threshold of %s.", + assertThresholdWarns(format(query, keyspace() + '.' + tableName), + format("The table %s has %s columns, this exceeds the warning threshold of %s.", tableName, numColumns, - guardrails().getColumnsPerTableWarnThreshold()), - format(query, keyspace() + '.' + tableName)); + guardrails().getColumnsPerTableWarnThreshold()) + ); } private void assertAddColumnFails(String query) throws Throwable @@ -179,10 +180,11 @@ private void assertCreateTableFails(long numColumns, String query) throws Throwa private void assertFails(long numColumns, String query, String tableName) throws Throwable { - assertThresholdFails(format("Tables cannot have more than %s columns, but %s provided for table %s", + assertThresholdFails(format(query, keyspace() + '.' + tableName), + format("Tables cannot have more than %s columns, but %s provided for table %s", guardrails().getColumnsPerTableFailThreshold(), numColumns, - tableName), - format(query, keyspace() + '.' + tableName)); + tableName) + ); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java new file mode 100644 index 000000000000..46829040d386 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import java.nio.ByteBuffer; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import org.junit.Before; +import org.junit.Test; + +import org.apache.cassandra.db.marshal.Int32Type; + +/** + * Tests the guardrail for the max number of restrictions produced by the cartesian product of the {@code IN} + * restrictions of a query, {@link Guardrails#inSelectCartesianProduct}. + */ +public class GuardrailInSelectCartesianProductTest extends ThresholdTester +{ + private static final int WARN_THRESHOLD = 16; + private static final int FAIL_THRESHOLD = 25; + + private static final String WARN_MESSAGE = "The cartesian product of the IN restrictions on %s produces %d " + + "values, this exceeds warning threshold of " + WARN_THRESHOLD; + private static final String FAIL_MESSAGE = "Aborting query because the cartesian product of the IN restrictions " + + "on %s produces %d values, this exceeds fail threshold of " + FAIL_THRESHOLD; + + public GuardrailInSelectCartesianProductTest() + { + super(WARN_THRESHOLD, + FAIL_THRESHOLD, + "in_select_cartesian_product", + Guardrails::setInSelectCartesianProductThreshold, + Guardrails::getInSelectCartesianProductWarnThreshold, + Guardrails::getInSelectCartesianProductFailThreshold); + } + + @Override + protected long currentValue() + { + throw new UnsupportedOperationException(); + } + + @Before + public void initSchema() + { + createTable("CREATE TABLE %s (pk1 int, pk2 int, ck1 int, ck2 int, PRIMARY KEY((pk1, pk2), ck1, ck2))"); + } + + @Test + public void testPkCartesianProduct() throws Throwable + { + // below both thresholds + testPkCartesianProduct(1, 1); + testPkCartesianProduct(1, 4); + testPkCartesianProduct(4, 4); + + // above warn threshold + testPkCartesianProduct(5, 5); + testPkCartesianProduct(2, 12); + testPkCartesianProduct(8, 3); + + // above cartesian product limit + testPkCartesianProduct(1, 26); + testPkCartesianProduct(5, 6); + testPkCartesianProduct(26, 1); + } + + @Test + public void testCkCartesianProduct() throws Throwable + { + // below both thresholds + testCkCartesianProduct(3, 8); + testCkCartesianProduct(5, 5); + + // above cartesian product limit + testCkCartesianProduct(1, 26); + testCkCartesianProduct(5, 6); + testCkCartesianProduct(6, 5); + testCkCartesianProduct(26, 1); + } + + @Test + public void testPkCkCartesianProduct() throws Throwable + { + // below both thresholds + testCartesianProduct(1, 10, 1, 10); + testCartesianProduct(10, 1, 10, 1); + testCartesianProduct(5, 5, 5, 5); + + // above cartesian product limit + testCartesianProduct(5, 6, 5, 5); + testCartesianProduct(6, 5, 5, 5); + testCartesianProduct(5, 5, 6, 5); + testCartesianProduct(5, 5, 5, 6); + } + + @Test + public void testExcludedUsers() throws Throwable + { + testExcludedUsers(() -> String.format("SELECT * FROM %%s WHERE pk1 in (%s) AND pk2 in (%s)", + terms(5), terms(5)), + () -> String.format("SELECT * FROM %%s WHERE pk1 in (%s) AND pk2 in (%s) AND ck1 in (%s) AND ck2 in (%s)", + terms(5), terms(5), terms(5), terms(6))); + } + + @Test + public void testPkCartesianProductMultiColumnBelowThreshold() throws Throwable + { + String inTerms = IntStream.range(0, 5).mapToObj(i -> String.format("(%d, %d)", i, i + 1)).collect(Collectors.joining(", ")); + String query = String.format("SELECT * FROM %%s WHERE (pk1, pk2) in (%s)", inTerms); + assertInvalidMessage("Multi-column relations can only be applied to clustering columns but was applied to: pk1", query); + } + + private void testPkCartesianProduct(int pk1Terms, int pk2Terms) throws Throwable + { + testCartesianProduct(pk1Terms, pk2Terms, 1, 1); + } + + private void testCkCartesianProduct(int ck1Terms, int ck2Terms) throws Throwable + { + testCartesianProduct(1, 1, ck1Terms, ck2Terms); + } + + private void testCartesianProduct(int pk1, int pk2, int ck1, int ck2) throws Throwable + { + int keys = pk1 * pk2; + int clusterings = ck1 * ck2; + + String query = String.format("SELECT * FROM %%s WHERE pk1 in (%s) AND pk2 in (%s) AND ck1 in (%s) AND ck2 in (%s)", + terms(pk1), terms(pk2), terms(ck1), terms(ck2)); + testCartesianProduct(() -> execute(userClientState, query), keys, clusterings); + + String queryWithBindVariables = String.format("SELECT * FROM %%s WHERE pk1 in (%s) AND pk2 in (%s) AND ck1 in (%s) AND ck2 in (%s)", + markers(pk1), markers(pk2), markers(ck1), markers(ck2)); + testCartesianProduct(() -> execute(userClientState, queryWithBindVariables, bindValues(pk1, pk2, ck1, ck2)), keys, clusterings); + } + + private void testCartesianProduct(CheckedFunction function, int keys, int clusterings) throws Throwable + { + String keysFailMessage = String.format(FAIL_MESSAGE, "partition key", keys); + String keysWarnMessage = String.format(WARN_MESSAGE, "partition key", keys); + String clusteringsFailMessage = String.format(FAIL_MESSAGE, "clustering key", clusterings); + String clusteringsWarnMessage = String.format(WARN_MESSAGE, "clustering key", clusterings); + + if (keys > FAIL_THRESHOLD) + { + assertFails(function, keysFailMessage); + } + else if (keys > WARN_THRESHOLD) + { + if (clusterings > FAIL_THRESHOLD) + assertFails(function, keysWarnMessage, clusteringsFailMessage); + else if (clusterings > WARN_THRESHOLD) + assertWarns(function, keysWarnMessage, clusteringsWarnMessage); + else + assertWarns(function, keysWarnMessage); + } + else if (clusterings > FAIL_THRESHOLD) + { + assertFails(function, clusteringsFailMessage); + } + else if (clusterings > WARN_THRESHOLD) + { + assertWarns(function, clusteringsWarnMessage); + } + else + { + assertValid(function); + } + } + + private static String terms(int terms) + { + assert terms > 0; + return IntStream.range(0, terms).mapToObj(String::valueOf).collect(Collectors.joining(", ")); + } + + private static String markers(int terms) + { + assert terms > 0; + return IntStream.range(0, terms).mapToObj(i -> "?").collect(Collectors.joining(", ")); + } + + private static List bindValues(int... termCounts) + { + return IntStream.of(termCounts) + .boxed() + .flatMap(terms -> IntStream.range(0, terms).boxed().map(Int32Type.instance::decompose)) + .collect(Collectors.toList()); + } +} diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java index c4ffec1598c0..2ccbad1278e4 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java @@ -89,18 +89,20 @@ private String assertCreateKeyspaceValid() throws Throwable private String assertCreateKeyspaceWarns() throws Throwable { String keyspaceName = createKeyspaceName(); - assertThresholdWarns(format("Creating keyspace %s, current number of keyspaces %d exceeds warning threshold of %d", - keyspaceName, currentValue() + 1, WARN_THRESHOLD), - createKeyspaceQuery(keyspaceName)); + assertThresholdWarns(createKeyspaceQuery(keyspaceName), + format("Creating keyspace %s, current number of keyspaces %d exceeds warning threshold of %d", + keyspaceName, currentValue() + 1, WARN_THRESHOLD) + ); return keyspaceName; } private void assertCreateKeyspaceFails() throws Throwable { String keyspaceName = createKeyspaceName(); - assertThresholdFails(format("Cannot have more than %d keyspaces, aborting the creation of keyspace %s", - FAIL_THRESHOLD, keyspaceName), - createKeyspaceQuery(keyspaceName)); + assertThresholdFails(createKeyspaceQuery(keyspaceName), + format("Cannot have more than %d keyspaces, aborting the creation of keyspace %s", + FAIL_THRESHOLD, keyspaceName) + ); } private String createKeyspaceQuery() diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java index 7a88fb48a30b..af0728ac91c8 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java @@ -52,14 +52,13 @@ public void testSelectStatementAgainstInClausePartitionKeys() throws Throwable assertValid("SELECT k, c, v FROM %s WHERE k = 2 and c IN (2, 3, 4, 5, 6, 7)"); - assertWarns(String.format("Query with partition keys in IN clause on table %s, with " + - "number of partition keys 4 exceeds warning threshold of 3.", tableName), - "SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5)"); + assertWarns("SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5)", + String.format("Query with partition keys in IN clause on table %s, with " + + "number of partition keys 4 exceeds warning threshold of 3.", tableName)); - assertFails(String.format("Aborting query with partition keys in IN clause on table %s, " + - "number of partition keys 6 exceeds fail threshold of 5.", tableName) , - "SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5, 6, 7)" - ); + assertFails("SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5, 6, 7)", + String.format("Aborting query with partition keys in IN clause on table %s, " + + "number of partition keys 6 exceeds fail threshold of 5.", tableName)); } @Test diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java index 80d1bef47dcd..98a7d09b188d 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java @@ -166,7 +166,7 @@ private void testGuardrail(String query, String expectedMessage, Object[]... row } else { - assertFails(expectedMessage, query); + assertFails(query, expectedMessage); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java index 7496bacb8af3..c7a3763b8186 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java @@ -97,24 +97,27 @@ private void assertCreateIndexSucceeds(String column, String indexName) throws T private void assertCreateIndexWarns(String column, String indexName) throws Throwable { - assertThresholdWarns(format("Creating secondary index %son table %s, current number of indexes %s exceeds warning threshold of %s.", + assertThresholdWarns(format("CREATE INDEX %s ON %%s(%s)", indexName, column), + format("Creating secondary index %son table %s, current number of indexes %s exceeds warning threshold of %s.", (Strings.isNullOrEmpty(indexName) ? "" : indexName + " "), currentTable(), currentValue() + 1, - guardrails().getSecondaryIndexesPerTableWarnThreshold()), - format("CREATE INDEX %s ON %%s(%s)", indexName, column)); + guardrails().getSecondaryIndexesPerTableWarnThreshold()) + ); } private void assertCreateIndexFails(String column, String indexName) throws Throwable { - assertThresholdFails(format("aborting the creation of secondary index %son table %s", - Strings.isNullOrEmpty(indexName) ? "" : indexName + " ", currentTable()), - format("CREATE INDEX %s ON %%s(%s)", indexName, column)); + assertThresholdFails(format("CREATE INDEX %s ON %%s(%s)", indexName, column), + format("aborting the creation of secondary index %son table %s", + Strings.isNullOrEmpty(indexName) ? "" : indexName + " ", currentTable()) + ); } private void assertCreateCustomIndexFails(String column) throws Throwable { - assertThresholdFails(format("aborting the creation of secondary index on table %s", currentTable()), - format("CREATE CUSTOM INDEX ON %%s (%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'", column)); + assertThresholdFails(format("CREATE CUSTOM INDEX ON %%s (%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'", column), + format("aborting the creation of secondary index on table %s", currentTable()) + ); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java index 83d0203dc207..c56ae0087bed 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java @@ -90,16 +90,18 @@ private String assertCreateTableValid() throws Throwable private String assertCreateTableWarns() throws Throwable { String tableName = createTableName(); - assertThresholdWarns(format("Creating table %s, current number of tables 2 exceeds warning threshold of 1", tableName), - createTableQuery(tableName)); + assertThresholdWarns(createTableQuery(tableName), + format("Creating table %s, current number of tables 2 exceeds warning threshold of 1", tableName) + ); return tableName; } private void assertCreateTableFails() throws Throwable { String tableName = createTableName(); - assertThresholdFails(format("Cannot have more than 2 tables, aborting the creation of table %s", tableName), - createTableQuery(tableName)); + assertThresholdFails(createTableQuery(tableName), + format("Cannot have more than 2 tables, aborting the creation of table %s", tableName) + ); } private String createTableQuery() diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java index 7e9ace716175..9e3a9f56f671 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java @@ -19,6 +19,7 @@ package org.apache.cassandra.db.guardrails; import java.net.InetSocketAddress; +import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; import java.util.function.BiConsumer; @@ -163,7 +164,7 @@ protected void assertValid(String query) throws Throwable assertValid(() -> execute(userClientState, query)); } - protected void assertWarns(CheckedFunction function, String message) throws Throwable + protected void assertWarns(CheckedFunction function, String... messages) throws Throwable { // We use client warnings to check we properly warn as this is the most convenient. Technically, // this doesn't validate we also log the warning, but that's probably fine ... @@ -171,7 +172,7 @@ protected void assertWarns(CheckedFunction function, String message) throws Thro try { function.apply(); - assertWarnings(message); + assertWarnings(messages); } finally { @@ -179,17 +180,17 @@ protected void assertWarns(CheckedFunction function, String message) throws Thro } } - protected void assertWarns(String message, String query) throws Throwable + protected void assertWarns(String query, String... messages) throws Throwable { - assertWarns(() -> execute(userClientState, query), message); + assertWarns(() -> execute(userClientState, query), messages); } - protected void assertFails(CheckedFunction function, String message) throws Throwable + protected void assertFails(CheckedFunction function, String... messages) throws Throwable { - assertFails(function, message, true); + assertFails(function, true, messages); } - protected void assertFails(CheckedFunction function, String message, boolean thrown) throws Throwable + protected void assertFails(CheckedFunction function, boolean thrown, String... messages) throws Throwable { ClientWarn.instance.captureWarnings(); try @@ -203,10 +204,12 @@ protected void assertFails(CheckedFunction function, String message, boolean thr { assertTrue("Expect no exception thrown", thrown); - assertTrue(format("Full error message '%s' does not contain expected message '%s'", e.getMessage(), message), - e.getMessage().contains(message)); + // the last message is the one raising the guardrail failure, the previous messages are warnings + String failMessage = messages[messages.length - 1]; + assertTrue(format("Full error message '%s' does not contain expected message '%s'", e.getMessage(), failMessage), + e.getMessage().contains(failMessage)); - assertWarnings(message); + assertWarnings(messages); } finally { @@ -214,23 +217,27 @@ protected void assertFails(CheckedFunction function, String message, boolean thr } } - protected void assertFails(String message, String query) throws Throwable + protected void assertFails(String query, String... messages) throws Throwable { - assertFails(() -> execute(userClientState, query), message); + assertFails(() -> execute(userClientState, query), messages); } - private void assertWarnings(String message) + private void assertWarnings(String... messages) { List warnings = getWarnings(); assertFalse("Expected to warn, but no warning was received", warnings == null || warnings.isEmpty()); - assertEquals(format("Got more thant 1 warning (got %d => %s)", warnings.size(), warnings), - 1, + assertEquals(format("Expected %d warnings but got %d: %s", messages.length, warnings.size(), warnings), + messages.length, warnings.size()); - String warning = warnings.get(0); - assertTrue(format("Warning log message '%s' does not contain expected message '%s'", warning, message), - warning.contains(message)); + for (int i = 0; i < messages.length; i++) + { + String message = messages[i]; + String warning = warnings.get(i); + assertTrue(format("Warning log message '%s' does not contain expected message '%s'", warning, message), + warning.contains(message)); + } } private void assertEmptyWarnings() @@ -270,6 +277,11 @@ protected void assertConfigFails(Consumer consumer, String message) } protected ResultMessage execute(ClientState state, String query) + { + return execute(state, query, Collections.emptyList()); + } + + protected ResultMessage execute(ClientState state, String query, List values) { QueryState queryState = new QueryState(state); @@ -277,7 +289,7 @@ protected ResultMessage execute(ClientState state, String query) CQLStatement statement = QueryProcessor.parseStatement(formattedQuery, queryState.getClientState()); statement.validate(state); - QueryOptions options = QueryOptions.forInternalCalls(Collections.emptyList()); + QueryOptions options = QueryOptions.forInternalCalls(values); return statement.executeLocally(queryState, options); } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java index 30909e97e4c1..72d88940ed6b 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java @@ -119,6 +119,6 @@ public void testExcludedUsers() throws Throwable private void assertFails(String query) throws Throwable { - assertFails("User provided timestamps (USING TIMESTAMP) is not allowed", query); + assertFails(query, "User provided timestamps (USING TIMESTAMP) is not allowed"); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java index 4780aa247eda..3cc3ab9c1def 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java @@ -108,16 +108,18 @@ private String assertCreateViewSucceeds() throws Throwable private void assertCreateViewWarns() throws Throwable { String viewName = createViewName(); - assertThresholdWarns(format("Creating materialized view %s on table %s, current number of views %s exceeds warning threshold of %s.", - viewName, currentTable(), currentValue() + 1, guardrails().getMaterializedViewsPerTableWarnThreshold()), - format(CREATE_VIEW, viewName)); + assertThresholdWarns(format(CREATE_VIEW, viewName), + format("Creating materialized view %s on table %s, current number of views %s exceeds warning threshold of %s.", + viewName, currentTable(), currentValue() + 1, guardrails().getMaterializedViewsPerTableWarnThreshold()) + ); } private void assertCreateViewFails() throws Throwable { String viewName = createViewName(); - assertThresholdFails(format("aborting the creation of materialized view %s on table %s", - viewName, currentTable()), - format(CREATE_VIEW, viewName)); + assertThresholdFails(format(CREATE_VIEW, viewName), + format("aborting the creation of materialized view %s on table %s", + viewName, currentTable()) + ); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java index e57bfe5b9056..0d2c9f4801de 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java @@ -116,18 +116,18 @@ protected void assertThresholdValid(String query) throws Throwable .isLessThanOrEqualTo(failGetter.applyAsLong(guardrails())); } - protected void assertThresholdWarns(String message, String query) throws Throwable + protected void assertThresholdWarns(String query, String... messages) throws Throwable { - assertWarns(message, query); + assertWarns(query, messages); Assertions.assertThat(currentValue()) .isGreaterThan(warnGetter.applyAsLong(guardrails())) .isLessThanOrEqualTo(failGetter.applyAsLong(guardrails())); } - protected void assertThresholdFails(String message, String query) throws Throwable + protected void assertThresholdFails(String query, String... messages) throws Throwable { - assertFails(message, query); + assertFails(query, messages); Assertions.assertThat(currentValue()) .isGreaterThanOrEqualTo(warnGetter.applyAsLong(guardrails())) diff --git a/tools/stress/src/org/apache/cassandra/io/sstable/StressCQLSSTableWriter.java b/tools/stress/src/org/apache/cassandra/io/sstable/StressCQLSSTableWriter.java index c4ecb07ee079..205b6953990c 100644 --- a/tools/stress/src/org/apache/cassandra/io/sstable/StressCQLSSTableWriter.java +++ b/tools/stress/src/org/apache/cassandra/io/sstable/StressCQLSSTableWriter.java @@ -244,8 +244,9 @@ public StressCQLSSTableWriter rawAddRow(List values) throw new InvalidRequestException(String.format("Invalid number of arguments, expecting %d values but got %d", boundNames.size(), values.size())); QueryOptions options = QueryOptions.forInternalCalls(null, values); - List keys = insert.buildPartitionKeyNames(options); - SortedSet> clusterings = insert.createClustering(options); + ClientState state = ClientState.forInternalCalls(); + List keys = insert.buildPartitionKeyNames(options, state); + SortedSet> clusterings = insert.createClustering(options, state); long now = currentTimeMillis(); // Note that we asks indexes to not validate values (the last 'false' arg below) because that triggers a 'Keyspace.open'