mirror of https://github.com/apache/cassandra
Add guardrail for the number of fields per UDT
patch by Andrés de la Peña; reviewed by Ekaterina Dimitrova for CASSANDRA-17385
This commit is contained in:
parent
7c1ed6c409
commit
8a3b9260a9
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue