This is an automated email from the ASF dual-hosted git repository. adelapena pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push: new 8a3b926 Add guardrail for the number of fields per UDT 8a3b926 is described below commit 8a3b9260a9494af56356f4c9829c4068b7ea182f Author: Andrés de la Peña <a.penya.gar...@gmail.com> AuthorDate: Fri Feb 11 18:01:43 2022 +0000 Add guardrail for the number of fields per UDT patch by Andrés de la Peña; reviewed by Ekaterina Dimitrova for CASSANDRA-17385 --- CHANGES.txt | 1 + conf/cassandra.yaml | 4 + src/java/org/apache/cassandra/config/Config.java | 2 + .../apache/cassandra/config/GuardrailsOptions.java | 26 +++++ .../cql3/statements/schema/AlterTypeStatement.java | 14 +++ .../statements/schema/CreateTypeStatement.java | 9 ++ .../apache/cassandra/db/guardrails/Guardrails.java | 31 ++++++ .../cassandra/db/guardrails/GuardrailsConfig.java | 10 ++ .../cassandra/db/guardrails/GuardrailsMBean.java | 16 +++ .../db/guardrails/GuardrailFieldsPerUDTTest.java | 109 +++++++++++++++++++++ .../GuardrailPartitionKeysInSelectTest.java | 6 -- 11 files changed, 222 insertions(+), 6 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 04c9340..ba48d61 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add guardrail for the number of fields per UDT (CASSANDRA-17385) * Allow users to change cqlsh history location using env variable (CASSANDRA-17448) * Add required -f option to use nodetool verify and standalone sstableverify (CASSANDRA-17017) * Add support for UUID based sstable generation identifiers (CASSANDRA-17048) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 5e776c5..fcae5a5 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1665,6 +1665,10 @@ drop_compact_storage_enabled: false # The two thresholds default to -1 to disable. # items_per_collection_warn_threshold: -1 # items_per_collection_fail_threshold: -1 +# Guardrail to warn or fail when creating a user-defined-type with more fields in than threshold. +# Default -1 to disable. +# fields_per_udt_warn_threshold: -1 +# fields_per_udt_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 76db9c9..61c64f8 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -805,6 +805,8 @@ public class Config public volatile DataStorageSpec collection_size_fail_threshold = DISABLED_SIZE_GUARDRAIL; public volatile int items_per_collection_warn_threshold = DISABLED_GUARDRAIL; public volatile int items_per_collection_fail_threshold = DISABLED_GUARDRAIL; + public volatile int fields_per_udt_warn_threshold = DISABLED_GUARDRAIL; + public volatile int fields_per_udt_fail_threshold = DISABLED_GUARDRAIL; 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 692c255..4a6ff6c 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -79,6 +79,7 @@ public class GuardrailsOptions implements GuardrailsConfig config.write_consistency_levels_disallowed = validateConsistencyLevels(config.write_consistency_levels_disallowed, "write_consistency_levels_disallowed"); validateSizeThreshold(config.collection_size_warn_threshold, config.collection_size_fail_threshold, "collection_size"); validateIntThreshold(config.items_per_collection_warn_threshold, config.items_per_collection_fail_threshold, "items_per_collection"); + validateIntThreshold(config.fields_per_udt_warn_threshold, config.fields_per_udt_fail_threshold, "fields_per_udt"); } @Override @@ -474,6 +475,31 @@ public class GuardrailsOptions implements GuardrailsConfig x -> config.items_per_collection_fail_threshold = x); } + @Override + public int getFieldsPerUDTWarnThreshold() + { + return config.fields_per_udt_warn_threshold; + } + + @Override + public int getFieldsPerUDTFailThreshold() + { + return config.fields_per_udt_fail_threshold; + } + + public void setFieldsPerUDTThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "fields_per_udt"); + updatePropertyWithLogging("fields_per_udt_warn_threshold", + warn, + () -> config.fields_per_udt_warn_threshold, + x -> config.fields_per_udt_warn_threshold = x); + updatePropertyWithLogging("fields_per_udt_fail_threshold", + fail, + () -> config.fields_per_udt_fail_threshold, + x -> config.fields_per_udt_fail_threshold = x); + } + private static <T> void updatePropertyWithLogging(String propertyName, T newValue, Supplier<T> getter, Consumer<T> setter) { T oldValue = getter.get(); diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java index 5099d33..582a970 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java @@ -27,6 +27,7 @@ import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.audit.AuditLogEntryType; import org.apache.cassandra.auth.Permission; import org.apache.cassandra.cql3.*; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.UserType; import org.apache.cassandra.schema.KeyspaceMetadata; @@ -98,6 +99,8 @@ public abstract class AlterTypeStatement extends AlterSchemaStatement private final FieldIdentifier fieldName; private final CQL3Type.Raw type; + private ClientState state; + private AddField(String keyspaceName, String typeName, FieldIdentifier fieldName, CQL3Type.Raw type) { super(keyspaceName, typeName); @@ -105,6 +108,15 @@ public abstract class AlterTypeStatement extends AlterSchemaStatement this.type = type; } + @Override + public void validate(ClientState state) + { + super.validate(state); + + // save the query state to use it for guardrails validation in #apply + this.state = state; + } + UserType apply(KeyspaceMetadata keyspace, UserType userType) { if (userType.fieldPosition(fieldName) >= 0) @@ -122,6 +134,8 @@ public abstract class AlterTypeStatement extends AlterSchemaStatement String.join(", ", transform(tablesWithTypeInPartitionKey, TableMetadata::toString))); } + Guardrails.fieldsPerUDT.guard(userType.size() + 1, userType.getNameAsString(), false, state); + List<FieldIdentifier> fieldNames = new ArrayList<>(userType.fieldNames()); fieldNames.add(fieldName); List<AbstractType<?>> fieldTypes = new ArrayList<>(userType.fieldTypes()); fieldTypes.add(fieldType); diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTypeStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTypeStatement.java index 3d506cc..e015c34 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTypeStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTypeStatement.java @@ -26,6 +26,7 @@ import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.CQLStatement; import org.apache.cassandra.cql3.FieldIdentifier; import org.apache.cassandra.cql3.UTName; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.UserType; import org.apache.cassandra.schema.KeyspaceMetadata; @@ -61,6 +62,14 @@ public final class CreateTypeStatement extends AlterSchemaStatement this.ifNotExists = ifNotExists; } + @Override + public void validate(ClientState state) + { + super.validate(state); + + Guardrails.fieldsPerUDT.guard(fieldNames.size(), typeName, false, state); + } + public Keyspaces apply(Keyspaces schema) { KeyspaceMetadata keyspace = schema.getNullable(keyspaceName); diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 99aae55..6a07fdd 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -226,6 +226,19 @@ public final class Guardrails implements GuardrailsMBean : format("Detected collection %s with %s items, this exceeds the failure threshold of %s.", what, value, threshold)); + /** + * Guardrail on the number of fields on each UDT. + */ + public static final Threshold fieldsPerUDT = + new Threshold("fields_per_udt", + state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("The user type %s has %s columns, this exceeds the warning threshold of %s.", + what, value, threshold) + : format("User types cannot have more than %s columns, but %s provided for user type %s.", + threshold, value, what)); + private Guardrails() { MBeanWrapper.instance.registerMBean(this, MBEAN_NAME); @@ -640,6 +653,24 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setWriteConsistencyLevelsDisallowed(fromCSV(consistencyLevels, ConsistencyLevel::fromString)); } + @Override + public int getFieldsPerUDTWarnThreshold() + { + return DEFAULT_CONFIG.getFieldsPerUDTWarnThreshold(); + } + + @Override + public int getFieldsPerUDTFailThreshold() + { + return DEFAULT_CONFIG.getFieldsPerUDTFailThreshold(); + } + + @Override + public void setFieldsPerUDTThreshold(int warn, int fail) + { + DEFAULT_CONFIG.setFieldsPerUDTThreshold(warn, fail); + } + private static String toCSV(Set<String> values) { return values == null || values.isEmpty() ? "" : 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 396d6cb..1e14131 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -204,4 +204,14 @@ public interface GuardrailsConfig * @return The threshold to prevent collections with more elements than threshold. */ int getItemsPerCollectionFailThreshold(); + + /** + * @return The threshold to warn when creating a UDT with more fields than threshold. + */ + int getFieldsPerUDTWarnThreshold(); + + /** + * @return The threshold to fail when creating a UDT with more fields than threshold. + */ + int getFieldsPerUDTFailThreshold(); } diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 7a4eb60..67c118b 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -394,4 +394,20 @@ public interface GuardrailsMBean * @param fail The threshold to prevent collectiosn with more elements than threshold. */ void setItemsPerCollectionThreshold(int warn, int fail); + + /** + * @return The threshold to warn when creating a UDT with more fields than threshold. -1 means disabled. + */ + int getFieldsPerUDTWarnThreshold(); + + /** + * @return The threshold to fail when creating a UDT with more fields than threshold. -1 means disabled. + */ + int getFieldsPerUDTFailThreshold(); + + /** + * @param warn The threshold to warn when creating a UDT with more fields than threshold. -1 means disabled. + * @param fail The threshold to prevent creating a UDT with more fields than threshold. -1 means disabled. + */ + void setFieldsPerUDTThreshold(int warn, int fail); } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailFieldsPerUDTTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailFieldsPerUDTTest.java new file mode 100644 index 0000000..96ad28a --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailFieldsPerUDTTest.java @@ -0,0 +1,109 @@ +/* + * 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 org.junit.Test; + +import static java.lang.String.format; + +/** + * Tests the guardrail for the number of fields in a user-defined type, {@link Guardrails#fieldsPerUDT}. + */ +public class GuardrailFieldsPerUDTTest extends ThresholdTester +{ + private static final int WARN_THRESHOLD = 2; + private static final int FAIL_THRESHOLD = 4; + + public GuardrailFieldsPerUDTTest() + { + super(WARN_THRESHOLD, + FAIL_THRESHOLD, + Guardrails.fieldsPerUDT, + Guardrails::setFieldsPerUDTThreshold, + Guardrails::getFieldsPerUDTWarnThreshold, + Guardrails::getFieldsPerUDTFailThreshold); + } + + @Test + public void testCreateType() throws Throwable + { + assertValid("CREATE TYPE %s (a int)"); + assertValid("CREATE TYPE %s (a int, b int)"); + assertWarns("CREATE TYPE %s (a int, b int, c int)", 3); + assertWarns("CREATE TYPE %s (a int, b int, c int, d int)", 4); + assertFails("CREATE TYPE %s (a int, b int, c int, d int, e int)", 5); + assertFails("CREATE TYPE %s (a int, b int, c int, d int, e int, f int)", 6); + } + + @Test + public void testAlterType() throws Throwable + { + String name = createType("CREATE TYPE %s (a int)"); + + assertValid("ALTER TYPE %s ADD b int", name); + assertWarns("ALTER TYPE %s ADD c int", name, 3); + assertWarns("ALTER TYPE %s ADD d int", name, 4); + assertFails("ALTER TYPE %s ADD e int", name, 5); + } + + @Test + public void testExcludedUsers() throws Throwable + { + String name = createTypeName(); + testExcludedUsers(() -> format("CREATE TYPE %s (a int, b int, c int, d int, e int)", name), + () -> format("ALTER TYPE %s ADD f int", name), + () -> format("DROP TYPE %s", name)); + } + + protected void assertValid(String query) throws Throwable + { + assertValid(query, createTypeName()); + } + + private void assertValid(String query, String typeName) throws Throwable + { + super.assertValid(format(query, typeName)); + } + + private void assertWarns(String query, int numFields) throws Throwable + { + String typeName = createTypeName(); + assertWarns(query, typeName, numFields); + } + + private void assertWarns(String query, String typeName, int numFields) throws Throwable + { + assertWarns(format(query, typeName), + format("The user type %s has %s columns, this exceeds the warning threshold of %s.", + typeName, numFields, WARN_THRESHOLD)); + } + + private void assertFails(String query, int numFields) throws Throwable + { + String typeName = createTypeName(); + assertFails(query, typeName, numFields); + } + + private void assertFails(String query, String typeName, int numFields) throws Throwable + { + assertFails(format(query, typeName), + format("User types cannot have more than %s columns, but %s provided for user type %s", + FAIL_THRESHOLD, numFields, typeName)); + } +} diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java index 9acef56..d4b913e 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java @@ -67,10 +67,4 @@ public class GuardrailPartitionKeysInSelectTest extends ThresholdTester testExcludedUsers(() -> "SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5)", () -> "SELECT k, c, v FROM %s WHERE k IN (2, 3, 4, 5, 6, 7)"); } - - @Override - protected long currentValue() - { - throw new UnsupportedOperationException(); - } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org