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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
