diff --git a/CHANGES.txt b/CHANGES.txt index 9b897d4399..d3808db893 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Flatten guardrails config (CASSANDRA-17353) * Instance failed to start up due to NPE in StartupClusterConnectivityChecker (CASSANDRA-17347) * add the shorter version of version flag (-v) in cqlsh (CASSANDRA-17236) * Make vtables accessible via internode messaging (CASSANDRA-17295) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 71fb5626b7..5e8bc55113 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1510,7 +1510,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 +# As of cassandra 4.1, these properties are deprecated in favor of keyspaces_warn_threshold and tables_warn_threshold # table_count_warn_threshold: 150 # keyspace_count_warn_threshold: 40 @@ -1577,46 +1577,37 @@ drop_compact_storage_enabled: false # warn_threshold_kb: 0 # abort_threshold_kb: 0 -# Guardrails settings. -# 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. +# guardrails_enabled: true +# Guardrail to warn or fail 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. +# keyspaces_warn_threshold: -1 +# keyspaces_fail_threshold: -1 +# Guardrail to warn or fail when creating more user tables than threshold. # The two thresholds default to -1 to disable. -# tables: -# warn_threshold: -1 -# abort_threshold: -1 -# Guardrail to warn or abort when creating/altering a table with more columns per table than threshold. +# tables_warn_threshold: -1 +# tables_fail_threshold: -1 +# Guardrail to warn or fail when creating/altering a table with more columns per table than threshold. # The two thresholds default to -1 to disable. -# columns_per_table: -# warn_threshold: -1 -# abort_threshold: -1 -# Guardrail to warn or abort when creating more secondary indexes per table than threshold. +# columns_per_table_warn_threshold: -1 +# columns_per_table_fail_threshold: -1 +# Guardrail to warn or fail when creating more secondary indexes per table than threshold. # The two thresholds default to -1 to disable. -# secondary_indexes_per_table: -# warn_threshold: -1 -# abort_threshold: -1 -# Guardrail to warn or abort when creating more materialized views per table than threshold. +# secondary_indexes_per_table_warn_threshold: -1 +# secondary_indexes_per_table_fail_threshold: -1 +# Guardrail to warn or fail when creating more materialized views per table than threshold. # The two thresholds default to -1 to disable. -# materialized_views_per_table: -# warn_threshold: -1 -# abort_threshold: -1 +# materialized_views_per_table_warn_threshold: -1 +# materialized_views_per_table_fail_threshold: -1 # Guardrail to ignore or reject properties when creating tables. By default all properties are allowed. -# table_properties: -# ignored: [] -# disallowed: [] +# table_properties_ignored: [] +# table_properties_disallowed: [] # Guardrail to allow/disallow user-provided timestamps. Defaults to true. -# user_timestamps_enabled: true -# Guardrail to warn or abort when using a page size greater than threshold. +# user_timestamps_enabled: true +# Guardrail to warn or fail when using a page size greater than threshold. # The two thresholds default to -1 to disable. -# page_size: -# warn_threshold: -1 -# abort_threshold: -1 +# page_size_warn_threshold: -1 +# page_size_fail_threshold: -1 # Guardrail to allow/disallow list operations that require read before write, i.e. setting list element by index and # removing list elements by either index or value. Defaults to true. -# read_before_write_list_operations_enabled: true +# read_before_write_list_operations_enabled: true diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 1d0158153a..df20954528 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -20,6 +20,7 @@ package org.apache.cassandra.config; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -666,9 +667,6 @@ public class Config */ public volatile double range_tombstone_list_growth_factor = 1.5; - /** The configuration for guardrails. */ - public final GuardrailsOptions guardrails = new GuardrailsOptions(); - /** * @deprecated migrate to {@link DatabaseDescriptor#isClientInitialized()} */ @@ -725,6 +723,25 @@ public class Config public volatile SubnetGroups client_error_reporting_exclusions = new SubnetGroups(); public volatile SubnetGroups internode_error_reporting_exclusions = new SubnetGroups(); + public static final int DISABLED_GUARDRAIL = -1; + public volatile boolean guardrails_enabled = false; + public volatile int keyspaces_warn_threshold = DISABLED_GUARDRAIL; + public volatile int keyspaces_fail_threshold = DISABLED_GUARDRAIL; + public volatile int tables_warn_threshold = DISABLED_GUARDRAIL; + public volatile int tables_fail_threshold = DISABLED_GUARDRAIL; + public volatile int columns_per_table_warn_threshold = DISABLED_GUARDRAIL; + public volatile int columns_per_table_fail_threshold = DISABLED_GUARDRAIL; + public volatile int secondary_indexes_per_table_warn_threshold = DISABLED_GUARDRAIL; + public volatile int secondary_indexes_per_table_fail_threshold = DISABLED_GUARDRAIL; + public volatile int materialized_views_per_table_warn_threshold = DISABLED_GUARDRAIL; + public volatile int materialized_views_per_table_fail_threshold = DISABLED_GUARDRAIL; + public volatile int page_size_warn_threshold = DISABLED_GUARDRAIL; + public volatile int page_size_fail_threshold = DISABLED_GUARDRAIL; + public volatile Set table_properties_ignored = Collections.emptySet(); + public volatile Set table_properties_disallowed = Collections.emptySet(); + public volatile boolean user_timestamps_enabled = true; + public volatile boolean read_before_write_list_operations_enabled = true; + public enum PaxosVariant { v1_without_linearizable_reads, // with legacy semantics for read/read linearizability (i.e. not guaranteed) diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index cfa3945a2c..74c8b26131 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -155,6 +155,9 @@ public class DatabaseDescriptor public static volatile boolean allowUnlimitedConcurrentValidations = Boolean.getBoolean("cassandra.allow_unlimited_concurrent_validations"); + /** The configuration for guardrails. */ + private static GuardrailsOptions guardrails; + private static Function commitLogSegmentMgrProvider = c -> DatabaseDescriptor.isCDCEnabled() ? new CommitLogSegmentManagerCDC(c, DatabaseDescriptor.getCommitLogLocation()) : new CommitLogSegmentManagerStandard(c, DatabaseDescriptor.getCommitLogLocation()); @@ -879,14 +882,14 @@ public class DatabaseDescriptor public static GuardrailsOptions getGuardrailsConfig() { - return conf.guardrails; + return guardrails; } private static void applyGuardrails() { try { - conf.guardrails.validate(); + guardrails = new GuardrailsOptions(conf); } catch (IllegalArgumentException e) { diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index ff42f59ba1..447c8d048c 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -18,7 +18,6 @@ package org.apache.cassandra.config; -import java.util.Collections; import java.util.Set; import java.util.function.Consumer; import java.util.function.Supplier; @@ -44,7 +43,7 @@ import static java.util.stream.Collectors.toSet; *

This contains a main setting, {@code enabled}, controlling if guardrails are globally active or not, and * individual settings to control each guardrail. * - *

We have 2 variants of guardrails, soft (warn) and hard (abort) limits, each guardrail having either one of the + *

We have 2 variants of guardrails, soft (warn) and hard (fail) limits, each guardrail having either one of the * variants or both. Note in particular that hard limits only make sense for guardrails triggering during query * execution. For other guardrails, say one triggering during compaction, aborting that compaction does not make sense. * @@ -53,36 +52,27 @@ 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); + + private final Config config; - 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(); - public final IntThreshold materialized_views_per_table = new IntThreshold(); - public final TableProperties table_properties = new TableProperties(); - public final IntThreshold page_size = new IntThreshold(); - - public volatile boolean user_timestamps_enabled = true; - public volatile boolean read_before_write_list_operations_enabled = true; - - public void validate() + public GuardrailsOptions(Config config) { - 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"); + this.config = config; + validateIntThreshold(config.keyspaces_warn_threshold, config.keyspaces_fail_threshold, "keyspaces"); + validateIntThreshold(config.tables_warn_threshold, config.tables_fail_threshold, "tables"); + validateIntThreshold(config.columns_per_table_warn_threshold, config.columns_per_table_fail_threshold, "columns_per_table"); + validateIntThreshold(config.secondary_indexes_per_table_warn_threshold, config.secondary_indexes_per_table_fail_threshold, "secondary_indexes_per_table"); + validateIntThreshold(config.materialized_views_per_table_warn_threshold, config.materialized_views_per_table_fail_threshold, "materialized_views_per_table"); + config.table_properties_ignored = validateTableProperties(config.table_properties_ignored, "table_properties_ignored"); + config.table_properties_disallowed = validateTableProperties(config.table_properties_disallowed, "table_properties_disallowed"); + validateIntThreshold(config.page_size_warn_threshold, config.page_size_fail_threshold, "page_size"); } @Override public boolean getEnabled() { - return enabled; + return config.guardrails_enabled; } /** @@ -92,77 +82,216 @@ public class GuardrailsOptions implements GuardrailsConfig */ public void setEnabled(boolean enabled) { - updatePropertyWithLogging(NAME_PREFIX + "enabled", enabled, () -> this.enabled, x -> this.enabled = x); + updatePropertyWithLogging("guardrails_enabled", + enabled, + () -> config.guardrails_enabled, + x -> config.guardrails_enabled = x); } @Override - public IntThreshold getKeyspaces() + public int getKeyspacesWarnThreshold() { - return keyspaces; + return config.keyspaces_warn_threshold; } @Override - public IntThreshold getTables() + public int getKeyspacesFailThreshold() { - return tables; + return config.keyspaces_fail_threshold; + } + + public void setKeyspacesThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "keyspaces"); + updatePropertyWithLogging("keyspaces_warn_threshold", + warn, + () -> config.keyspaces_warn_threshold, + x -> config.keyspaces_warn_threshold = x); + updatePropertyWithLogging("keyspaces_fail_threshold", + fail, + () -> config.keyspaces_fail_threshold, + x -> config.keyspaces_fail_threshold = x); } @Override - public IntThreshold getColumnsPerTable() + public int getTablesWarnThreshold() { - return columns_per_table; + return config.tables_warn_threshold; } @Override - public IntThreshold getSecondaryIndexesPerTable() + public int getTablesFailThreshold() { - return secondary_indexes_per_table; + return config.tables_fail_threshold; + } + + public void setTablesThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "tables"); + updatePropertyWithLogging("tables_warn_threshold", + warn, + () -> config.tables_warn_threshold, + x -> config.tables_warn_threshold = x); + updatePropertyWithLogging("tables_fail_threshold", + fail, + () -> config.tables_fail_threshold, + x -> config.tables_fail_threshold = x); } @Override - public IntThreshold getMaterializedViewsPerTable() + public int getColumnsPerTableWarnThreshold() { - return materialized_views_per_table; + return config.columns_per_table_warn_threshold; } @Override - public TableProperties getTableProperties() + public int getColumnsPerTableFailThreshold() { - return table_properties; + return config.columns_per_table_fail_threshold; + } + + public void setColumnsPerTableThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "columns_per_table"); + updatePropertyWithLogging("columns_per_table_warn_threshold", + warn, + () -> config.columns_per_table_warn_threshold, + x -> config.columns_per_table_warn_threshold = x); + updatePropertyWithLogging("columns_per_table_fail_threshold", + fail, + () -> config.columns_per_table_fail_threshold, + x -> config.columns_per_table_fail_threshold = x); + } + + @Override + public int getSecondaryIndexesPerTableWarnThreshold() + { + return config.secondary_indexes_per_table_warn_threshold; + } + + @Override + public int getSecondaryIndexesPerTableFailThreshold() + { + return config.secondary_indexes_per_table_fail_threshold; + } + + public void setSecondaryIndexesPerTableThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "secondary_indexes_per_table"); + updatePropertyWithLogging("secondary_indexes_per_table_warn_threshold", + warn, + () -> config.secondary_indexes_per_table_warn_threshold, + x -> config.secondary_indexes_per_table_warn_threshold = x); + updatePropertyWithLogging("secondary_indexes_per_table_fail_threshold", + fail, + () -> config.secondary_indexes_per_table_fail_threshold, + x -> config.secondary_indexes_per_table_fail_threshold = x); + } + + @Override + public int getMaterializedViewsPerTableWarnThreshold() + { + return config.materialized_views_per_table_warn_threshold; + } + + @Override + public int getMaterializedViewsPerTableFailThreshold() + { + return config.materialized_views_per_table_fail_threshold; + } + + public void setMaterializedViewsPerTableThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "materialized_views_per_table"); + updatePropertyWithLogging("materialized_views_per_table_warn_threshold", + warn, + () -> config.materialized_views_per_table_warn_threshold, + x -> config.materialized_views_per_table_warn_threshold = x); + updatePropertyWithLogging("materialized_views_per_table_fail_threshold", + fail, + () -> config.materialized_views_per_table_fail_threshold, + x -> config.materialized_views_per_table_fail_threshold = x); + } + + @Override + public int getPageSizeWarnThreshold() + { + return config.page_size_warn_threshold; + } + + @Override + public int getPageSizeFailThreshold() + { + return config.page_size_fail_threshold; + } + + public void setPageSizeThreshold(int warn, int fail) + { + validateIntThreshold(warn, fail, "page_size"); + updatePropertyWithLogging("page_size_warn_threshold", + warn, + () -> config.page_size_warn_threshold, + x -> config.page_size_warn_threshold = x); + updatePropertyWithLogging("page_size_fail_threshold", + fail, + () -> config.page_size_fail_threshold, + x -> config.page_size_fail_threshold = x); + } + + @Override + public Set getTablePropertiesIgnored() + { + return config.table_properties_ignored; + } + + public void setTablePropertiesIgnored(Set properties) + { + updatePropertyWithLogging("table_properties_ignored", + validateTableProperties(properties, "table_properties_ignored"), + () -> config.table_properties_ignored, + x -> config.table_properties_ignored = x); + } + + @Override + public Set getTablePropertiesDisallowed() + { + return config.table_properties_disallowed; + } + + public void setTablePropertiesDisallowed(Set properties) + { + updatePropertyWithLogging("table_properties_disallowed", + validateTableProperties(properties, "table_properties_disallowed"), + () -> config.table_properties_disallowed, + x -> config.table_properties_disallowed = x); } @Override public boolean getUserTimestampsEnabled() { - return user_timestamps_enabled; - } - - @Override - public IntThreshold getPageSize() - { - return page_size; + return config.user_timestamps_enabled; } public void setUserTimestampsEnabled(boolean enabled) { - updatePropertyWithLogging(NAME_PREFIX + "user_timestamps_enabled", + updatePropertyWithLogging("user_timestamps_enabled", enabled, - () -> user_timestamps_enabled, - x -> user_timestamps_enabled = x); + () -> config.user_timestamps_enabled, + x -> config.user_timestamps_enabled = x); } @Override public boolean getReadBeforeWriteListOperationsEnabled() { - return read_before_write_list_operations_enabled; + return config.read_before_write_list_operations_enabled; } public void setReadBeforeWriteListOperationsEnabled(boolean enabled) { - updatePropertyWithLogging(NAME_PREFIX + "read_before_write_list_operations_enabled", + updatePropertyWithLogging("read_before_write_list_operations_enabled", enabled, - () -> read_before_write_list_operations_enabled, - x -> read_before_write_list_operations_enabled = x); + () -> config.read_before_write_list_operations_enabled, + x -> config.read_before_write_list_operations_enabled = x); } private static void updatePropertyWithLogging(String propertyName, T newValue, Supplier getter, Consumer setter) @@ -175,169 +304,62 @@ public class GuardrailsOptions implements GuardrailsConfig } } - protected static abstract class Config + private static void validatePositiveNumeric(long value, long maxValue, boolean allowZero, String name) { - protected String name; + if (value > maxValue) + throw new IllegalArgumentException(format("Invalid value %d for %s: maximum allowed value is %d", + value, name, maxValue)); - public String getName() - { - return name; - } + if (value == 0 && !allowZero) + throw new IllegalArgumentException(format("Invalid value for %s: 0 is not allowed; " + + "if attempting to disable use %d", + name, Config.DISABLED_GUARDRAIL)); - protected void init(String name) - { - this.name = NAME_PREFIX + name; - validate(); - } - - protected abstract void validate(); + // We allow -1 as a general "disabling" flag. But reject anything lower to avoid mistakes. + if (value < Config.DISABLED_GUARDRAIL) + throw new IllegalArgumentException(format("Invalid value %d for %s: negative values are not allowed, " + + "outside of %d which disables the guardrail", + value, name, Config.DISABLED_GUARDRAIL)); } - public static abstract class Threshold extends Config + private static void validateStrictlyPositiveInteger(long value, String name) { - public static final long DISABLED = -1; - - public abstract long maxValue(); - - public abstract boolean allowZero(); - - 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", - value, name, maxValue())); - - if (value == 0 && !allowZero()) - throw new IllegalArgumentException(format("Invalid value for %s: 0 is not allowed; " + - "if attempting to disable use %d", - name, DISABLED)); - - // We allow -1 as a general "disabling" flag. But reject anything lower to avoid mistakes. - if (value < DISABLED) - throw new IllegalArgumentException(format("Invalid value %d for %s: negative values are not allowed, " + - "outside of %d which disables the guardrail", - value, name, DISABLED)); - } - - private void validateWarnLowerThanAbort(long warn, long abort) - { - if (warn == DISABLED || abort == DISABLED) - return; - - if (abort < warn) - throw new IllegalArgumentException(format("The warn threshold %d for %s should be lower than the " + - "abort threshold %d", warn, name, abort)); - } + // We use 'long' for generality, but most numeric guardrail cannot effectively be more than a 'int' for various + // internal reasons. Not that any should ever come close in practice ... + // Also, in most cases, zero does not make sense (allowing 0 tables or columns is not exactly useful). + validatePositiveNumeric(value, Integer.MAX_VALUE, false, name); } - public static class IntThreshold extends Threshold implements GuardrailsConfig.IntThreshold + private static void validateIntThreshold(int warn, int fail, String name) { - 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() - { - return Integer.MAX_VALUE; - } - - @Override - public boolean allowZero() - { - return false; - } + validateStrictlyPositiveInteger(warn, name + "_warn_threshold"); + validateStrictlyPositiveInteger(fail, name + "_fail_threshold"); + validateWarnLowerThanFail(warn, fail, name); } - public static class TableProperties extends Config implements GuardrailsConfig.TableProperties + private static void validateWarnLowerThanFail(long warn, long fail, String name) { - public volatile Set ignored = Collections.emptySet(); - public volatile Set disallowed = Collections.emptySet(); + if (warn == Config.DISABLED_GUARDRAIL || fail == Config.DISABLED_GUARDRAIL) + return; - @Override - public Set getIgnored() - { - return ignored; - } + if (fail < warn) + throw new IllegalArgumentException(format("The warn threshold %d for %s_warn_threshold should be lower " + + "than the fail threshold %d", warn, name, fail)); + } - @Override - public Set getDisallowed() - { - return disallowed; - } + private static Set validateTableProperties(Set properties, String name) + { + if (properties == null) + throw new IllegalArgumentException(format("Invalid value for %s: null is not allowed", name)); - public void setIgnored(Set properties) - { - updatePropertyWithLogging(name + ".ignored", validateIgnored(properties), () -> ignored, x -> ignored = x); - } + Set lowerCaseProperties = properties.stream().map(String::toLowerCase).collect(toSet()); - public void setDisallowed(Set properties) - { - updatePropertyWithLogging(name + ".disallowed", validateDisallowed(properties), () -> disallowed, x -> disallowed = x); - } + Set diff = Sets.difference(lowerCaseProperties, TableAttributes.allKeywords()); - @Override - protected void validate() - { - validateIgnored(ignored); - validateDisallowed(disallowed); - } + if (!diff.isEmpty()) + throw new IllegalArgumentException(format("Invalid value for %s: '%s' do not parse as valid table properties", + name, diff)); - private Set validateIgnored(Set properties) - { - return validateTableProperties(properties, name + ".ignored"); - } - - private Set validateDisallowed(Set properties) - { - return validateTableProperties(properties, name + ".disallowed"); - } - - private Set validateTableProperties(Set properties, String name) - { - if (properties == null) - throw new IllegalArgumentException(format("Invalid value for %s: null is not allowed", name)); - - Set lowerCaseProperties = properties.stream().map(String::toLowerCase).collect(toSet()); - - Set diff = Sets.difference(lowerCaseProperties, TableAttributes.allKeywords()); - - if (!diff.isEmpty()) - throw new IllegalArgumentException(format("Invalid value for %s: '%s' do not parse as valid table properties", - name, diff)); - - return lowerCaseProperties; - } + return lowerCaseProperties; } } diff --git a/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java b/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java index 1d34514b5d..e8a3a2ccda 100644 --- a/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java +++ b/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java @@ -76,6 +76,6 @@ public class DisableFlag extends Guardrail public void ensureEnabled(String what, @Nullable ClientState state) { if (enabled(state) && disabled.test(state)) - abort(what + " is not allowed"); + fail(what + " is not allowed"); } } diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java index 749fddfe9a..74496e3400 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java @@ -68,7 +68,7 @@ public abstract class Guardrail Tracing.trace(message); } - protected void abort(String message) + protected void fail(String message) { logger.error(message); // Note that ClientWarn will simply ignore the message if we're not running this as part of a user query diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 00f3492332..0852c59a1b 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -47,8 +47,8 @@ public final class Guardrails implements GuardrailsMBean * 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(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspacesWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getKeyspacesFailThreshold(), (isWarning, what, value, threshold) -> isWarning ? format("Creating keyspace %s, current number of keyspaces %s exceeds warning threshold of %s.", what, value, threshold) @@ -59,8 +59,8 @@ public final class Guardrails implements GuardrailsMBean * Guardrail on the total number of tables on user keyspaces. */ public static final Threshold tables = - new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getTables().getWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getTables().getAbortThreshold(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getTablesWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getTablesFailThreshold(), (isWarning, what, value, threshold) -> isWarning ? format("Creating table %s, current number of tables %s exceeds warning threshold of %s.", what, value, threshold) @@ -71,8 +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().getWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTable().getAbortThreshold(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTableWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTableFailThreshold(), (isWarning, what, value, threshold) -> isWarning ? format("The table %s has %s columns, this exceeds the warning threshold of %s.", what, value, threshold) @@ -80,8 +80,8 @@ public final class Guardrails implements GuardrailsMBean threshold, value, what)); public static final Threshold secondaryIndexesPerTable = - new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTable().getWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTable().getAbortThreshold(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTableWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTableFailThreshold(), (isWarning, what, value, threshold) -> isWarning ? format("Creating secondary index %s, current number of indexes %s exceeds warning threshold of %s.", what, value, threshold) @@ -92,8 +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().getWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTable().getAbortThreshold(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTableWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTableFailThreshold(), (isWarning, what, value, threshold) -> isWarning ? format("Creating materialized view %s, current number of views %s exceeds warning threshold of %s.", what, value, threshold) @@ -104,8 +104,8 @@ public final class Guardrails implements GuardrailsMBean * Guardrail ignoring/disallowing the usage of certain table properties. */ public static final Values tableProperties = - new Values<>(state -> CONFIG_PROVIDER.getOrCreate(state).getTableProperties().getIgnored(), - state -> CONFIG_PROVIDER.getOrCreate(state).getTableProperties().getDisallowed(), + new Values<>(state -> CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesIgnored(), + state -> CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesDisallowed(), "Table Properties"); /** @@ -119,12 +119,12 @@ 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().getWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getPageSize().getAbortThreshold(), + new Threshold(state -> CONFIG_PROVIDER.getOrCreate(state).getPageSizeWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getPageSizeFailThreshold(), (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.", + : format("Aborting query for table %s, page size %s exceeds fail threshold of %s.", what, value, threshold)); /** @@ -165,103 +165,103 @@ public final class Guardrails implements GuardrailsMBean @Override public int getKeyspacesWarnThreshold() { - return DEFAULT_CONFIG.getKeyspaces().getWarnThreshold(); + return DEFAULT_CONFIG.getKeyspacesWarnThreshold(); } @Override - public int getKeyspacesAbortThreshold() + public int getKeyspacesFailThreshold() { - return DEFAULT_CONFIG.getKeyspaces().getAbortThreshold(); + return DEFAULT_CONFIG.getKeyspacesFailThreshold(); } @Override - public void setKeyspacesThreshold(int warn, int abort) + public void setKeyspacesThreshold(int warn, int fail) { - DEFAULT_CONFIG.getKeyspaces().setThresholds(warn, abort); + DEFAULT_CONFIG.setKeyspacesThreshold(warn, fail); } @Override public int getTablesWarnThreshold() { - return DEFAULT_CONFIG.getTables().getWarnThreshold(); + return DEFAULT_CONFIG.getTablesWarnThreshold(); } @Override - public int getTablesAbortThreshold() + public int getTablesFailThreshold() { - return DEFAULT_CONFIG.getTables().getAbortThreshold(); + return DEFAULT_CONFIG.getTablesFailThreshold(); } @Override - public void setTablesThreshold(int warn, int abort) + public void setTablesThreshold(int warn, int fail) { - DEFAULT_CONFIG.getTables().setThresholds(warn, abort); + DEFAULT_CONFIG.setTablesThreshold(warn, fail); } @Override public int getColumnsPerTableWarnThreshold() { - return DEFAULT_CONFIG.getColumnsPerTable().getWarnThreshold(); + return DEFAULT_CONFIG.getColumnsPerTableWarnThreshold(); } @Override - public int getColumnsPerTableAbortThreshold() + public int getColumnsPerTableFailThreshold() { - return DEFAULT_CONFIG.getColumnsPerTable().getAbortThreshold(); + return DEFAULT_CONFIG.getColumnsPerTableFailThreshold(); } @Override - public void setColumnsPerTableThreshold(int warn, int abort) + public void setColumnsPerTableThreshold(int warn, int fail) { - DEFAULT_CONFIG.getColumnsPerTable().setThresholds(warn, abort); + DEFAULT_CONFIG.setColumnsPerTableThreshold(warn, fail); } @Override public int getSecondaryIndexesPerTableWarnThreshold() { - return DEFAULT_CONFIG.getSecondaryIndexesPerTable().getWarnThreshold(); + return DEFAULT_CONFIG.getSecondaryIndexesPerTableWarnThreshold(); } @Override - public int getSecondaryIndexesPerTableAbortThreshold() + public int getSecondaryIndexesPerTableFailThreshold() { - return DEFAULT_CONFIG.getSecondaryIndexesPerTable().getAbortThreshold(); + return DEFAULT_CONFIG.getSecondaryIndexesPerTableFailThreshold(); } @Override - public void setSecondaryIndexesPerTableThreshold(int warn, int abort) + public void setSecondaryIndexesPerTableThreshold(int warn, int fail) { - DEFAULT_CONFIG.getSecondaryIndexesPerTable().setThresholds(warn, abort); + DEFAULT_CONFIG.setSecondaryIndexesPerTableThreshold(warn, fail); } @Override public int getMaterializedViewsPerTableWarnThreshold() { - return DEFAULT_CONFIG.getMaterializedViewsPerTable().getWarnThreshold(); + return DEFAULT_CONFIG.getMaterializedViewsPerTableWarnThreshold(); } @Override - public int getMaterializedViewsPerTableAbortThreshold() + public int getMaterializedViewsPerTableFailThreshold() { - return DEFAULT_CONFIG.getMaterializedViewsPerTable().getAbortThreshold(); + return DEFAULT_CONFIG.getMaterializedViewsPerTableFailThreshold(); } @Override - public void setMaterializedViewsPerTableThreshold(int warn, int abort) + public void setMaterializedViewsPerTableThreshold(int warn, int fail) { - DEFAULT_CONFIG.getMaterializedViewsPerTable().setThresholds(warn, abort); + DEFAULT_CONFIG.setMaterializedViewsPerTableThreshold(warn, fail); } @Override public Set getTablePropertiesDisallowed() { - return DEFAULT_CONFIG.getTableProperties().getDisallowed(); + return DEFAULT_CONFIG.getTablePropertiesDisallowed(); } @Override public String getTablePropertiesDisallowedCSV() { - return toCSV(DEFAULT_CONFIG.getTableProperties().getDisallowed()); + return toCSV(DEFAULT_CONFIG.getTablePropertiesDisallowed()); } public void setTablePropertiesDisallowed(String... properties) @@ -272,7 +272,7 @@ public final class Guardrails implements GuardrailsMBean @Override public void setTablePropertiesDisallowed(Set properties) { - DEFAULT_CONFIG.getTableProperties().setDisallowed(properties); + DEFAULT_CONFIG.setTablePropertiesDisallowed(properties); } @Override @@ -284,13 +284,13 @@ public final class Guardrails implements GuardrailsMBean @Override public Set getTablePropertiesIgnored() { - return DEFAULT_CONFIG.getTableProperties().getIgnored(); + return DEFAULT_CONFIG.getTablePropertiesIgnored(); } @Override public String getTablePropertiesIgnoredCSV() { - return toCSV(DEFAULT_CONFIG.getTableProperties().getIgnored()); + return toCSV(DEFAULT_CONFIG.getTablePropertiesIgnored()); } public void setTablePropertiesIgnored(String... properties) @@ -301,7 +301,7 @@ public final class Guardrails implements GuardrailsMBean @Override public void setTablePropertiesIgnored(Set properties) { - DEFAULT_CONFIG.getTableProperties().setIgnored(properties); + DEFAULT_CONFIG.setTablePropertiesIgnored(properties); } @Override @@ -325,19 +325,19 @@ public final class Guardrails implements GuardrailsMBean @Override public int getPageSizeWarnThreshold() { - return DEFAULT_CONFIG.getPageSize().getWarnThreshold(); + return DEFAULT_CONFIG.getPageSizeWarnThreshold(); } @Override - public int getPageSizeAbortThreshold() + public int getPageSizeFailThreshold() { - return DEFAULT_CONFIG.getPageSize().getAbortThreshold(); + return DEFAULT_CONFIG.getPageSizeFailThreshold(); } @Override - public void setPageSizeThreshold(int warn, int abort) + public void setPageSizeThreshold(int warn, int fail) { - DEFAULT_CONFIG.getPageSize().setThresholds(warn, abort); + DEFAULT_CONFIG.setPageSizeThreshold(warn, fail); } public boolean getReadBeforeWriteListOperationsEnabled() diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 2abbdb5536..4e46368fcf 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -30,7 +30,7 @@ import java.util.Set; *

This contains a main setting, {@code enabled}, controlling if guardrails are globally active or not, and * individual settings to control each guardrail. * - *

We have 2 variants of guardrails, soft (warn) and hard (abort) limits, each guardrail having either one of the + *

We have 2 variants of guardrails, soft (warn) and hard (fail) limits, each guardrail having either one of the * variants or both. Note in particular that hard limits only make sense for guardrails triggering during query * execution. For other guardrails, say one triggering during compaction, aborting that compaction does not make sense. * @@ -51,34 +51,64 @@ public interface GuardrailsConfig boolean getEnabled(); /** - * @return The threshold to warn or abort when creating more user keyspaces than threshold. + * @return The threshold to warn when creating more user keyspaces than threshold. */ - IntThreshold getKeyspaces(); + int getKeyspacesWarnThreshold(); /** - * @return The threshold to warn or abort when creating more user tables than threshold. + * @return The threshold to fail when creating more user keyspaces than threshold. */ - IntThreshold getTables(); + int getKeyspacesFailThreshold(); /** - * @return The threshold to warn or abort when creating more columns per table than threshold. + * @return The threshold to warn when creating more user tables than threshold. */ - IntThreshold getColumnsPerTable(); + int getTablesWarnThreshold(); /** - * @return The threshold to warn or abort when creating more secondary indexes per table than threshold. + * @return The threshold to fail when creating more user tables than threshold. */ - IntThreshold getSecondaryIndexesPerTable(); + int getTablesFailThreshold(); /** - * @return The threshold to warn or abort when creating more materialized views per table than threshold. + * @return The threshold to warn when creating more columns per table than threshold. */ - IntThreshold getMaterializedViewsPerTable(); + int getColumnsPerTableWarnThreshold(); /** - * @return The table properties that are ignored/disallowed when creating or altering a table. + * @return The threshold to fail when creating more columns per table than threshold. */ - TableProperties getTableProperties(); + int getColumnsPerTableFailThreshold(); + + /** + * @return The threshold to warn when creating more secondary indexes per table than threshold. + */ + int getSecondaryIndexesPerTableWarnThreshold(); + + /** + * @return The threshold to fail when creating more secondary indexes per table than threshold. + */ + int getSecondaryIndexesPerTableFailThreshold(); + + /** + * @return The threshold to warn when creating more materialized views per table than threshold. + */ + int getMaterializedViewsPerTableWarnThreshold(); + + /** + * @return The threshold to fail when creating more materialized views per table than threshold. + */ + int getMaterializedViewsPerTableFailThreshold(); + + /** + * @return The table properties that are ignored when creating or altering a table. + */ + Set getTablePropertiesIgnored(); + + /** + * @return The table properties that are disallowed when creating or altering a table. + */ + Set getTablePropertiesDisallowed(); /** * Returns whether user-provided timestamps are allowed. @@ -88,9 +118,14 @@ public interface GuardrailsConfig boolean getUserTimestampsEnabled(); /** - * @return The threshold to warn or abort when page size exceeds given size. + * @return The threshold to warn when page size exceeds given size. */ - IntThreshold getPageSize(); + int getPageSizeWarnThreshold(); + + /** + * @return The threshold to fail when page size exceeds given size. + */ + int getPageSizeFailThreshold(); /** * Returns whether list operations that require read before write are allowed. @@ -98,37 +133,4 @@ public interface GuardrailsConfig * @return {@code true} if list operations that require read before write are allowed, {@code false} otherwise. */ boolean getReadBeforeWriteListOperationsEnabled(); - - /** - * 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 getIgnored(); - - /** - * @return The values to be rejected. - */ - Set getDisallowed(); - } } diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 197adddec2..aa0cd46f01 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -26,9 +26,9 @@ import java.util.Set; * This is different to just exposing {@link GuardrailsConfig} in that the methods here should be JMX-friendly. * *

For consistency, guardrails based on a simple numeric threshold should use the naming scheme - * {@code WarnThreshold} for soft limits and {@code AbortThreshold} for hard + * {@code WarnThreshold} for soft limits and {@code FailThreshold} for hard * ones, and if the value has a unit, that unit should be added at the end (for instance, - * {@code AbortThresholdInKb}). For "boolean" guardrails that disable a feature, use + * {@code FailThresholdInKb}). For "boolean" guardrails that disable a feature, use * {@code }. */ @@ -58,13 +58,13 @@ public interface GuardrailsMBean * @return The threshold to prevent creating more user keyspaces than threshold. * -1 means disabled. */ - int getKeyspacesAbortThreshold(); + int getKeyspacesFailThreshold(); /** * @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. + * @param fail The threshold to prevent creating more user keyspaces than threshold. -1 means disabled. */ - void setKeyspacesThreshold(int warn, int abort); + void setKeyspacesThreshold(int warn, int fail); /** * @return The threshold to warn when creating more tables than threshold. @@ -76,13 +76,13 @@ public interface GuardrailsMBean * @return The threshold to prevent creating more tables than threshold. * -1 means disabled. */ - int getTablesAbortThreshold(); + int getTablesFailThreshold(); /** * @param warn The threshold to warn when creating more tables than threshold. -1 means disabled. - * @param abort The threshold to prevent creating more tables than threshold. -1 means disabled. + * @param fail The threshold to prevent creating more tables than threshold. -1 means disabled. */ - void setTablesThreshold(int warn, int abort); + void setTablesThreshold(int warn, int fail); /** * @return The threshold to warn when having more columns per table than threshold. @@ -93,13 +93,13 @@ public interface GuardrailsMBean /** * @return The threshold to prevent having more columns per table than threshold. -1 means disabled. */ - int getColumnsPerTableAbortThreshold(); + int getColumnsPerTableFailThreshold(); /** * @param warn The threshold to warn when having more columns per table than threshold. -1 means disabled. - * @param abort The threshold to prevent having more columns per table than threshold. -1 means disabled. + * @param fail The threshold to prevent having more columns per table than threshold. -1 means disabled. */ - void setColumnsPerTableThreshold(int warn, int abort); + void setColumnsPerTableThreshold(int warn, int fail); /** * @return The threshold to warn when creating more secondary indexes per table than threshold. -1 means disabled. @@ -109,13 +109,13 @@ public interface GuardrailsMBean /** * @return The threshold to prevent creating more secondary indexes per table than threshold. -1 means disabled. */ - int getSecondaryIndexesPerTableAbortThreshold(); + int getSecondaryIndexesPerTableFailThreshold(); /** * @param warn The threshold to warn when creating more secondary indexes per table than threshold. -1 means disabled. - * @param abort The threshold to prevent creating more secondary indexes per table than threshold. -1 means disabled. + * @param fail The threshold to prevent creating more secondary indexes per table than threshold. -1 means disabled. */ - void setSecondaryIndexesPerTableThreshold(int warn, int abort); + void setSecondaryIndexesPerTableThreshold(int warn, int fail); /** * @return The threshold to warn when creating more materialized views per table than threshold. @@ -127,13 +127,13 @@ public interface GuardrailsMBean * @return The threshold to prevent creating more materialized views per table than threshold. * -1 means disabled. */ - int getMaterializedViewsPerTableAbortThreshold(); + int getMaterializedViewsPerTableFailThreshold(); /** * @param warn The threshold to warn when creating more materialized views per table than threshold. -1 means disabled. - * @param abort The threshold to prevent creating more materialized views per table than threshold. -1 means disabled. + * @param fail The threshold to prevent creating more materialized views per table than threshold. -1 means disabled. */ - void setMaterializedViewsPerTableThreshold(int warn, int abort); + void setMaterializedViewsPerTableThreshold(int warn, int fail); /** * @return properties that are not allowed when creating or altering a table. @@ -199,13 +199,13 @@ public interface GuardrailsMBean * @return The threshold to prevent requesting page with more elements than threshold. * -1 means disabled. */ - int getPageSizeAbortThreshold(); + int getPageSizeFailThreshold(); /** * @param warn The threshold to warn when the requested page size is greater than threshold. -1 means disabled. - * @param abort The threshold to prevent requesting pages with more elements than threshold. -1 means disabled. + * @param fail The threshold to prevent requesting pages with more elements than threshold. -1 means disabled. */ - void setPageSizeThreshold(int warn, int abort); + void setPageSizeThreshold(int warn, int fail); /** * Returns whether list operations that require read before write are allowed. diff --git a/src/java/org/apache/cassandra/db/guardrails/Threshold.java b/src/java/org/apache/cassandra/db/guardrails/Threshold.java index d0fd8ebb70..ccf7ec5f03 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Threshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/Threshold.java @@ -34,22 +34,22 @@ import org.apache.cassandra.service.ClientState; public class Threshold extends Guardrail { private final ToLongFunction warnThreshold; - private final ToLongFunction abortThreshold; + private final ToLongFunction failThreshold; private final ErrorMessageProvider messageProvider; /** * Creates a new threshold guardrail. * * @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 failThreshold 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(ToLongFunction warnThreshold, - ToLongFunction abortThreshold, + ToLongFunction failThreshold, ErrorMessageProvider messageProvider) { this.warnThreshold = warnThreshold; - this.abortThreshold = abortThreshold; + this.failThreshold = failThreshold; this.messageProvider = messageProvider; } @@ -61,10 +61,10 @@ public class Threshold extends Guardrail thresholdValue); } - private long abortValue(ClientState state) + private long failValue(ClientState state) { - long abortValue = abortThreshold.applyAsLong(state); - return abortValue < 0 ? Long.MAX_VALUE : abortValue; + long failValue = failThreshold.applyAsLong(state); + return failValue < 0 ? Long.MAX_VALUE : failValue; } private long warnValue(ClientState state) @@ -79,11 +79,11 @@ public class Threshold extends Guardrail if (!super.enabled(state)) return false; - return abortThreshold.applyAsLong(state) >= 0 || warnThreshold.applyAsLong(state) >= 0; + return failThreshold.applyAsLong(state) >= 0 || warnThreshold.applyAsLong(state) >= 0; } /** - * Apply the guardrail to the provided value, warning or aborting if appropriate. + * Apply the guardrail to the provided value, warning or failing if appropriate. * * @param value The value to check. * @param what A string describing what {@code value} is a value of. This is used in the error message if the @@ -97,10 +97,10 @@ public class Threshold extends Guardrail if (!enabled(state)) return; - long abortValue = abortValue(state); - if (value > abortValue) + long failValue = failValue(state); + if (value > failValue) { - triggerAbort(value, abortValue, what); + triggerFail(value, failValue, what); return; } @@ -109,9 +109,9 @@ public class Threshold extends Guardrail triggerWarn(value, warnValue, what); } - private void triggerAbort(long value, long abortValue, String what) + private void triggerFail(long value, long failValue, String what) { - abort(errMsg(false, what, value, abortValue)); + fail(errMsg(false, what, value, failValue)); } private void triggerWarn(long value, long warnValue, String what) @@ -127,7 +127,7 @@ public class Threshold extends Guardrail /** * Called when the guardrail is triggered to build the corresponding error message. * - * @param isWarning Whether the trigger is a warning one; otherwise it is an abort one. + * @param isWarning Whether the trigger is a warning one; otherwise it is a failure one. * @param what A string, provided by the call to the {@link #guard} method, describing what the guardrail * has been applied to (and that has triggered it). * @param value The value that triggered the guardrail (as a string). diff --git a/src/java/org/apache/cassandra/db/guardrails/Values.java b/src/java/org/apache/cassandra/db/guardrails/Values.java index 66508a14fb..f1a62bf338 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Values.java +++ b/src/java/org/apache/cassandra/db/guardrails/Values.java @@ -75,8 +75,8 @@ public class Values extends Guardrail Set disallowed = disallowedValues.apply(state); Set 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)); + fail(format("Provided values %s are not allowed for %s (disallowed values are: %s)", + toDisallow.stream().sorted().collect(Collectors.toList()), what, disallowed)); Set ignored = ignoredValues.apply(state); Set toIgnore = Sets.intersection(values, ignored); diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java index cd2338b14b..84f416fc5e 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java @@ -20,8 +20,6 @@ package org.apache.cassandra.db.guardrails; import org.junit.Test; -import org.apache.cassandra.config.DatabaseDescriptor; - import static java.lang.String.format; /** @@ -30,16 +28,16 @@ import static java.lang.String.format; public class GuardrailColumnsPerTableTest extends ThresholdTester { private static final int COLUMNS_PER_TABLE_WARN_THRESHOLD = 2; - private static final int COLUMNS_PER_TABLE_ABORT_THRESHOLD = 4; + private static final int COLUMNS_PER_TABLE_FAIL_THRESHOLD = 4; public GuardrailColumnsPerTableTest() { super(COLUMNS_PER_TABLE_WARN_THRESHOLD, - COLUMNS_PER_TABLE_ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getColumnsPerTable(), + COLUMNS_PER_TABLE_FAIL_THRESHOLD, + "columns_per_table", Guardrails::setColumnsPerTableThreshold, Guardrails::getColumnsPerTableWarnThreshold, - Guardrails::getColumnsPerTableAbortThreshold); + Guardrails::getColumnsPerTableFailThreshold); } @Override @@ -55,39 +53,39 @@ public class GuardrailColumnsPerTableTest extends ThresholdTester assertCreateTableValid("CREATE TABLE %s (k1 int, v int, PRIMARY KEY((k1)))"); assertCreateTableWarns(3, "CREATE TABLE %s (k1 int, k2 int, v int, PRIMARY KEY((k1, k2)))"); assertCreateTableWarns(4, "CREATE TABLE %s (k1 int, k2 int, k3 int, v int, PRIMARY KEY((k1, k2, k3)))"); - assertCreateTableAborts(5, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, v int, PRIMARY KEY((k1, k2, k3, k4)))"); - assertCreateTableAborts(6, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, k5 int, v int, PRIMARY KEY((k1, k2, k3, k4, k5)))"); + assertCreateTableFails(5, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, v int, PRIMARY KEY((k1, k2, k3, k4)))"); + assertCreateTableFails(6, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, k5 int, v int, PRIMARY KEY((k1, k2, k3, k4, k5)))"); // partition key on wide table assertCreateTableWarns(3, "CREATE TABLE %s (k1 int, c int, v int, PRIMARY KEY(k1, c))"); assertCreateTableWarns(4, "CREATE TABLE %s (k1 int, k2 int, c int, v int, PRIMARY KEY((k1, k2), c))"); - assertCreateTableAborts(5, "CREATE TABLE %s (k1 int, k2 int, k3 int, c int, v int, PRIMARY KEY((k1, k2, k3), c))"); - assertCreateTableAborts(6, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, c int, v int, PRIMARY KEY((k1, k2, k3, k4), c))"); + assertCreateTableFails(5, "CREATE TABLE %s (k1 int, k2 int, k3 int, c int, v int, PRIMARY KEY((k1, k2, k3), c))"); + assertCreateTableFails(6, "CREATE TABLE %s (k1 int, k2 int, k3 int, k4 int, c int, v int, PRIMARY KEY((k1, k2, k3, k4), c))"); // clustering key assertCreateTableWarns(3, "CREATE TABLE %s (k int, c1 int, v int, PRIMARY KEY(k, c1))"); assertCreateTableWarns(4, "CREATE TABLE %s (k int, c1 int, c2 int, v int, PRIMARY KEY(k, c1, c2))"); - assertCreateTableAborts(5, "CREATE TABLE %s (k int, c1 int, c2 int, c3 int, v int, PRIMARY KEY(k, c1, c2, c3))"); - assertCreateTableAborts(6, "CREATE TABLE %s (k int, c1 int, c2 int, c3 int, c4 int, v int, PRIMARY KEY(k, c1, c2, c3, c4))"); + assertCreateTableFails(5, "CREATE TABLE %s (k int, c1 int, c2 int, c3 int, v int, PRIMARY KEY(k, c1, c2, c3))"); + assertCreateTableFails(6, "CREATE TABLE %s (k int, c1 int, c2 int, c3 int, c4 int, v int, PRIMARY KEY(k, c1, c2, c3, c4))"); // static column assertCreateTableWarns(3, "CREATE TABLE %s (k int, c int, s1 int STATIC, PRIMARY KEY(k, c))"); assertCreateTableWarns(4, "CREATE TABLE %s (k int, c int, s1 int STATIC, s2 int STATIC, PRIMARY KEY(k, c))"); - assertCreateTableAborts(5, "CREATE TABLE %s (k int, c int, s1 int STATIC, s2 int STATIC, s3 int STATIC, PRIMARY KEY(k, c))"); - assertCreateTableAborts(6, "CREATE TABLE %s (k int, c int, s1 int STATIC, s2 int STATIC, s3 int STATIC, s4 int STATIC, PRIMARY KEY(k, c))"); + assertCreateTableFails(5, "CREATE TABLE %s (k int, c int, s1 int STATIC, s2 int STATIC, s3 int STATIC, PRIMARY KEY(k, c))"); + assertCreateTableFails(6, "CREATE TABLE %s (k int, c int, s1 int STATIC, s2 int STATIC, s3 int STATIC, s4 int STATIC, PRIMARY KEY(k, c))"); // regular column on skinny table assertCreateTableValid("CREATE TABLE %s (k int PRIMARY KEY, v1 int)"); assertCreateTableWarns(3, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int)"); assertCreateTableWarns(4, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int)"); - assertCreateTableAborts(5, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int, v4 int)"); - assertCreateTableAborts(6, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int, v4 int, v5 int)"); + assertCreateTableFails(5, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int, v4 int)"); + assertCreateTableFails(6, "CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int, v4 int, v5 int)"); // regular column on wide table assertCreateTableWarns(3, "CREATE TABLE %s (k int, c int, v1 int, PRIMARY KEY(k, c))"); assertCreateTableWarns(4, "CREATE TABLE %s (k int, c int, v1 int, v2 int, PRIMARY KEY(k, c))"); - assertCreateTableAborts(5, "CREATE TABLE %s (k int, c int, v1 int, v2 int, v3 int, PRIMARY KEY(k, c))"); - assertCreateTableAborts(6, "CREATE TABLE %s (k int, c int, v1 int, v2 int, v3 int, v4 int, PRIMARY KEY(k, c))"); + assertCreateTableFails(5, "CREATE TABLE %s (k int, c int, v1 int, v2 int, v3 int, PRIMARY KEY(k, c))"); + assertCreateTableFails(6, "CREATE TABLE %s (k int, c int, v1 int, v2 int, v3 int, v4 int, PRIMARY KEY(k, c))"); // udt String udt = createType("CREATE TYPE %s (a int, b int, c int, d int)"); @@ -101,16 +99,16 @@ public class GuardrailColumnsPerTableTest extends ThresholdTester createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 int)"); assertAddColumnWarns("ALTER TABLE %s ADD v2 int"); assertAddColumnWarns("ALTER TABLE %s ADD v3 int"); - assertAddColumnAborts("ALTER TABLE %s ADD v4 int"); + assertAddColumnFails("ALTER TABLE %s ADD v4 int"); // skinny table at threshold createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 int, v2 int, v3 int)"); - assertAddColumnAborts("ALTER TABLE %s ADD v4 int"); + assertAddColumnFails("ALTER TABLE %s ADD v4 int"); // wide table createTable("CREATE TABLE %s (k int, c int, v1 int, v2 int, PRIMARY KEY(k, c))"); - assertAddColumnAborts("ALTER TABLE %s ADD v3 int"); - assertAddColumnAborts("ALTER TABLE %s ADD s int STATIC"); + assertAddColumnFails("ALTER TABLE %s ADD v3 int"); + assertAddColumnFails("ALTER TABLE %s ADD s int STATIC"); // udt createTable("CREATE TABLE %s (k int PRIMARY KEY, v1 int)"); @@ -128,7 +126,7 @@ public class GuardrailColumnsPerTableTest extends ThresholdTester guardrails().setColumnsPerTableThreshold(2, 2); assertDropColumnValid("ALTER TABLE %s DROP v2"); assertDropColumnValid("ALTER TABLE %s DROP v3"); - assertAddColumnAborts("ALTER TABLE %s ADD v2 int"); + assertAddColumnFails("ALTER TABLE %s ADD v2 int"); } @Test @@ -169,22 +167,22 @@ public class GuardrailColumnsPerTableTest extends ThresholdTester format(query, keyspace() + '.' + tableName)); } - private void assertAddColumnAborts(String query) throws Throwable + private void assertAddColumnFails(String query) throws Throwable { - assertAborts(currentValue() + 1, query, currentTable()); + assertFails(currentValue() + 1, query, currentTable()); } - private void assertCreateTableAborts(long numColumns, String query) throws Throwable + private void assertCreateTableFails(long numColumns, String query) throws Throwable { - assertAborts(numColumns, query, ABORT_TABLE); + assertFails(numColumns, query, FAIL_TABLE); } - private void assertAborts(long numColumns, String query, String tableName) throws Throwable + private void assertFails(long numColumns, String query, String tableName) throws Throwable { - assertThresholdAborts(format("Tables cannot have more than %s columns, but %s provided for table %s", - guardrails().getColumnsPerTableAbortThreshold(), - numColumns, - tableName), - format(query, keyspace() + '.' + tableName)); + assertThresholdFails(format("Tables cannot have more than %s columns, but %s provided for table %s", + guardrails().getColumnsPerTableFailThreshold(), + numColumns, + tableName), + format(query, keyspace() + '.' + tableName)); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java index 281b197b8e..c4ffec1598 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java @@ -20,7 +20,6 @@ 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; @@ -31,16 +30,16 @@ import static java.lang.String.format; 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; + private static final int FAIL_THRESHOLD = WARN_THRESHOLD + 1; public GuardrailKeyspacesTest() { super(WARN_THRESHOLD, - ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getKeyspaces(), + FAIL_THRESHOLD, + "keyspaces", Guardrails::setKeyspacesThreshold, Guardrails::getKeyspacesWarnThreshold, - Guardrails::getKeyspacesAbortThreshold); + Guardrails::getKeyspacesFailThreshold); } @Override @@ -52,22 +51,22 @@ public class GuardrailKeyspacesTest extends ThresholdTester @Test public void testCreateKeyspace() throws Throwable { - // create keyspaces until hitting the two warn/abort thresholds + // create keyspaces until hitting the two warn/fail thresholds String k1 = assertCreateKeyspaceValid(); String k2 = assertCreateKeyspaceWarns(); - assertCreateKeyspaceAborts(); + assertCreateKeyspaceFails(); - // drop a keyspace and hit the warn/abort threshold again + // drop a keyspace and hit the warn/fail threshold again dropKeyspace(k2); String k3 = assertCreateKeyspaceWarns(); - assertCreateKeyspaceAborts(); + assertCreateKeyspaceFails(); - // drop two keyspaces and hit the warn/abort threshold again + // drop two keyspaces and hit the warn/fail threshold again dropKeyspace(k1); dropKeyspace(k3); assertCreateKeyspaceValid(); assertCreateKeyspaceWarns(); - assertCreateKeyspaceAborts(); + assertCreateKeyspaceFails(); // test excluded users testExcludedUsers(this::createKeyspaceQuery, @@ -96,12 +95,12 @@ public class GuardrailKeyspacesTest extends ThresholdTester return keyspaceName; } - private void assertCreateKeyspaceAborts() throws Throwable + private void assertCreateKeyspaceFails() throws Throwable { String keyspaceName = createKeyspaceName(); - assertThresholdAborts(format("Cannot have more than %d keyspaces, aborting the creation of keyspace %s", - ABORT_THRESHOLD, keyspaceName), - createKeyspaceQuery(keyspaceName)); + assertThresholdFails(format("Cannot have more than %d keyspaces, aborting the creation of keyspace %s", + FAIL_THRESHOLD, keyspaceName), + createKeyspaceQuery(keyspaceName)); } private String createKeyspaceQuery() diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java index 4873c5790b..ba53a17c0f 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java @@ -23,7 +23,6 @@ import java.util.Collections; import org.junit.Before; import org.junit.Test; -import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CQLStatement; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.QueryProcessor; @@ -40,16 +39,16 @@ import static java.lang.String.format; public class GuardrailPageSizeTest extends ThresholdTester { private static final int PAGE_SIZE_WARN_THRESHOLD = 5; - private static final int PAGE_SIZE_ABORT_THRESHOLD = 10; + private static final int PAGE_SIZE_FAIL_THRESHOLD = 10; public GuardrailPageSizeTest() { super(PAGE_SIZE_WARN_THRESHOLD, - PAGE_SIZE_ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getPageSize(), + PAGE_SIZE_FAIL_THRESHOLD, + "page_size", Guardrails::setPageSizeThreshold, Guardrails::getPageSizeWarnThreshold, - Guardrails::getPageSizeAbortThreshold); + Guardrails::getPageSizeFailThreshold); } @Before @@ -66,31 +65,31 @@ public class GuardrailPageSizeTest extends ThresholdTester assertPagingValid(query, 3); assertPagingValid(query, PAGE_SIZE_WARN_THRESHOLD); assertPagingWarns(query, 6); - assertPagingWarns(query, PAGE_SIZE_ABORT_THRESHOLD); - assertPagingAborts(query, 11); + assertPagingWarns(query, PAGE_SIZE_FAIL_THRESHOLD); + assertPagingFails(query, 11); // aggregation query query = "SELECT COUNT(*) FROM %s WHERE k=0"; assertPagingValid(query, 3); assertPagingValid(query, PAGE_SIZE_WARN_THRESHOLD); assertPagingWarns(query, 6); - assertPagingWarns(query, PAGE_SIZE_ABORT_THRESHOLD); - assertPagingAborts(query, 11); + assertPagingWarns(query, PAGE_SIZE_FAIL_THRESHOLD); + assertPagingFails(query, 11); // query with limit over thresholds query = "SELECT * FROM %s LIMIT 100"; assertPagingValid(query, 3); assertPagingValid(query, PAGE_SIZE_WARN_THRESHOLD); assertPagingWarns(query, 6); - assertPagingWarns(query, PAGE_SIZE_ABORT_THRESHOLD); - assertPagingAborts(query, 11); + assertPagingWarns(query, PAGE_SIZE_FAIL_THRESHOLD); + assertPagingFails(query, 11); // query with limit under thresholds query = "SELECT * FROM %s LIMIT 1"; assertPagingValid(query, 3); assertPagingValid(query, PAGE_SIZE_WARN_THRESHOLD); assertPagingValid(query, 6); - assertPagingValid(query, PAGE_SIZE_ABORT_THRESHOLD); + assertPagingValid(query, PAGE_SIZE_FAIL_THRESHOLD); assertPagingValid(query, 11); } @@ -98,7 +97,7 @@ public class GuardrailPageSizeTest extends ThresholdTester public void testExcludedUsers() throws Throwable { assertPagingIgnored("SELECT * FROM %s", PAGE_SIZE_WARN_THRESHOLD + 1); - assertPagingIgnored("SELECT * FROM %s", PAGE_SIZE_ABORT_THRESHOLD + 1); + assertPagingIgnored("SELECT * FROM %s", PAGE_SIZE_FAIL_THRESHOLD + 1); } private void assertPagingValid(String query, int pageSize) throws Throwable @@ -119,11 +118,11 @@ public class GuardrailPageSizeTest extends ThresholdTester currentTable(), pageSize, PAGE_SIZE_WARN_THRESHOLD)); } - private void assertPagingAborts(String query, int pageSize) throws Throwable + private void assertPagingFails(String query, int pageSize) throws Throwable { - assertAborts(() -> executeWithPaging(userClientState, query, pageSize), - format("Aborting query for table %s, page size %s exceeds abort threshold of %s.", - currentTable(), pageSize, PAGE_SIZE_ABORT_THRESHOLD)); + assertFails(() -> executeWithPaging(userClientState, query, pageSize), + format("Aborting query for table %s, page size %s exceeds fail threshold of %s.", + currentTable(), pageSize, PAGE_SIZE_FAIL_THRESHOLD)); } private void executeWithPaging(ClientState state, String query, int pageSize) diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java index a4049c7e07..80d1bef47d 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java @@ -166,7 +166,7 @@ public class GuardrailReadBeforeWriteListOperationsTest extends GuardrailTester } else { - assertAborts(expectedMessage, query); + assertFails(expectedMessage, query); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java index debbfb1ded..7496bacb8a 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java @@ -21,8 +21,6 @@ package org.apache.cassandra.db.guardrails; import com.google.common.base.Strings; import org.junit.Test; -import org.apache.cassandra.config.DatabaseDescriptor; - import static java.lang.String.format; /** @@ -31,16 +29,16 @@ import static java.lang.String.format; public class GuardrailSecondaryIndexesPerTable extends ThresholdTester { private static final int INDEXES_PER_TABLE_WARN_THRESHOLD = 1; - private static final int INDEXES_PER_TABLE_ABORT_THRESHOLD = 3; + private static final int INDEXES_PER_TABLE_FAIL_THRESHOLD = 3; public GuardrailSecondaryIndexesPerTable() { super(INDEXES_PER_TABLE_WARN_THRESHOLD, - INDEXES_PER_TABLE_ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getSecondaryIndexesPerTable(), + INDEXES_PER_TABLE_FAIL_THRESHOLD, + "secondary_indexes_per_table", Guardrails::setSecondaryIndexesPerTableThreshold, Guardrails::getSecondaryIndexesPerTableWarnThreshold, - Guardrails::getSecondaryIndexesPerTableAbortThreshold); + Guardrails::getSecondaryIndexesPerTableFailThreshold); } @Override @@ -58,19 +56,19 @@ public class GuardrailSecondaryIndexesPerTable extends ThresholdTester assertCreateIndexWarns("v2", ""); assertCreateIndexWarns("v3", "v3_idx"); - assertCreateIndexAborts("v4", ""); - assertCreateIndexAborts("v2", "v2_idx"); + assertCreateIndexFails("v4", ""); + assertCreateIndexFails("v2", "v2_idx"); assertCurrentValue(3); // 2i guardrail will also affect custom indexes - assertCreateCustomIndexAborts("v2"); + assertCreateCustomIndexFails("v2"); // drop the two first indexes, we should be able to create new indexes again dropIndex(format("DROP INDEX %s.%s", keyspace(), "v3_idx")); assertCurrentValue(2); assertCreateIndexWarns("v3", ""); - assertCreateCustomIndexAborts("v4"); + assertCreateCustomIndexFails("v4"); assertCurrentValue(3); // previous guardrail should not apply to another base table @@ -78,7 +76,7 @@ public class GuardrailSecondaryIndexesPerTable extends ThresholdTester assertCreateIndexSucceeds("v4", ""); assertCreateIndexWarns("v3", ""); assertCreateIndexWarns("v2", ""); - assertCreateIndexAborts("v1", ""); + assertCreateIndexFails("v1", ""); assertCurrentValue(3); } @@ -107,16 +105,16 @@ public class GuardrailSecondaryIndexesPerTable extends ThresholdTester format("CREATE INDEX %s ON %%s(%s)", indexName, column)); } - private void assertCreateIndexAborts(String column, String indexName) throws Throwable + private void assertCreateIndexFails(String column, String indexName) throws Throwable { - assertThresholdAborts(format("aborting the creation of secondary index %son table %s", + assertThresholdFails(format("aborting the creation of secondary index %son table %s", Strings.isNullOrEmpty(indexName) ? "" : indexName + " ", currentTable()), - format("CREATE INDEX %s ON %%s(%s)", indexName, column)); + format("CREATE INDEX %s ON %%s(%s)", indexName, column)); } - private void assertCreateCustomIndexAborts(String column) throws Throwable + private void assertCreateCustomIndexFails(String column) throws Throwable { - assertThresholdAborts(format("aborting the creation of secondary index on table %s", currentTable()), - format("CREATE CUSTOM INDEX ON %%s (%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'", column)); + assertThresholdFails(format("aborting the creation of secondary index on table %s", currentTable()), + format("CREATE CUSTOM INDEX ON %%s (%s) USING 'org.apache.cassandra.index.sasi.SASIIndex'", column)); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java index c4ea369400..03e94dca10 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablePropertiesTest.java @@ -30,7 +30,6 @@ 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; @@ -46,9 +45,8 @@ 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"; + private static final String IGNORED_PROPERTY_NAME = "table_properties_ignored"; + private static final String DISALLOWED_PROPERTY_NAME = "table_properties_disallowed"; @Before public void before() @@ -97,11 +95,11 @@ public class GuardrailTablePropertiesTest extends GuardrailTester { // most table properties are not allowed assertValid(this::createTableWithProperties); - assertAborts(() -> createTableWithProperties("with id = " + UUID.randomUUID()), "[id]"); - assertAborts(() -> createTableWithProperties("with compression = { 'enabled': 'false' }"), "[compression]"); - assertAborts(() -> createTableWithProperties("with compression = { 'enabled': 'false' } AND id = " + UUID.randomUUID()), "[compression, id]"); - assertAborts(() -> createTableWithProperties("with compaction = { 'class': 'SizeTieredCompactionStrategy' }"), "[compaction]"); - assertAborts(() -> createTableWithProperties("with gc_grace_seconds = 1000 and compression = { 'enabled': 'false' }"), "[compression]"); + assertFails(() -> createTableWithProperties("with id = " + UUID.randomUUID()), "[id]"); + assertFails(() -> createTableWithProperties("with compression = { 'enabled': 'false' }"), "[compression]"); + assertFails(() -> createTableWithProperties("with compression = { 'enabled': 'false' } AND id = " + UUID.randomUUID()), "[compression, id]"); + assertFails(() -> createTableWithProperties("with compaction = { 'class': 'SizeTieredCompactionStrategy' }"), "[compaction]"); + assertFails(() -> createTableWithProperties("with gc_grace_seconds = 1000 and compression = { 'enabled': 'false' }"), "[compression]"); // though gc_grace_seconds alone is assertValid(() -> createTableWithProperties("with gc_grace_seconds = 1000")); @@ -127,15 +125,15 @@ public class GuardrailTablePropertiesTest extends GuardrailTester // view properties is not allowed createTableWithProperties(); assertValid(() -> createViewWithProperties("")); - assertAborts(() -> createViewWithProperties("with compression = { 'enabled': 'false' }"), "[compression]"); + assertFails(() -> createViewWithProperties("with compression = { 'enabled': 'false' }"), "[compression]"); assertValid(() -> createViewWithProperties("with gc_grace_seconds = 1000")); // alter mv properties except "gc_grace_seconds" is not allowed assertValid(() -> alterViewWithProperties("gc_grace_seconds = 1000")); - assertAborts(() -> alterViewWithProperties("compaction = { 'class': 'SizeTieredCompactionStrategy' } AND default_time_to_live = 1"), - "[compaction, default_time_to_live]"); - assertAborts(() -> alterViewWithProperties("compaction = { 'class': 'SizeTieredCompactionStrategy' } AND crc_check_chance = 1"), - "[compaction, crc_check_chance]"); + assertFails(() -> alterViewWithProperties("compaction = { 'class': 'SizeTieredCompactionStrategy' } AND default_time_to_live = 1"), + "[compaction, default_time_to_live]"); + assertFails(() -> alterViewWithProperties("compaction = { 'class': 'SizeTieredCompactionStrategy' } AND crc_check_chance = 1"), + "[compaction, crc_check_chance]"); } @Test diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java index 7a32b78eab..83d0203dc2 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java @@ -20,7 +20,6 @@ package org.apache.cassandra.db.guardrails; import org.junit.Test; -import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.Keyspace; import static java.lang.String.format; @@ -31,16 +30,16 @@ import static java.lang.String.format; public class GuardrailTablesTest extends ThresholdTester { private static final int TABLES_LIMIT_WARN_THRESHOLD = 1; - private static final int TABLES_LIMIT_ABORT_THRESHOLD = 2; + private static final int TABLES_LIMIT_FAIL_THRESHOLD = 2; public GuardrailTablesTest() { super(TABLES_LIMIT_WARN_THRESHOLD, - TABLES_LIMIT_ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getTables(), + TABLES_LIMIT_FAIL_THRESHOLD, + "tables", Guardrails::setTablesThreshold, Guardrails::getTablesWarnThreshold, - Guardrails::getTablesAbortThreshold); + Guardrails::getTablesFailThreshold); } @Override @@ -52,22 +51,22 @@ public class GuardrailTablesTest extends ThresholdTester @Test public void testCreateTable() throws Throwable { - // create tables until hitting the two warn/abort thresholds + // create tables until hitting the two warn/fail thresholds String t1 = assertCreateTableValid(); String t2 = assertCreateTableWarns(); - assertCreateTableAborts(); + assertCreateTableFails(); - // drop a table and hit the warn/abort threshold again + // drop a table and hit the warn/fail threshold again dropTable(t2); String t3 = assertCreateTableWarns(); - assertCreateTableAborts(); + assertCreateTableFails(); - // drop two tables and hit the warn/abort threshold again + // drop two tables and hit the warn/fail threshold again dropTable(t1); dropTable(t3); assertCreateTableValid(); assertCreateTableWarns(); - assertCreateTableAborts(); + assertCreateTableFails(); // test excluded users testExcludedUsers(this::createTableQuery, @@ -96,11 +95,11 @@ public class GuardrailTablesTest extends ThresholdTester return tableName; } - private void assertCreateTableAborts() throws Throwable + private void assertCreateTableFails() throws Throwable { String tableName = createTableName(); - assertThresholdAborts(format("Cannot have more than 2 tables, aborting the creation of table %s", tableName), - createTableQuery(tableName)); + assertThresholdFails(format("Cannot have more than 2 tables, aborting the creation of table %s", tableName), + createTableQuery(tableName)); } private String createTableQuery() diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java index 134b360488..7e9ace7161 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java @@ -53,10 +53,10 @@ import static org.junit.Assert.fail; public abstract class GuardrailTester extends CQLTester { - // Name used when testing CREATE TABLE that should be aborted (we need to provide it as assertAborts, which + // Name used when testing CREATE TABLE that should be aborted (we need to provide it as assertFails, which // is used to assert the failure, does not know that it is a CREATE TABLE and would thus reuse the name of the // previously created table, which is not what we want). - protected static final String ABORT_TABLE = "abort_table_creation_test"; + protected static final String FAIL_TABLE = "abort_table_creation_test"; private static final String USERNAME = "guardrail_user"; private static final String PASSWORD = "guardrail_password"; @@ -184,12 +184,12 @@ public abstract class GuardrailTester extends CQLTester assertWarns(() -> execute(userClientState, query), message); } - protected void assertAborts(CheckedFunction function, String message) throws Throwable + protected void assertFails(CheckedFunction function, String message) throws Throwable { - assertAborts(function, message, true); + assertFails(function, message, true); } - protected void assertAborts(CheckedFunction function, String message, boolean thrown) throws Throwable + protected void assertFails(CheckedFunction function, String message, boolean thrown) throws Throwable { ClientWarn.instance.captureWarnings(); try @@ -214,9 +214,9 @@ public abstract class GuardrailTester extends CQLTester } } - protected void assertAborts(String message, String query) throws Throwable + protected void assertFails(String message, String query) throws Throwable { - assertAborts(() -> execute(userClientState, query), message); + assertFails(() -> execute(userClientState, query), message); } private void assertWarnings(String message) diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java index 20b6a52b14..30909e97e4 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java @@ -41,7 +41,7 @@ public class GuardrailUserTimestampsTest extends GuardrailTester public void testInsertWithDisabledUserTimestamps() throws Throwable { setGuardrail(false); - assertAborts("INSERT INTO %s (k, c, v) VALUES (1, 2, 'val') USING TIMESTAMP 1"); + assertFails("INSERT INTO %s (k, c, v) VALUES (1, 2, 'val') USING TIMESTAMP 1"); } @Test @@ -55,7 +55,7 @@ public class GuardrailUserTimestampsTest extends GuardrailTester public void testUpdateWithDisabledUserTimestamps() throws Throwable { setGuardrail(false); - assertAborts("UPDATE %s USING TIMESTAMP 1 SET v = 'val2' WHERE k = 1 and c = 2"); + assertFails("UPDATE %s USING TIMESTAMP 1 SET v = 'val2' WHERE k = 1 and c = 2"); } @Test @@ -69,7 +69,7 @@ public class GuardrailUserTimestampsTest extends GuardrailTester public void testDeleteWithDisabledUserTimestamps() throws Throwable { setGuardrail(false); - assertAborts("DELETE FROM %s USING TIMESTAMP 1 WHERE k=1"); + assertFails("DELETE FROM %s USING TIMESTAMP 1 WHERE k=1"); } @Test @@ -86,9 +86,9 @@ public class GuardrailUserTimestampsTest extends GuardrailTester assertValid("BEGIN BATCH USING TIMESTAMP 1 " + "INSERT INTO %s (k, c, v) VALUES (1, 2, 'val') " + "APPLY BATCH"); - assertAborts("BEGIN BATCH " + - "INSERT INTO %s (k, c, v) VALUES (1, 2, 'val') USING TIMESTAMP 1 " + - "APPLY BATCH"); + assertFails("BEGIN BATCH " + + "INSERT INTO %s (k, c, v) VALUES (1, 2, 'val') USING TIMESTAMP 1 " + + "APPLY BATCH"); } @Test @@ -117,8 +117,8 @@ public class GuardrailUserTimestampsTest extends GuardrailTester } } - private void assertAborts(String query) throws Throwable + private void assertFails(String query) throws Throwable { - assertAborts("User provided timestamps (USING TIMESTAMP) is not allowed", query); + assertFails("User provided timestamps (USING TIMESTAMP) is not allowed", query); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java index 628e24437c..4780aa247e 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java @@ -21,8 +21,6 @@ package org.apache.cassandra.db.guardrails; import org.junit.Before; import org.junit.Test; -import org.apache.cassandra.config.DatabaseDescriptor; - import static java.lang.String.format; /** @@ -31,7 +29,7 @@ import static java.lang.String.format; public class GuardrailViewsPerTableTest extends ThresholdTester { private static final int VIEWS_PER_TABLE_WARN_THRESHOLD = 1; - private static final int VIEWS_PER_TABLE_ABORT_THRESHOLD = 3; + private static final int VIEWS_PER_TABLE_FAIL_THRESHOLD = 3; private static final String CREATE_TABLE = "CREATE TABLE %s (k int PRIMARY KEY, v int)"; private static final String CREATE_VIEW = "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s " + @@ -40,11 +38,11 @@ public class GuardrailViewsPerTableTest extends ThresholdTester public GuardrailViewsPerTableTest() { super(VIEWS_PER_TABLE_WARN_THRESHOLD, - VIEWS_PER_TABLE_ABORT_THRESHOLD, - DatabaseDescriptor.getGuardrailsConfig().getMaterializedViewsPerTable(), + VIEWS_PER_TABLE_FAIL_THRESHOLD, + "materialized_views_per_table", Guardrails::setMaterializedViewsPerTableThreshold, Guardrails::getMaterializedViewsPerTableWarnThreshold, - Guardrails::getMaterializedViewsPerTableAbortThreshold); + Guardrails::getMaterializedViewsPerTableFailThreshold); } @Override @@ -68,14 +66,14 @@ public class GuardrailViewsPerTableTest extends ThresholdTester assertCreateViewWarns(); assertCreateViewWarns(); - assertCreateViewAborts(); + assertCreateViewFails(); assertCurrentValue(3); // drop the first view, we should be able to create new MV again dropView(view1); assertCurrentValue(2); assertCreateViewWarns(); - assertCreateViewAborts(); + assertCreateViewFails(); assertCurrentValue(3); // previous guardrail should not apply to another base table @@ -83,7 +81,7 @@ public class GuardrailViewsPerTableTest extends ThresholdTester assertCreateViewSucceeds(); assertCreateViewWarns(); assertCreateViewWarns(); - assertCreateViewAborts(); + assertCreateViewFails(); assertCurrentValue(3); } @@ -115,11 +113,11 @@ public class GuardrailViewsPerTableTest extends ThresholdTester format(CREATE_VIEW, viewName)); } - private void assertCreateViewAborts() throws Throwable + private void assertCreateViewFails() throws Throwable { String viewName = createViewName(); - assertThresholdAborts(format("aborting the creation of materialized view %s on table %s", - viewName, currentTable()), - format(CREATE_VIEW, viewName)); + assertThresholdFails(format("aborting the creation of materialized view %s on table %s", + viewName, currentTable()), + format(CREATE_VIEW, viewName)); } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java index 24be9a9c83..35f8280a53 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java @@ -20,6 +20,8 @@ package org.apache.cassandra.db.guardrails; import org.junit.Test; +import org.apache.cassandra.config.Config; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.GuardrailsOptions; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.service.ClientState; @@ -34,16 +36,16 @@ 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().getWarnThreshold(), - state -> provider.getOrCreate(state).getTables().getAbortThreshold(), + Threshold guard = new Threshold(state -> provider.getOrCreate(state).getTablesWarnThreshold(), + state -> provider.getOrCreate(state).getTablesFailThreshold(), (isWarn, what, v, t) -> format("%s: for %s, %s > %s", isWarn ? "Warning" : "Aborting", what, v, t)); assertValid(() -> guard.guard(5, "Z", userClientState)); assertWarns(() -> guard.guard(25, "A", userClientState), "Warning: for A, 25 > 10"); assertWarns(() -> guard.guard(100, "B", userClientState), "Warning: for B, 100 > 10"); - assertAborts(() -> guard.guard(101, "X", userClientState), "Aborting: for X, 101 > 100"); - assertAborts(() -> guard.guard(200, "Y", userClientState), "Aborting: for Y, 200 > 100"); + assertFails(() -> guard.guard(101, "X", userClientState), "Aborting: for X, 101 > 100"); + assertFails(() -> guard.guard(200, "Y", userClientState), "Aborting: for Y, 200 > 100"); assertValid(() -> guard.guard(5, "Z", userClientState)); Assertions.assertThatThrownBy(() -> GuardrailsConfigProvider.build("unexistent_class")) @@ -58,23 +60,27 @@ public class GuardrailsConfigProviderTest extends GuardrailTester { public GuardrailsConfig getOrCreate(ClientState state) { - return new CustomConfig(); + return new CustomConfig(DatabaseDescriptor.getRawConfig()); } } public static class CustomConfig extends GuardrailsOptions { - private final IntThreshold tables = new IntThreshold(); - - public CustomConfig() + public CustomConfig(Config config) { - tables.setThresholds(10, 100); + super(config); } @Override - public IntThreshold getTables() + public int getTablesWarnThreshold() { - return tables; + return 10; + } + + @Override + public int getTablesFailThreshold() + { + return 100; } } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java index edc4c230ac..e7eaeb4016 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java @@ -68,8 +68,8 @@ public class GuardrailsTest extends GuardrailTester assertValid(() -> guard.guard(5, "Z", userClientState)); assertWarns(() -> guard.guard(25, "A", userClientState), "Warning: for A, 25 > 10"); assertWarns(() -> guard.guard(100, "B", userClientState), "Warning: for B, 100 > 10"); - assertAborts(() -> guard.guard(101, "X", userClientState), "Aborting: for X, 101 > 100"); - assertAborts(() -> guard.guard(200, "Y", userClientState), "Aborting: for Y, 200 > 100"); + assertFails(() -> guard.guard(101, "X", userClientState), "Aborting: for X, 101 > 100"); + assertFails(() -> guard.guard(200, "Y", userClientState), "Aborting: for Y, 200 > 100"); assertValid(() -> guard.guard(5, "Z", userClientState)); } @@ -88,7 +88,7 @@ public class GuardrailsTest extends GuardrailTester } @Test - public void testAbortOnlyThreshold() throws Throwable + public void testFailOnlyThreshold() throws Throwable { Threshold guard = new Threshold(state -> DISABLED, state -> 10, @@ -98,7 +98,7 @@ public class GuardrailsTest extends GuardrailTester assertTrue(guard.enabled(userClientState)); assertValid(() -> guard.guard(5, "Z", userClientState)); - assertAborts(() -> guard.guard(11, "A", userClientState), "Aborting: for A, 11 > 10"); + assertFails(() -> guard.guard(11, "A", userClientState), "Aborting: for A, 11 > 10"); } @Test @@ -121,9 +121,9 @@ public class GuardrailsTest extends GuardrailTester assertValid(() -> guard.guard(100, "y", systemClientState)); assertValid(() -> guard.guard(100, "y", superClientState)); - // value over abort threshold - assertAborts(() -> guard.guard(101, "z", null), "Aborting: for z, 101 > 100"); - assertAborts(() -> guard.guard(101, "z", userClientState), "Aborting: for z, 101 > 100"); + // value over fail threshold + assertFails(() -> guard.guard(101, "z", null), "Aborting: for z, 101 > 100"); + assertFails(() -> guard.guard(101, "z", userClientState), "Aborting: for z, 101 > 100"); assertValid(() -> guard.guard(101, "z", systemClientState)); assertValid(() -> guard.guard(101, "z", superClientState)); } @@ -131,10 +131,10 @@ public class GuardrailsTest extends GuardrailTester @Test public void testDisableFlag() throws Throwable { - assertAborts(() -> new DisableFlag(state -> true, "X").ensureEnabled(userClientState), "X is not allowed"); + assertFails(() -> new DisableFlag(state -> true, "X").ensureEnabled(userClientState), "X is not allowed"); assertValid(() -> new DisableFlag(state -> false, "X").ensureEnabled(userClientState)); - assertAborts(() -> new DisableFlag(state -> true, "X").ensureEnabled("Y", userClientState), "Y is not allowed"); + assertFails(() -> new DisableFlag(state -> true, "X").ensureEnabled("Y", userClientState), "Y is not allowed"); assertValid(() -> new DisableFlag(state -> false, "X").ensureEnabled("Y", userClientState)); } @@ -148,8 +148,8 @@ public class GuardrailsTest extends GuardrailTester assertValid(() -> enabled.ensureEnabled(superClientState)); DisableFlag disabled = new DisableFlag(state -> true, "X"); - assertAborts(() -> disabled.ensureEnabled(null), "X is not allowed"); - assertAborts(() -> disabled.ensureEnabled(userClientState), "X is not allowed"); + assertFails(() -> disabled.ensureEnabled(null), "X is not allowed"); + assertFails(() -> disabled.ensureEnabled(userClientState), "X is not allowed"); assertValid(() -> disabled.ensureEnabled(systemClientState)); assertValid(() -> disabled.ensureEnabled(superClientState)); } @@ -164,18 +164,18 @@ public class GuardrailsTest extends GuardrailTester Consumer action = i -> Assert.fail("The ignore action shouldn't have been triggered"); assertValid(() -> disallowed.guard(set(3), action, userClientState)); - assertAborts(() -> disallowed.guard(set(4), action, userClientState), - "Provided values [4] are not allowed for integer (disallowed values are: [4, 6, 20])"); + assertFails(() -> disallowed.guard(set(4), action, userClientState), + "Provided values [4] are not allowed for integer (disallowed values are: [4, 6, 20])"); assertValid(() -> disallowed.guard(set(10), action, userClientState)); - assertAborts(() -> disallowed.guard(set(20), action, userClientState), - "Provided values [20] are not allowed for integer (disallowed values are: [4, 6, 20])"); + assertFails(() -> disallowed.guard(set(20), action, userClientState), + "Provided values [20] are not allowed for integer (disallowed values are: [4, 6, 20])"); assertValid(() -> disallowed.guard(set(200), action, userClientState)); assertValid(() -> disallowed.guard(set(1, 2, 3), action, userClientState)); - assertAborts(() -> disallowed.guard(set(4, 6), action, null), - "Provided values [4, 6] are not allowed for integer (disallowed values are: [4, 6, 20])"); - assertAborts(() -> disallowed.guard(set(4, 5, 6, 7), action, null), - "Provided values [4, 6] are not allowed for integer (disallowed values are: [4, 6, 20])"); + assertFails(() -> disallowed.guard(set(4, 6), action, null), + "Provided values [4, 6] are not allowed for integer (disallowed values are: [4, 6, 20])"); + assertFails(() -> disallowed.guard(set(4, 5, 6, 7), action, null), + "Provided values [4, 6] are not allowed for integer (disallowed values are: [4, 6, 20])"); } @Test @@ -192,8 +192,8 @@ public class GuardrailsTest extends GuardrailTester assertValid(() -> disallowed.guard(set(1), action, superClientState)); String message = "Provided values [2] are not allowed for integer (disallowed values are: [2])"; - assertAborts(() -> disallowed.guard(set(2), action, null), message); - assertAborts(() -> disallowed.guard(set(2), action, userClientState), message); + assertFails(() -> disallowed.guard(set(2), action, null), message); + assertFails(() -> disallowed.guard(set(2), action, userClientState), message); assertValid(() -> disallowed.guard(set(2), action, systemClientState)); assertValid(() -> disallowed.guard(set(2), action, superClientState)); @@ -205,8 +205,8 @@ public class GuardrailsTest extends GuardrailTester Set disallowedValues = set(2); message = "Provided values [2] are not allowed for integer (disallowed values are: [2])"; - assertAborts(() -> disallowed.guard(disallowedValues, action, null), message); - assertAborts(() -> disallowed.guard(disallowedValues, action, userClientState), message); + assertFails(() -> disallowed.guard(disallowedValues, action, null), message); + assertFails(() -> disallowed.guard(disallowedValues, action, userClientState), message); assertValid(() -> disallowed.guard(disallowedValues, action, systemClientState)); assertValid(() -> disallowed.guard(disallowedValues, action, superClientState)); } diff --git a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java index 6974e46202..e57bfe5b90 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java @@ -25,7 +25,7 @@ import java.util.function.ToLongFunction; import org.junit.Before; import org.junit.Test; -import org.apache.cassandra.config.GuardrailsOptions; +import org.apache.cassandra.config.Config; import org.assertj.core.api.Assertions; import static java.lang.String.format; @@ -39,41 +39,40 @@ public abstract class ThresholdTester extends GuardrailTester { private final String name; private final long warnThreshold; - private final long abortThreshold; - private final GuardrailsOptions.Threshold config; + private final long failThreshold; private final TriConsumer setter; private final ToLongFunction warnGetter; - private final ToLongFunction abortGetter; + private final ToLongFunction failGetter; + private final long maxValue = Integer.MAX_VALUE; protected ThresholdTester(long warnThreshold, - long abortThreshold, - GuardrailsOptions.Threshold config, + long failThreshold, + String name, TriConsumer setter, ToLongFunction warnGetter, - ToLongFunction abortGetter) + ToLongFunction failGetter) { - this.name = config.getName(); + this.name = name; this.warnThreshold = warnThreshold; - this.abortThreshold = abortThreshold; - this.config = config; + this.failThreshold = failThreshold; this.setter = setter; this.warnGetter = warnGetter; - this.abortGetter = abortGetter; + this.failGetter = failGetter; } protected ThresholdTester(int warnThreshold, - int abortThreshold, - GuardrailsOptions.IntThreshold config, + int failThreshold, + String name, TriConsumer setter, ToIntFunction warnGetter, - ToIntFunction abortGetter) + ToIntFunction failGetter) { - this(warnThreshold, - abortThreshold, - (GuardrailsOptions.Threshold) config, - (g, w, a) -> setter.accept(g, w.intValue(), a.intValue()), - g -> (long) warnGetter.applyAsInt(g), - g -> (long) abortGetter.applyAsInt(g)); + this.name = name; + this.warnThreshold = warnThreshold; + this.failThreshold = failThreshold; + this.setter = (g, w, a) -> setter.accept(g, w.intValue(), a.intValue()); + this.warnGetter = g -> (long) warnGetter.applyAsInt(g); + this.failGetter = g -> (long) failGetter.applyAsInt(g); } protected abstract long currentValue(); @@ -86,26 +85,26 @@ public abstract class ThresholdTester extends GuardrailTester @Before public void before() { - setter.accept(guardrails(), warnThreshold, abortThreshold); + setter.accept(guardrails(), warnThreshold, failThreshold); } @Test public void testConfigValidation() { - testValidationOfThresholdProperties(name + ".warn_threshold", name + ".abort_threshold"); + testValidationOfThresholdProperties(name + "_warn_threshold", name + "_fail_threshold"); } - protected void testValidationOfThresholdProperties(String warnName, String abortName) + protected void testValidationOfThresholdProperties(String warnName, String failName) { setter.accept(guardrails(), -1L, -1L); - testValidationOfStrictlyPositiveProperty((g, a) -> setter.accept(g, -1L, a), abortName); + testValidationOfStrictlyPositiveProperty((g, a) -> setter.accept(g, -1L, a), failName); testValidationOfStrictlyPositiveProperty((g, w) -> setter.accept(g, w, -1L), warnName); setter.accept(guardrails(), -1L, -1L); Assertions.assertThatThrownBy(() -> setter.accept(guardrails(), 2L, 1L)) - .hasMessageContaining(format("The warn threshold 2 for %s should be lower than the abort threshold 1", - name)); + .hasMessageContaining(format("The warn threshold 2 for %s should be lower than the fail threshold 1", + name + "_warn_threshold")); } protected void assertThresholdValid(String query) throws Throwable @@ -114,7 +113,7 @@ public abstract class ThresholdTester extends GuardrailTester Assertions.assertThat(currentValue()) .isLessThanOrEqualTo(warnGetter.applyAsLong(guardrails())) - .isLessThanOrEqualTo(abortGetter.applyAsLong(guardrails())); + .isLessThanOrEqualTo(failGetter.applyAsLong(guardrails())); } protected void assertThresholdWarns(String message, String query) throws Throwable @@ -123,16 +122,16 @@ public abstract class ThresholdTester extends GuardrailTester Assertions.assertThat(currentValue()) .isGreaterThan(warnGetter.applyAsLong(guardrails())) - .isLessThanOrEqualTo(abortGetter.applyAsLong(guardrails())); + .isLessThanOrEqualTo(failGetter.applyAsLong(guardrails())); } - protected void assertThresholdAborts(String message, String query) throws Throwable + protected void assertThresholdFails(String message, String query) throws Throwable { - assertAborts(message, query); + assertFails(message, query); Assertions.assertThat(currentValue()) .isGreaterThanOrEqualTo(warnGetter.applyAsLong(guardrails())) - .isEqualTo(abortGetter.applyAsLong(guardrails())); + .isEqualTo(failGetter.applyAsLong(guardrails())); } private void assertInvalidPositiveProperty(BiConsumer setter, @@ -155,12 +154,12 @@ public abstract class ThresholdTester extends GuardrailTester value, name, maxValue); if (value == 0 && !allowZero) expectedMessage = format("Invalid value for %s: 0 is not allowed; if attempting to disable use %s", - name, GuardrailsOptions.Threshold.DISABLED); + name, Config.DISABLED_GUARDRAIL); - if (value < GuardrailsOptions.Threshold.DISABLED) + if (value < Config.DISABLED_GUARDRAIL) expectedMessage = format("Invalid value %d for %s: negative values are not " + "allowed, outside of %s which disables the guardrail", - value, name, GuardrailsOptions.Threshold.DISABLED); + value, name, Config.DISABLED_GUARDRAIL); assertEquals(format("Exception message '%s' does not contain '%s'", e.getMessage(), expectedMessage), expectedMessage, e.getMessage()); @@ -169,18 +168,18 @@ public abstract class ThresholdTester extends GuardrailTester private void assertInvalidStrictlyPositiveProperty(BiConsumer setter, long value, String name) { - assertInvalidPositiveProperty(setter, value, config.maxValue(), config.allowZero(), name); + assertInvalidPositiveProperty(setter, value, maxValue, false, name); } protected void testValidationOfStrictlyPositiveProperty(BiConsumer setter, String name) { assertInvalidStrictlyPositiveProperty(setter, Integer.MIN_VALUE, name); assertInvalidStrictlyPositiveProperty(setter, -2, name); - assertValidProperty(setter, GuardrailsOptions.Threshold.DISABLED); // disabled + assertValidProperty(setter, (long) Config.DISABLED_GUARDRAIL); // disabled assertInvalidStrictlyPositiveProperty(setter, 0, name); assertValidProperty(setter, 1L); assertValidProperty(setter, 2L); - assertValidProperty(setter, config.maxValue()); + assertValidProperty(setter, maxValue); } @FunctionalInterface