You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ad...@apache.org on 2021/12/17 16:35:06 UTC
[cassandra] branch trunk updated: Migrate thresholds for number of keyspaces and tables to guardrails
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 bae92ee Migrate thresholds for number of keyspaces and tables to guardrails
bae92ee is described below
commit bae92ee139b411c94228f8fd5bb8befb4183ca9f
Author: Andrés de la Peña <a....@gmail.com>
AuthorDate: Wed Dec 8 13:28:17 2021 +0000
Migrate thresholds for number of keyspaces and tables to guardrails
This adds a new guardrail for limiting the number of keyspaces. It also marks the previous not-guardrail thresholds
for keyspaces and tables as deprecated in favour of the equivalent guardrails. GuardrailsOptions is modified to always
log updates on guardrails config. A small refactor decouples guardrails from their configuration interfaces.
patch by Andrés de la Peña; reviewed by Ekaterina Dimitrova for CASSANDRA-17195
---
CHANGES.txt | 1 +
NEWS.txt | 5 +
conf/cassandra.yaml | 6 +
src/java/org/apache/cassandra/config/Config.java | 2 +
.../cassandra/config/DatabaseDescriptor.java | 5 +
.../apache/cassandra/config/GuardrailsOptions.java | 169 ++++++++++++++-------
.../statements/schema/CreateKeyspaceStatement.java | 11 ++
.../statements/schema/CreateTableStatement.java | 9 +-
.../apache/cassandra/db/guardrails/Guardrails.java | 75 ++++++---
.../cassandra/db/guardrails/GuardrailsConfig.java | 54 ++++++-
.../cassandra/db/guardrails/GuardrailsMBean.java | 18 +++
.../apache/cassandra/db/guardrails/Threshold.java | 49 +++---
.../org/apache/cassandra/db/guardrails/Values.java | 41 ++---
.../apache/cassandra/service/StorageService.java | 4 +
.../cassandra/service/StorageServiceMBean.java | 5 +
.../config/DatabaseDescriptorRefTest.java | 3 +
.../db/guardrails/GuardrailKeyspacesTest.java | 117 ++++++++++++++
.../guardrails/GuardrailTablePropertiesTest.java | 17 ++-
...blesLimitTest.java => GuardrailTablesTest.java} | 6 +-
.../guardrails/GuardrailsConfigProviderTest.java | 3 +-
.../cassandra/db/guardrails/GuardrailsTest.java | 77 +++-------
.../cassandra/db/guardrails/ThresholdTester.java | 7 +-
.../schema/CreateTableValidationTest.java | 1 +
23 files changed, 467 insertions(+), 218 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index e829293..99c38b8 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.1
+ * Migrate thresholds for number of keyspaces and tables to guardrails (CASSANDRA-17195)
* Remove self-reference in SSTableTidier (CASSANDRA-17205)
* Add guardrail for query page size (CASSANDRA-17189)
* Allow column_index_size_in_kb to be configurable through nodetool (CASSANDRA-17121)
diff --git a/NEWS.txt b/NEWS.txt
index f365a68..07ee8a4 100644
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -86,6 +86,11 @@ Upgrading
Deprecation
-----------
+ - The properties `keyspace_count_warn_threshold` and `table_count_warn_threshold` in cassandra.yaml have been
+ deprecated in favour of the new `guardrails.keyspaces` and `guardrails.tables` properties and will be removed
+ in a subsequent major version. This also affects the setters and getters for those properties in the JMX MBean
+ `org.apache.cassandra.db:type=StorageService`, which are equally deprecated in favour of the analogous methods
+ in the JMX MBean `org.apache.cassandra.db:type=Guardrails`. See CASSANDRA-17195 for further details.
4.0
===
diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml
index b0c3cb2..541aa6e 100644
--- a/conf/cassandra.yaml
+++ b/conf/cassandra.yaml
@@ -1518,6 +1518,7 @@ report_unconfirmed_repaired_data_mismatches: false
# Having many tables and/or keyspaces negatively affects performance of many operations in the
# cluster. When the number of tables/keyspaces in the cluster exceeds the following thresholds
# a client warning will be sent back to the user when creating a table or keyspace.
+# As of cassandra 4.1, these properties are deprecated in favor of guardrails.keyspaces and guardrails.tables
# table_count_warn_threshold: 150
# keyspace_count_warn_threshold: 40
@@ -1581,6 +1582,11 @@ enable_drop_compact_storage: false
# guardrails:
# Whether guardrails are enabled or not. Guardrails are disabled by default.
# enabled: false
+# Guardrail to warn or abort when creating more user keyspaces than threshold.
+# The two thresholds default to -1 to disable.
+# keyspaces:
+# warn_threshold: -1
+# abort_threshold: -1
# Guardrail to warn or abort when creating more user tables than threshold.
# The two thresholds default to -1 to disable.
# tables:
diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java
index 61aaf3c..b88767e 100644
--- a/src/java/org/apache/cassandra/config/Config.java
+++ b/src/java/org/apache/cassandra/config/Config.java
@@ -617,7 +617,9 @@ public class Config
isClientMode = clientMode;
}
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public volatile int table_count_warn_threshold = 150;
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public volatile int keyspace_count_warn_threshold = 40;
public volatile int consecutive_message_errors_threshold = 1;
diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
index fe820cd..869678b 100644
--- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
+++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
@@ -3539,26 +3539,31 @@ public class DatabaseDescriptor
conf.auto_optimise_preview_repair_streams = enabled;
}
+ @Deprecated
public static int tableCountWarnThreshold()
{
return conf.table_count_warn_threshold;
}
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static void setTableCountWarnThreshold(int value)
{
conf.table_count_warn_threshold = value;
}
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static int keyspaceCountWarnThreshold()
{
return conf.keyspace_count_warn_threshold;
}
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static void setKeyspaceCountWarnThreshold(int value)
{
conf.keyspace_count_warn_threshold = value;
}
+ @Deprecated // this warning threshold will be replaced by an equivalent guardrail
public static ConsistencyLevel getAuthWriteConsistencyLevel()
{
return ConsistencyLevel.valueOf(conf.auth_write_consistency_level);
diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
index aa512f3..e019b84 100644
--- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
+++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
@@ -20,14 +20,16 @@ package org.apache.cassandra.config;
import java.util.Collections;
import java.util.Set;
-import javax.annotation.Nullable;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
import com.google.common.collect.Sets;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.apache.cassandra.cql3.statements.schema.TableAttributes;
import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.db.guardrails.GuardrailsConfig;
-import org.apache.cassandra.db.guardrails.Values;
import static java.lang.String.format;
import static java.util.stream.Collectors.toSet;
@@ -51,7 +53,11 @@ import static java.util.stream.Collectors.toSet;
*/
public class GuardrailsOptions implements GuardrailsConfig
{
+ private static final String NAME_PREFIX = "guardrails.";
+ private static final Logger logger = LoggerFactory.getLogger(GuardrailsOptions.class);
+
public volatile boolean enabled = false;
+ public final IntThreshold keyspaces = new IntThreshold();
public final IntThreshold tables = new IntThreshold();
public final IntThreshold columns_per_table = new IntThreshold();
public final IntThreshold secondary_indexes_per_table = new IntThreshold();
@@ -63,12 +69,13 @@ public class GuardrailsOptions implements GuardrailsConfig
public void validate()
{
- tables.validate("guardrails.tables");
- columns_per_table.validate("guardrails.columns_per_table");
- secondary_indexes_per_table.validate("guardrails.secondary_indexes_per_table");
- materialized_views_per_table.validate("guardrails.materialized_views_per_table");
- table_properties.validate("guardrails.table_properties");
- page_size.validate("guardrails.page_size");
+ keyspaces.init("keyspaces");
+ tables.init("tables");
+ columns_per_table.init("columns_per_table");
+ secondary_indexes_per_table.init("secondary_indexes_per_table");
+ materialized_views_per_table.init("materialized_views_per_table");
+ table_properties.init("table_properties");
+ page_size.init("guardrails.page_size");
}
@Override
@@ -84,7 +91,13 @@ public class GuardrailsOptions implements GuardrailsConfig
*/
public void setEnabled(boolean enabled)
{
- this.enabled = enabled;
+ updatePropertyWithLogging(NAME_PREFIX + "enabled", enabled, () -> this.enabled, x -> this.enabled = x);
+ }
+
+ @Override
+ public IntThreshold getKeyspaces()
+ {
+ return keyspaces;
}
@Override
@@ -131,48 +144,56 @@ public class GuardrailsOptions implements GuardrailsConfig
public void setUserTimestampsEnabled(boolean enabled)
{
- user_timestamps_enabled = enabled;
+ updatePropertyWithLogging(NAME_PREFIX + "user_timestamps_enabled",
+ enabled,
+ () -> user_timestamps_enabled,
+ x -> user_timestamps_enabled = x);
}
- public static abstract class Threshold implements org.apache.cassandra.db.guardrails.Threshold.Config
+ private static <T> void updatePropertyWithLogging(String propertyName, T newValue, Supplier<T> getter, Consumer<T> setter)
{
- public static final long DISABLED = -1;
-
- public volatile long warn_threshold = DISABLED;
- public volatile long abort_threshold = DISABLED;
-
- @Override
- public long getWarnThreshold()
+ T oldValue = getter.get();
+ if (!newValue.equals(oldValue))
{
- return warn_threshold;
+ setter.accept(newValue);
+ logger.info("Updated {} from {} to {}", propertyName, oldValue, newValue);
}
+ }
- @Override
- public long getAbortThreshold()
- {
- return abort_threshold;
- }
+ protected static abstract class Config
+ {
+ protected String name;
- public void setThresholds(long warn, long abort)
+ public String getName()
{
- validateStrictlyPositive(warn, "warn threshold");
- validateStrictlyPositive(abort, "abort threshold");
- validateWarnLowerThanAbort(warn, abort, null);
- warn_threshold = warn;
- abort_threshold = abort;
+ return name;
}
- public void validate(String name)
+ protected void init(String name)
{
- validateStrictlyPositive(warn_threshold, name + ".warn_threshold");
- validateStrictlyPositive(abort_threshold, name + ".abort_threshold");
- validateWarnLowerThanAbort(warn_threshold, abort_threshold, name);
+ this.name = NAME_PREFIX + name;
+ validate();
}
+ protected abstract void validate();
+ }
+
+ public static abstract class Threshold extends Config
+ {
+ public static final long DISABLED = -1;
+
public abstract long maxValue();
+
public abstract boolean allowZero();
- private void validateStrictlyPositive(long value, String name)
+ protected void validate(long warn, long abort)
+ {
+ validateLimits(warn, name + ".warn_threshold");
+ validateLimits(abort, name + ".abort_threshold");
+ validateWarnLowerThanAbort(warn, abort);
+ }
+
+ protected void validateLimits(long value, String name)
{
if (value > maxValue())
throw new IllegalArgumentException(format("Invalid value %d for %s: maximum allowed value is %d",
@@ -190,22 +211,47 @@ public class GuardrailsOptions implements GuardrailsConfig
value, name, DISABLED));
}
- private void validateWarnLowerThanAbort(long warnValue, long abortValue, @Nullable String name)
+ private void validateWarnLowerThanAbort(long warn, long abort)
{
- if (warnValue == DISABLED || abortValue == DISABLED)
+ if (warn == DISABLED || abort == DISABLED)
return;
- if (abortValue < warnValue)
- throw new IllegalArgumentException(format("The warn threshold %d%s should be lower than the abort " +
- "threshold %d",
- warnValue,
- name == null ? "" : " for " + name,
- abortValue));
+ if (abort < warn)
+ throw new IllegalArgumentException(format("The warn threshold %d for %s should be lower than the " +
+ "abort threshold %d", warn, name, abort));
}
}
- public static class IntThreshold extends Threshold
+ public static class IntThreshold extends Threshold implements GuardrailsConfig.IntThreshold
{
+ public volatile int warn_threshold = (int) DISABLED;
+ public volatile int abort_threshold = (int) DISABLED;
+
+ @Override
+ public int getWarnThreshold()
+ {
+ return warn_threshold;
+ }
+
+ @Override
+ public int getAbortThreshold()
+ {
+ return abort_threshold;
+ }
+
+ public void setThresholds(int warn, int abort)
+ {
+ validate(warn, abort);
+ updatePropertyWithLogging(name + ".warn_threshold", warn, () -> warn_threshold, x -> warn_threshold = x);
+ updatePropertyWithLogging(name + ".abort_threshold", abort, () -> abort_threshold, x -> abort_threshold = x);
+ }
+
+ @Override
+ protected void validate()
+ {
+ validate(warn_threshold, abort_threshold);
+ }
+
@Override
public long maxValue()
{
@@ -219,7 +265,7 @@ public class GuardrailsOptions implements GuardrailsConfig
}
}
- public static class TableProperties implements Values.Config<String>
+ public static class TableProperties extends Config implements GuardrailsConfig.TableProperties
{
public volatile Set<String> ignored = Collections.emptySet();
public volatile Set<String> disallowed = Collections.emptySet();
@@ -236,14 +282,31 @@ public class GuardrailsOptions implements GuardrailsConfig
return disallowed;
}
- public void setIgnoredValues(Set<String> values)
+ public void setIgnored(Set<String> properties)
+ {
+ updatePropertyWithLogging(name + ".ignored", validateIgnored(properties), () -> ignored, x -> ignored = x);
+ }
+
+ public void setDisallowed(Set<String> properties)
+ {
+ updatePropertyWithLogging(name + ".disallowed", validateDisallowed(properties), () -> disallowed, x -> disallowed = x);
+ }
+
+ @Override
+ protected void validate()
+ {
+ validateIgnored(ignored);
+ validateDisallowed(disallowed);
+ }
+
+ private Set<String> validateIgnored(Set<String> properties)
{
- this.ignored = validateTableProperties(values, "ignored properties");
+ return validateTableProperties(properties, name + ".ignored");
}
- public void setDisallowedValues(Set<String> values)
+ private Set<String> validateDisallowed(Set<String> properties)
{
- this.disallowed = validateTableProperties(values, "disallowed properties");
+ return validateTableProperties(properties, name + ".disallowed");
}
private Set<String> validateTableProperties(Set<String> properties, String name)
@@ -257,15 +320,9 @@ public class GuardrailsOptions implements GuardrailsConfig
if (!diff.isEmpty())
throw new IllegalArgumentException(format("Invalid value for %s: '%s' do not parse as valid table properties",
- name, diff.toString()));
+ name, diff));
return lowerCaseProperties;
}
-
- public void validate(String name)
- {
- validateTableProperties(ignored, name + ".ignored");
- validateTableProperties(disallowed, name + ".disallowed");
- }
}
}
diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
index 9aa27ce..a6036a3 100644
--- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
@@ -33,6 +33,7 @@ import org.apache.cassandra.auth.IResource;
import org.apache.cassandra.auth.Permission;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.CQLStatement;
+import org.apache.cassandra.db.guardrails.Guardrails;
import org.apache.cassandra.exceptions.AlreadyExistsException;
import org.apache.cassandra.locator.LocalStrategy;
import org.apache.cassandra.schema.KeyspaceMetadata;
@@ -111,8 +112,18 @@ public final class CreateKeyspaceStatement extends AlterSchemaStatement
}
@Override
+ public void validate(ClientState state)
+ {
+ super.validate(state);
+
+ // Guardrail on number of keyspaces
+ Guardrails.keyspaces.guard(Schema.instance.getUserKeyspaces().size() + 1, keyspaceName, state);
+ }
+
+ @Override
Set<String> clientWarnings(KeyspacesDiff diff)
{
+ // this threshold is deprecated, it will be replaced by the guardrail used in #validate(ClientState)
int keyspaceCount = Schema.instance.getKeyspaces().size();
if (keyspaceCount > DatabaseDescriptor.keyspaceCountWarnThreshold())
{
diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
index c765696..44a6042 100644
--- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
@@ -129,21 +129,21 @@ public final class CreateTableStatement extends AlterSchemaStatement
// optimization for the case where they are disabled, so we don't have to do the same check on every guardrail
if (Guardrails.enabled(state))
{
- // Guardrails on table properties
+ // Guardrail on table properties
Guardrails.tableProperties.guard(attrs.updatedProperties(), attrs::removeProperty, state);
// Guardrail on columns per table
Guardrails.columnsPerTable.guard(rawColumns.size(), tableName, state);
- // Guardrails on number of tables
- if (Guardrails.tablesLimit.enabled(state))
+ // Guardrail on number of tables
+ if (Guardrails.tables.enabled(state))
{
int totalUserTables = Schema.instance.getUserKeyspaces()
.stream()
.map(Keyspace::open)
.mapToInt(keyspace -> keyspace.getColumnFamilyStores().size())
.sum();
- Guardrails.tablesLimit.guard(totalUserTables + 1, tableName, state);
+ Guardrails.tables.guard(totalUserTables + 1, tableName, state);
}
}
}
@@ -402,6 +402,7 @@ public final class CreateTableStatement extends AlterSchemaStatement
@Override
public Set<String> clientWarnings(KeyspacesDiff diff)
{
+ // this threshold is deprecated, it will be replaced by the guardrail used in #validate(ClientState)
int tableCount = Schema.instance.getNumberOfTables();
if (tableCount > DatabaseDescriptor.tableCountWarnThreshold())
{
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
index d299215..d29ab34 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
@@ -44,10 +44,23 @@ public final class Guardrails implements GuardrailsMBean
static final Guardrails instance = new Guardrails();
/**
+ * Guardrail on the total number of user keyspaces.
+ */
+ public static final Threshold keyspaces =
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspaces().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspaces().getAbortThreshold(),
+ (isWarning, what, value, threshold) ->
+ isWarning ? format("Creating keyspace %s, current number of keyspaces %s exceeds warning threshold of %s.",
+ what, value, threshold)
+ : format("Cannot have more than %s keyspaces, aborting the creation of keyspace %s",
+ threshold, what));
+
+ /**
* Guardrail on the total number of tables on user keyspaces.
*/
- public static final Threshold tablesLimit =
- new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getTables(),
+ public static final Threshold tables =
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getTables().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getTables().getAbortThreshold(),
(isWarning, what, value, threshold) ->
isWarning ? format("Creating table %s, current number of tables %s exceeds warning threshold of %s.",
what, value, threshold)
@@ -58,7 +71,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of columns per table.
*/
public static final Threshold columnsPerTable =
- new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTable(),
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTable().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTable().getAbortThreshold(),
(isWarning, what, value, threshold) ->
isWarning ? format("The table %s has %s columns, this exceeds the warning threshold of %s.",
what, value, threshold)
@@ -66,7 +80,8 @@ public final class Guardrails implements GuardrailsMBean
threshold, value, what));
public static final Threshold secondaryIndexesPerTable =
- new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTable(),
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTable().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTable().getAbortThreshold(),
(isWarning, what, value, threshold) ->
isWarning ? format("Creating secondary index %s, current number of indexes %s exceeds warning threshold of %s.",
what, value, threshold)
@@ -77,7 +92,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of materialized views per table.
*/
public static final Threshold materializedViewsPerTable =
- new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTable(),
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTable().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTable().getAbortThreshold(),
(isWarning, what, value, threshold) ->
isWarning ? format("Creating materialized view %s, current number of views %s exceeds warning threshold of %s.",
what, value, threshold)
@@ -88,7 +104,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail ignoring/disallowing the usage of certain table properties.
*/
public static final Values<String> tableProperties =
- new Values<>(state -> CONFIG_PROVIDER.getOrCreate(state).getTableProperties(),
+ new Values<>(state -> CONFIG_PROVIDER.getOrCreate(state).getTableProperties().getIgnored(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getTableProperties().getDisallowed(),
"Table Properties");
/**
@@ -102,14 +119,14 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of elements returned within page.
*/
public static final Threshold pageSize =
- new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getPageSize(),
+ new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getPageSize().getWarnThreshold(),
+ state -> CONFIG_PROVIDER.getOrCreate(state).getPageSize().getAbortThreshold(),
(isWarning, what, value, threshold) ->
isWarning ? format("Query for table %s with page size %s exceeds warning threshold of %s.",
what, value, threshold)
: format("Aborting query for table %s, page size %s exceeds abort threshold of %s.",
what, value, threshold));
-
private Guardrails()
{
MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
@@ -139,15 +156,33 @@ public final class Guardrails implements GuardrailsMBean
}
@Override
+ public int getKeyspacesWarnThreshold()
+ {
+ return DEFAULT_CONFIG.getKeyspaces().getWarnThreshold();
+ }
+
+ @Override
+ public int getKeyspacesAbortThreshold()
+ {
+ return DEFAULT_CONFIG.getKeyspaces().getAbortThreshold();
+ }
+
+ @Override
+ public void setKeyspacesThreshold(int warn, int abort)
+ {
+ DEFAULT_CONFIG.getKeyspaces().setThresholds(warn, abort);
+ }
+
+ @Override
public int getTablesWarnThreshold()
{
- return (int) DEFAULT_CONFIG.getTables().getWarnThreshold();
+ return DEFAULT_CONFIG.getTables().getWarnThreshold();
}
@Override
public int getTablesAbortThreshold()
{
- return (int) DEFAULT_CONFIG.getTables().getAbortThreshold();
+ return DEFAULT_CONFIG.getTables().getAbortThreshold();
}
@Override
@@ -159,13 +194,13 @@ public final class Guardrails implements GuardrailsMBean
@Override
public int getColumnsPerTableWarnThreshold()
{
- return (int) DEFAULT_CONFIG.getColumnsPerTable().getWarnThreshold();
+ return DEFAULT_CONFIG.getColumnsPerTable().getWarnThreshold();
}
@Override
public int getColumnsPerTableAbortThreshold()
{
- return (int) DEFAULT_CONFIG.getColumnsPerTable().getAbortThreshold();
+ return DEFAULT_CONFIG.getColumnsPerTable().getAbortThreshold();
}
@Override
@@ -177,13 +212,13 @@ public final class Guardrails implements GuardrailsMBean
@Override
public int getSecondaryIndexesPerTableWarnThreshold()
{
- return (int) DEFAULT_CONFIG.getSecondaryIndexesPerTable().getWarnThreshold();
+ return DEFAULT_CONFIG.getSecondaryIndexesPerTable().getWarnThreshold();
}
@Override
public int getSecondaryIndexesPerTableAbortThreshold()
{
- return (int) DEFAULT_CONFIG.getSecondaryIndexesPerTable().getAbortThreshold();
+ return DEFAULT_CONFIG.getSecondaryIndexesPerTable().getAbortThreshold();
}
@Override
@@ -195,13 +230,13 @@ public final class Guardrails implements GuardrailsMBean
@Override
public int getMaterializedViewsPerTableWarnThreshold()
{
- return (int) DEFAULT_CONFIG.getMaterializedViewsPerTable().getWarnThreshold();
+ return DEFAULT_CONFIG.getMaterializedViewsPerTable().getWarnThreshold();
}
@Override
public int getMaterializedViewsPerTableAbortThreshold()
{
- return (int) DEFAULT_CONFIG.getMaterializedViewsPerTable().getAbortThreshold();
+ return DEFAULT_CONFIG.getMaterializedViewsPerTable().getAbortThreshold();
}
@Override
@@ -230,7 +265,7 @@ public final class Guardrails implements GuardrailsMBean
@Override
public void setTablePropertiesDisallowed(Set<String> properties)
{
- DEFAULT_CONFIG.getTableProperties().setDisallowedValues(properties);
+ DEFAULT_CONFIG.getTableProperties().setDisallowed(properties);
}
@Override
@@ -259,7 +294,7 @@ public final class Guardrails implements GuardrailsMBean
@Override
public void setTablePropertiesIgnored(Set<String> properties)
{
- DEFAULT_CONFIG.getTableProperties().setIgnoredValues(properties);
+ DEFAULT_CONFIG.getTableProperties().setIgnored(properties);
}
@Override
@@ -283,13 +318,13 @@ public final class Guardrails implements GuardrailsMBean
@Override
public int getPageSizeWarnThreshold()
{
- return (int) DEFAULT_CONFIG.getPageSize().getWarnThreshold();
+ return DEFAULT_CONFIG.getPageSize().getWarnThreshold();
}
@Override
public int getPageSizeAbortThreshold()
{
- return (int) DEFAULT_CONFIG.getPageSize().getAbortThreshold();
+ return DEFAULT_CONFIG.getPageSize().getAbortThreshold();
}
@Override
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
index df00a8d..c34f224 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
@@ -18,6 +18,8 @@
package org.apache.cassandra.db.guardrails;
+import java.util.Set;
+
/**
* Configuration settings for guardrails.
*
@@ -49,29 +51,34 @@ public interface GuardrailsConfig
boolean getEnabled();
/**
- * @return The threshold to warn or abort when creating more tables than threshold.
+ * @return The threshold to warn or abort when creating more user keyspaces than threshold.
+ */
+ IntThreshold getKeyspaces();
+
+ /**
+ * @return The threshold to warn or abort when creating more user tables than threshold.
*/
- Threshold.Config getTables();
+ IntThreshold getTables();
/**
* @return The threshold to warn or abort when creating more columns per table than threshold.
*/
- Threshold.Config getColumnsPerTable();
+ IntThreshold getColumnsPerTable();
/**
* @return The threshold to warn or abort when creating more secondary indexes per table than threshold.
*/
- Threshold.Config getSecondaryIndexesPerTable();
+ IntThreshold getSecondaryIndexesPerTable();
/**
* @return The threshold to warn or abort when creating more materialized views per table than threshold.
*/
- Threshold.Config getMaterializedViewsPerTable();
+ IntThreshold getMaterializedViewsPerTable();
/**
* @return The table properties that are ignored/disallowed when creating or altering a table.
*/
- Values.Config<String> getTableProperties();
+ TableProperties getTableProperties();
/**
* Returns whether user-provided timestamps are allowed.
@@ -83,5 +90,38 @@ public interface GuardrailsConfig
/**
* @return The threshold to warn or abort when page size exceeds given size.
*/
- Threshold.Config getPageSize();
+ IntThreshold getPageSize();
+
+ /**
+ * Configuration of {@code int}-based thresholds to check if the guarded value should trigger a warning or abort the
+ * operation.
+ */
+ public interface IntThreshold
+ {
+ /**
+ * @return The threshold to warn when the guarded value exceeds it. A negative value means disabled.
+ */
+ public int getWarnThreshold();
+
+ /**
+ * @return The threshold to abort the operation when the guarded value exceeds it. A negative value means disabled.
+ */
+ public int getAbortThreshold();
+ }
+
+ /**
+ * Configuration class containing the sets of table properties to ignore and/or reject.
+ */
+ public interface TableProperties
+ {
+ /**
+ * @return The values to be ignored.
+ */
+ Set<String> getIgnored();
+
+ /**
+ * @return The values to be rejected.
+ */
+ Set<String> getDisallowed();
+ }
}
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
index 14ea9b0..c3509ee 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
@@ -49,6 +49,24 @@ public interface GuardrailsMBean
void setEnabled(boolean enabled);
/**
+ * @return The threshold to warn when creating more user keyspaces than threshold.
+ * -1 means disabled.
+ */
+ int getKeyspacesWarnThreshold();
+
+ /**
+ * @return The threshold to prevent creating more user keyspaces than threshold.
+ * -1 means disabled.
+ */
+ int getKeyspacesAbortThreshold();
+
+ /**
+ * @param warn The threshold to warn when creating more user keyspaces than threshold. -1 means disabled.
+ * @param abort The threshold to prevent creating more user keyspaces than threshold. -1 means disabled.
+ */
+ void setKeyspacesThreshold(int warn, int abort);
+
+ /**
* @return The threshold to warn when creating more tables than threshold.
* -1 means disabled.
*/
diff --git a/src/java/org/apache/cassandra/db/guardrails/Threshold.java b/src/java/org/apache/cassandra/db/guardrails/Threshold.java
index 7115b9c..d0fd8eb 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Threshold.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Threshold.java
@@ -18,7 +18,7 @@
package org.apache.cassandra.db.guardrails;
-import java.util.function.Function;
+import java.util.function.ToLongFunction;
import javax.annotation.Nullable;
import org.apache.cassandra.service.ClientState;
@@ -33,18 +33,23 @@ import org.apache.cassandra.service.ClientState;
*/
public class Threshold extends Guardrail
{
- private final Function<ClientState, Config> configProvider;
+ private final ToLongFunction<ClientState> warnThreshold;
+ private final ToLongFunction<ClientState> abortThreshold;
private final ErrorMessageProvider messageProvider;
/**
* Creates a new threshold guardrail.
*
- * @param configProvider a {@link ClientState}-based provider of {@link Config}s.
+ * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered.
+ * @param abortThreshold a {@link ClientState}-based provider of the value above which the operation should be aborted.
* @param messageProvider a function to generate the warning or error message if the guardrail is triggered
*/
- public Threshold(Function<ClientState, Config> configProvider, ErrorMessageProvider messageProvider)
+ public Threshold(ToLongFunction<ClientState> warnThreshold,
+ ToLongFunction<ClientState> abortThreshold,
+ ErrorMessageProvider messageProvider)
{
- this.configProvider = configProvider;
+ this.warnThreshold = warnThreshold;
+ this.abortThreshold = abortThreshold;
this.messageProvider = messageProvider;
}
@@ -56,15 +61,15 @@ public class Threshold extends Guardrail
thresholdValue);
}
- private long abortValue(Config config)
+ private long abortValue(ClientState state)
{
- long abortValue = config.getAbortThreshold();
+ long abortValue = abortThreshold.applyAsLong(state);
return abortValue < 0 ? Long.MAX_VALUE : abortValue;
}
- private long warnValue(Config config)
+ private long warnValue(ClientState state)
{
- long warnValue = config.getWarnThreshold();
+ long warnValue = warnThreshold.applyAsLong(state);
return warnValue < 0 ? Long.MAX_VALUE : warnValue;
}
@@ -74,8 +79,7 @@ public class Threshold extends Guardrail
if (!super.enabled(state))
return false;
- Config config = configProvider.apply(state);
- return config.getAbortThreshold() >= 0 || config.getWarnThreshold() >= 0;
+ return abortThreshold.applyAsLong(state) >= 0 || warnThreshold.applyAsLong(state) >= 0;
}
/**
@@ -93,16 +97,14 @@ public class Threshold extends Guardrail
if (!enabled(state))
return;
- Config config = configProvider.apply(state);
-
- long abortValue = abortValue(config);
+ long abortValue = abortValue(state);
if (value > abortValue)
{
triggerAbort(value, abortValue, what);
return;
}
- long warnValue = warnValue(config);
+ long warnValue = warnValue(state);
if (value > warnValue)
triggerWarn(value, warnValue, what);
}
@@ -133,21 +135,4 @@ public class Threshold extends Guardrail
*/
String createMessage(boolean isWarning, String what, long value, long threshold);
}
-
- /**
- * Configuration class containing the thresholds to be used to check if the guarded value should trigger a warning
- * or abort the operation.
- */
- public interface Config
- {
- /**
- * @return The threshold to warn when the guarded value exceeds it. A negative value means disabled.
- */
- public long getWarnThreshold();
-
- /**
- * @return The threshold to abort the operation when the guarded value exceeds it. A negative value means disabled.
- */
- public long getAbortThreshold();
- }
}
diff --git a/src/java/org/apache/cassandra/db/guardrails/Values.java b/src/java/org/apache/cassandra/db/guardrails/Values.java
index 969b1f6..66508a1 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Values.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Values.java
@@ -37,18 +37,23 @@ import static java.lang.String.format;
*/
public class Values<T> extends Guardrail
{
- private final Function<ClientState, Config<T>> configProvider;
+ private final Function<ClientState, Set<T>> ignoredValues;
+ private final Function<ClientState, Set<T>> disallowedValues;
private final String what;
/**
* Creates a new values guardrail.
*
- * @param configProvider a {@link ClientState}-based provider of {@link Config}s.
- * @param what The feature that is guarded by this guardrail (for reporting in error messages).
+ * @param ignoredValues a {@link ClientState}-based of the values that are ignored.
+ * @param disallowedValues a {@link ClientState}-based of the values that are disallowed.
+ * @param what The feature that is guarded by this guardrail (for reporting in error messages).
*/
- public Values(Function<ClientState, Config<T>> configProvider, String what)
+ public Values(Function<ClientState, Set<T>> ignoredValues,
+ Function<ClientState, Set<T>> disallowedValues,
+ String what)
{
- this.configProvider = configProvider;
+ this.ignoredValues = ignoredValues;
+ this.disallowedValues = disallowedValues;
this.what = what;
}
@@ -67,37 +72,19 @@ public class Values<T> extends Guardrail
if (!enabled(state))
return;
- Config<T> config = configProvider.apply(state);
-
- Set<T> disallowed = config.getDisallowed();
+ Set<T> disallowed = disallowedValues.apply(state);
Set<T> toDisallow = Sets.intersection(values, disallowed);
if (!toDisallow.isEmpty())
abort(format("Provided values %s are not allowed for %s (disallowed values are: %s)",
- toDisallow.stream().sorted().collect(Collectors.toList()), what, disallowed.toString()));
+ toDisallow.stream().sorted().collect(Collectors.toList()), what, disallowed));
- Set<T> ignored = config.getIgnored();
+ Set<T> ignored = ignoredValues.apply(state);
Set<T> toIgnore = Sets.intersection(values, ignored);
if (!toIgnore.isEmpty())
{
warn(format("Ignoring provided values %s as they are not supported for %s (ignored values are: %s)",
- toIgnore.stream().sorted().collect(Collectors.toList()), what, ignored.toString()));
+ toIgnore.stream().sorted().collect(Collectors.toList()), what, ignored));
toIgnore.forEach(ignoreAction);
}
}
-
- /**
- * Configuration class containing the sets of values to ignore and/or reject.
- */
- public interface Config<T>
- {
- /**
- * @return The values to be ignored.
- */
- Set<T> getIgnored();
-
- /**
- * @return The values to be rejected.
- */
- Set<T> getDisallowed();
- }
}
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index 8d07784..38fa191 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -6166,11 +6166,13 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
DatabaseDescriptor.setAutoOptimisePreviewRepairStreams(enabled);
}
+ @Deprecated
public int getTableCountWarnThreshold()
{
return DatabaseDescriptor.tableCountWarnThreshold();
}
+ @Deprecated
public void setTableCountWarnThreshold(int value)
{
if (value < 0)
@@ -6179,11 +6181,13 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
DatabaseDescriptor.setTableCountWarnThreshold(value);
}
+ @Deprecated
public int getKeyspaceCountWarnThreshold()
{
return DatabaseDescriptor.keyspaceCountWarnThreshold();
}
+ @Deprecated
public void setKeyspaceCountWarnThreshold(int value)
{
if (value < 0)
diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java
index dcc1b1e..0f3ac51 100644
--- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java
+++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java
@@ -895,9 +895,14 @@ public interface StorageServiceMBean extends NotificationEmitter
public boolean autoOptimisePreviewRepairStreams();
public void setAutoOptimisePreviewRepairStreams(boolean enabled);
+ // warning thresholds will be replaced by equivalent guardrails
+ @Deprecated
int getTableCountWarnThreshold();
+ @Deprecated
void setTableCountWarnThreshold(int value);
+ @Deprecated
int getKeyspaceCountWarnThreshold();
+ @Deprecated
void setKeyspaceCountWarnThreshold(int value);
public void setCompactionTombstoneWarningThreshold(int count);
diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java
index 7c398c4..9fd1128 100644
--- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java
+++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java
@@ -93,7 +93,10 @@ public class DatabaseDescriptorRefTest
"org.apache.cassandra.config.EncryptionOptions$ServerEncryptionOptions$OutgoingEncryptedPortSource",
"org.apache.cassandra.db.guardrails.GuardrailsConfig",
"org.apache.cassandra.db.guardrails.GuardrailsConfigMBean",
+ "org.apache.cassandra.db.guardrails.GuardrailsConfig$IntThreshold",
+ "org.apache.cassandra.db.guardrails.GuardrailsConfig$TableProperties",
"org.apache.cassandra.config.GuardrailsOptions",
+ "org.apache.cassandra.config.GuardrailsOptions$Config",
"org.apache.cassandra.config.GuardrailsOptions$IntThreshold",
"org.apache.cassandra.config.GuardrailsOptions$TableProperties",
"org.apache.cassandra.config.GuardrailsOptions$Threshold",
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
new file mode 100644
index 0000000..281b197
--- /dev/null
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
@@ -0,0 +1,117 @@
+/*
+ * 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 org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.schema.Schema;
+
+import static java.lang.String.format;
+
+/**
+ * Tests the guardrail for the max number of user keyspaces, {@link Guardrails#keyspaces}.
+ */
+public class GuardrailKeyspacesTest extends ThresholdTester
+{
+ private static final int WARN_THRESHOLD = 3; // CQLTester creates two keyspaces
+ private static final int ABORT_THRESHOLD = WARN_THRESHOLD + 1;
+
+ public GuardrailKeyspacesTest()
+ {
+ super(WARN_THRESHOLD,
+ ABORT_THRESHOLD,
+ DatabaseDescriptor.getGuardrailsConfig().getKeyspaces(),
+ Guardrails::setKeyspacesThreshold,
+ Guardrails::getKeyspacesWarnThreshold,
+ Guardrails::getKeyspacesAbortThreshold);
+ }
+
+ @Override
+ protected long currentValue()
+ {
+ return Schema.instance.getUserKeyspaces().size();
+ }
+
+ @Test
+ public void testCreateKeyspace() throws Throwable
+ {
+ // create keyspaces until hitting the two warn/abort thresholds
+ String k1 = assertCreateKeyspaceValid();
+ String k2 = assertCreateKeyspaceWarns();
+ assertCreateKeyspaceAborts();
+
+ // drop a keyspace and hit the warn/abort threshold again
+ dropKeyspace(k2);
+ String k3 = assertCreateKeyspaceWarns();
+ assertCreateKeyspaceAborts();
+
+ // drop two keyspaces and hit the warn/abort threshold again
+ dropKeyspace(k1);
+ dropKeyspace(k3);
+ assertCreateKeyspaceValid();
+ assertCreateKeyspaceWarns();
+ assertCreateKeyspaceAborts();
+
+ // test excluded users
+ testExcludedUsers(this::createKeyspaceQuery,
+ this::createKeyspaceQuery,
+ this::createKeyspaceQuery);
+ }
+
+ private void dropKeyspace(String keyspaceName)
+ {
+ schemaChange(format("DROP KEYSPACE %s", keyspaceName));
+ }
+
+ private String assertCreateKeyspaceValid() throws Throwable
+ {
+ String keyspaceName = createKeyspaceName();
+ assertThresholdValid(createKeyspaceQuery(keyspaceName));
+ return keyspaceName;
+ }
+
+ 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));
+ return keyspaceName;
+ }
+
+ private void assertCreateKeyspaceAborts() throws Throwable
+ {
+ String keyspaceName = createKeyspaceName();
+ assertThresholdAborts(format("Cannot have more than %d keyspaces, aborting the creation of keyspace %s",
+ ABORT_THRESHOLD, keyspaceName),
+ createKeyspaceQuery(keyspaceName));
+ }
+
+ private String createKeyspaceQuery()
+ {
+ return createKeyspaceQuery(createKeyspaceName());
+ }
+
+ private String createKeyspaceQuery(String keyspaceName)
+ {
+ return format("CREATE KEYSPACE %s WITH replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }",
+ keyspaceName);
+ }
+}
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java
index 818a0b6..c4ea369 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java
@@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableSet;
import org.junit.Before;
import org.junit.Test;
+import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.cql3.statements.schema.TableAttributes;
import static java.lang.String.format;
@@ -45,6 +46,10 @@ public class GuardrailTablePropertiesTest extends GuardrailTester
"WHERE pk IS NOT null and ck IS NOT null PRIMARY KEY(ck, pk) %s";
private static final String ALTER_VIEW = "ALTER MATERIALIZED VIEW %s.%s WITH %s";
+ private static final String PROPERTY_NAME = DatabaseDescriptor.getGuardrailsConfig().getTableProperties().getName();
+ private static final String IGNORED_PROPERTY_NAME = PROPERTY_NAME + ".ignored";
+ private static final String DISALLOWED_PROPERTY_NAME = PROPERTY_NAME + ".disallowed";
+
@Before
public void before()
{
@@ -62,9 +67,9 @@ public class GuardrailTablePropertiesTest extends GuardrailTester
@Test
public void testConfigValidation()
{
- String message = "Invalid value for %s properties: null is not allowed";
- assertInvalidProperty(Guardrails::setTablePropertiesIgnored, (Set<String>) null, message, "ignored");
- assertInvalidProperty(Guardrails::setTablePropertiesDisallowed, (Set<String>) null, message, "disallowed");
+ String message = "Invalid value for %s: null is not allowed";
+ assertInvalidProperty(Guardrails::setTablePropertiesIgnored, (Set<String>) null, message, IGNORED_PROPERTY_NAME);
+ assertInvalidProperty(Guardrails::setTablePropertiesDisallowed, (Set<String>) null, message, DISALLOWED_PROPERTY_NAME);
assertValidProperty(Collections.emptySet());
assertValidProperty(TableAttributes.allKeywords());
@@ -82,9 +87,9 @@ public class GuardrailTablePropertiesTest extends GuardrailTester
private void assertInvalidProperty(Set<String> properties, Set<String> rejected)
{
- String message = "Invalid value for %s properties: '%s' do not parse as valid table properties";
- assertInvalidProperty(Guardrails::setTablePropertiesIgnored, properties, message, "ignored", rejected);
- assertInvalidProperty(Guardrails::setTablePropertiesDisallowed, properties, message, "disallowed", rejected);
+ String message = "Invalid value for %s: '%s' do not parse as valid table properties";
+ assertInvalidProperty(Guardrails::setTablePropertiesIgnored, properties, message, IGNORED_PROPERTY_NAME, rejected);
+ assertInvalidProperty(Guardrails::setTablePropertiesDisallowed, properties, message, DISALLOWED_PROPERTY_NAME, rejected);
}
@Test
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesLimitTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
similarity index 96%
rename from test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesLimitTest.java
rename to test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
index 96f1681..7a32b78 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesLimitTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
@@ -26,14 +26,14 @@ import org.apache.cassandra.db.Keyspace;
import static java.lang.String.format;
/**
- * Tests the guardrail for the max number of user tables, {@link Guardrails#tablesLimit}.
+ * Tests the guardrail for the max number of user tables, {@link Guardrails#tables}.
*/
-public class GuardrailTablesLimitTest extends ThresholdTester
+public class GuardrailTablesTest extends ThresholdTester
{
private static final int TABLES_LIMIT_WARN_THRESHOLD = 1;
private static final int TABLES_LIMIT_ABORT_THRESHOLD = 2;
- public GuardrailTablesLimitTest()
+ public GuardrailTablesTest()
{
super(TABLES_LIMIT_WARN_THRESHOLD,
TABLES_LIMIT_ABORT_THRESHOLD,
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
index 20e1aea..24be9a9 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
@@ -34,7 +34,8 @@ public class GuardrailsConfigProviderTest extends GuardrailTester
{
String name = getClass().getCanonicalName() + '$' + CustomProvider.class.getSimpleName();
GuardrailsConfigProvider provider = GuardrailsConfigProvider.build(name);
- Threshold guard = new Threshold(state -> provider.getOrCreate(state).getTables(),
+ Threshold guard = new Threshold(state -> provider.getOrCreate(state).getTables().getWarnThreshold(),
+ state -> provider.getOrCreate(state).getTables().getAbortThreshold(),
(isWarn, what, v, t) -> format("%s: for %s, %s > %s",
isWarn ? "Warning" : "Aborting", what, v, t));
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
index 13d9ee3..edc4c23 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
@@ -35,11 +35,13 @@ import static org.junit.Assert.assertTrue;
public class GuardrailsTest extends GuardrailTester
{
+ public static final int DISABLED = -1;
+
@Test
public void testDisabledThreshold() throws Throwable
{
Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger";
- testDisabledThreshold(new Threshold(state -> new ThresholdConfig(), errorMessageProvider));
+ testDisabledThreshold(new Threshold(state -> DISABLED, state -> DISABLED, errorMessageProvider));
}
private void testDisabledThreshold(Threshold guard) throws Throwable
@@ -56,7 +58,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> new ThresholdConfig(10, 100),
+ Threshold guard = new Threshold(state -> 10,
+ state -> 100,
(isWarn, what, v, t) -> format("%s: for %s, %s > %s",
isWarn ? "Warning" : "Aborting", what, v, t));
@@ -73,7 +76,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testWarnOnlyThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> new ThresholdConfig(10, ThresholdConfig.DISABLED),
+ Threshold guard = new Threshold(state -> 10,
+ state -> DISABLED,
(isWarn, what, v, t) -> format("%s: for %s, %s > %s",
isWarn ? "Warning" : "Aborting", what, v, t));
@@ -86,7 +90,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testAbortOnlyThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> new ThresholdConfig(ThresholdConfig.DISABLED, 10),
+ Threshold guard = new Threshold(state -> DISABLED,
+ state -> 10,
(isWarn, what, v, t) -> format("%s: for %s, %s > %s",
isWarn ? "Warning" : "Aborting", what, v, t));
@@ -99,7 +104,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testThresholdUsers() throws Throwable
{
- Threshold guard = new Threshold(state -> new ThresholdConfig(10, 100),
+ Threshold guard = new Threshold(state -> 10,
+ state -> 100,
(isWarn, what, v, t) -> format("%s: for %s, %s > %s",
isWarn ? "Warning" : "Aborting", what, v, t));
@@ -152,7 +158,8 @@ public class GuardrailsTest extends GuardrailTester
public void testDisallowedValues() throws Throwable
{
// Using a sorted set below to ensure the order in the error message checked below are not random
- Values<Integer> disallowed = new Values<>(state -> new ValuesConfig(Collections.emptySet(), insertionOrderedSet(4, 6, 20)),
+ Values<Integer> disallowed = new Values<>(state -> Collections.emptySet(),
+ state -> insertionOrderedSet(4, 6, 20),
"integer");
Consumer<Integer> action = i -> Assert.fail("The ignore action shouldn't have been triggered");
@@ -174,7 +181,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testDisallowedValuesUsers() throws Throwable
{
- Values<Integer> disallowed = new Values<>(state -> new ValuesConfig(Collections.emptySet(), Collections.singleton(2)),
+ Values<Integer> disallowed = new Values<>(state -> Collections.emptySet(),
+ state -> Collections.singleton(2),
"integer");
Consumer<Integer> action = i -> Assert.fail("The ignore action shouldn't have been triggered");
@@ -207,7 +215,8 @@ public class GuardrailsTest extends GuardrailTester
public void testIgnoredValues() throws Throwable
{
// Using a sorted set below to ensure the order in the error message checked below are not random
- Values<Integer> ignored = new Values<>(state -> new ValuesConfig(insertionOrderedSet(4, 6, 20), Collections.emptySet()),
+ Values<Integer> ignored = new Values<>(state -> insertionOrderedSet(4, 6, 20),
+ state -> Collections.emptySet(),
"integer");
Set<Integer> triggeredOn = set();
@@ -245,56 +254,4 @@ public class GuardrailsTest extends GuardrailTester
{
return new LinkedHashSet<>(Arrays.asList(values));
}
-
- private static class ThresholdConfig implements Threshold.Config
- {
- public static final int DISABLED = -1;
-
- private final int warn;
- private final int abort;
-
- public ThresholdConfig()
- {
- this.warn = DISABLED;
- this.abort = DISABLED;
- }
-
- public ThresholdConfig(int warn, int abort)
- {
- this.warn = warn;
- this.abort = abort;
- }
-
- public long getWarnThreshold()
- {
- return warn;
- }
-
- public long getAbortThreshold()
- {
- return abort;
- }
- }
-
- private static class ValuesConfig implements Values.Config<Integer>
- {
- private final Set<Integer> ignored;
- private final Set<Integer> disallowed;
-
- public ValuesConfig(Set<Integer> ignored, Set<Integer> disallowed)
- {
- this.ignored = ignored;
- this.disallowed = disallowed;
- }
-
- public Set<Integer> getIgnored()
- {
- return ignored;
- }
-
- public Set<Integer> getDisallowed()
- {
- return disallowed;
- }
- }
}
diff --git a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
index fdaa50b..6974e46 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
@@ -37,6 +37,7 @@ import static org.junit.Assert.fail;
*/
public abstract class ThresholdTester extends GuardrailTester
{
+ private final String name;
private final long warnThreshold;
private final long abortThreshold;
private final GuardrailsOptions.Threshold config;
@@ -51,6 +52,7 @@ public abstract class ThresholdTester extends GuardrailTester
ToLongFunction<Guardrails> warnGetter,
ToLongFunction<Guardrails> abortGetter)
{
+ this.name = config.getName();
this.warnThreshold = warnThreshold;
this.abortThreshold = abortThreshold;
this.config = config;
@@ -90,7 +92,7 @@ public abstract class ThresholdTester extends GuardrailTester
@Test
public void testConfigValidation()
{
- testValidationOfThresholdProperties("warn threshold", "abort threshold");
+ testValidationOfThresholdProperties(name + ".warn_threshold", name + ".abort_threshold");
}
protected void testValidationOfThresholdProperties(String warnName, String abortName)
@@ -102,7 +104,8 @@ public abstract class ThresholdTester extends GuardrailTester
setter.accept(guardrails(), -1L, -1L);
Assertions.assertThatThrownBy(() -> setter.accept(guardrails(), 2L, 1L))
- .hasMessageContaining("The warn threshold 2 should be lower than the abort threshold 1");
+ .hasMessageContaining(format("The warn threshold 2 for %s should be lower than the abort threshold 1",
+ name));
}
protected void assertThresholdValid(String query) throws Throwable
diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java
index f2abc7c..5541e35 100644
--- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java
+++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java
@@ -58,6 +58,7 @@ public class CreateTableValidationTest extends CQLTester
createTable("CREATE TABLE %s (a int PRIMARY KEY, b int) WITH bloom_filter_fp_chance = 0.1");
}
+ @Deprecated // these warning thresholds will be replaced by equivalent guardrails
@Test
public void testCreateKeyspaceTableWarning() throws IOException
{
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org