From da7c66222740f02c6de5b563681dd381d28f8201 Mon Sep 17 00:00:00 2001 From: Savni Nagarkar Date: Thu, 28 Apr 2022 13:18:39 -0700 Subject: [PATCH] Migrate threshold for minimum keyspace replication factor to guardrails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Savni Nagarkar; reviewed by Andres de la Peña, David Capwell for CASSANDRA-17212 --- CHANGES.txt | 1 + NEWS.txt | 1 + conf/cassandra.yaml | 10 +- .../org/apache/cassandra/config/Config.java | 5 +- .../cassandra/config/DatabaseDescriptor.java | 26 +- .../cassandra/config/GuardrailsOptions.java | 109 +++++-- .../schema/AlterKeyspaceStatement.java | 2 +- .../schema/CreateKeyspaceStatement.java | 2 +- .../cassandra/db/guardrails/Guardrails.java | 237 ++++++++------- .../db/guardrails/GuardrailsConfig.java | 11 + .../db/guardrails/GuardrailsMBean.java | 19 ++ .../cassandra/db/guardrails/MaxThreshold.java | 65 ++++ .../cassandra/db/guardrails/MinThreshold.java | 65 ++++ .../db/guardrails/PercentageThreshold.java | 2 +- .../cassandra/db/guardrails/Threshold.java | 35 +-- .../locator/AbstractReplicationStrategy.java | 25 +- .../locator/NetworkTopologyStrategy.java | 5 +- .../cassandra/locator/SimpleStrategy.java | 5 +- .../cassandra/schema/KeyspaceMetadata.java | 2 +- .../cassandra/schema/KeyspaceParams.java | 6 +- .../cassandra/schema/ReplicationParams.java | 5 +- .../cassandra/service/StorageService.java | 11 - .../service/StorageServiceMBean.java | 2 - .../org/apache/cassandra/tools/NodeProbe.java | 10 - .../org/apache/cassandra/tools/NodeTool.java | 2 - .../tools/nodetool/GetMinimumKeyspaceRF.java | 33 --- .../tools/nodetool/SetMinimumKeyspaceRF.java | 36 --- .../config/DatabaseDescriptorRefTest.java | 6 - .../config/DatabaseDescriptorTest.java | 20 -- .../cql3/validation/operations/AlterTest.java | 24 -- .../validation/operations/CreateTest.java | 22 -- .../GuardrailColumnsPerTableTest.java | 2 +- .../db/guardrails/GuardrailKeyspacesTest.java | 2 +- ...GuardrailMinimumReplicationFactorTest.java | 277 ++++++++++++++++++ .../GuardrailSecondaryIndexesPerTable.java | 2 +- .../db/guardrails/GuardrailTablesTest.java | 2 +- .../db/guardrails/GuardrailTester.java | 2 +- .../GuardrailViewsPerTableTest.java | 2 +- .../GuardrailsConfigProviderTest.java | 2 +- .../db/guardrails/GuardrailsTest.java | 127 +++++++- .../db/guardrails/ThresholdTester.java | 43 ++- .../locator/NetworkTopologyStrategyTest.java | 2 +- .../cassandra/locator/SimpleStrategyTest.java | 2 +- .../nodetool/GetMinimumKeyspaceRFTest.java | 87 ------ .../nodetool/SetMinimumKeyspaceRFTest.java | 94 ------ 45 files changed, 878 insertions(+), 572 deletions(-) create mode 100644 src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java create mode 100644 src/java/org/apache/cassandra/db/guardrails/MinThreshold.java delete mode 100644 src/java/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRF.java delete mode 100644 src/java/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRF.java create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailMinimumReplicationFactorTest.java delete mode 100644 test/unit/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRFTest.java delete mode 100644 test/unit/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRFTest.java diff --git a/CHANGES.txt b/CHANGES.txt index b069eacb4a..09e1471b33 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Migrate threshold for minimum keyspace replication factor to guardrails (CASSANDRA-17212) * Add guardrail to disallow TRUNCATE and DROP TABLE commands (CASSANDRA-17558) * Add plugin support for CQLSH (CASSANDRA-16456) * Add guardrail to disallow querying with ALLOW FILTERING (CASSANDRA-17370) diff --git a/NEWS.txt b/NEWS.txt index 97bfd331c4..cd13c996d4 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -79,6 +79,7 @@ New features - Allowed write consistency levels. - Collections size. - Query page size. + - Minimum replication factor. - Data disk usage, defined either as a percentage or as an absolute size. - Whether user-defined timestamps are allowed. - Whether GROUP BY queries are allowed. diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index c3e183fbd3..89feab4b12 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1449,11 +1449,6 @@ compaction_tombstone_warning_threshold: 100000 # Suggested value for use in production: 3 # default_keyspace_rf: 1 -# The minimum allowable replication factor. Creating a keyspace with a replication factor less than this value will be rejected. -# This would also apply to system keyspaces. -# Suggested value for use in production: 2 or higher -# minimum_keyspace_rf: 0 - # Track a metric per keyspace indicating whether replication achieved the ideal consistency # level for writes without timing out. This is different from the consistency level requested by # each write which may be lower in order to facilitate availability. @@ -1690,6 +1685,11 @@ drop_compact_storage_enabled: false # Valid values are in [1, max available disk size of all data directories]. # Defaults to null to disable and use the physically available disk size of data directories during calculations. # data_disk_usage_max_disk_size: +# Guardrail to warn or fail when the minimum replication factor is lesser than threshold. +# This would also apply to system keyspaces. +# Suggested value for use in production: 2 or higher +# minimum_replication_factor_warn_threshold: -1 +# minimum_replication_factor_fail_threshold: -1 # Startup Checks are executed as part of Cassandra startup process, not all of them # are configurable (so you can disable them) but these which are enumerated bellow. diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 1c277cf0c7..c39d9daaae 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -636,10 +636,9 @@ public class Config public volatile boolean diagnostic_events_enabled = false; - // Default and minimum keyspace replication factors allow validation of newly created keyspaces + // Default keyspace replication factors allow validation of newly created keyspaces // and good defaults if no replication factor is provided by the user public volatile int default_keyspace_rf = 1; - public volatile int minimum_keyspace_rf = 0; /** * flags for enabling tracking repaired state of data during reads @@ -823,6 +822,8 @@ public class Config public volatile int data_disk_usage_percentage_warn_threshold = -1; public volatile int data_disk_usage_percentage_fail_threshold = -1; public volatile DataStorageSpec data_disk_usage_max_disk_size = null; + public volatile int minimum_replication_factor_warn_threshold = -1; + public volatile int minimum_replication_factor_fail_threshold = -1; public volatile DurationSpec streaming_state_expires = DurationSpec.inDays(3); public volatile DataStorageSpec streaming_state_size = DataStorageSpec.inMebibytes(40); diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 0ce787fe32..a264cbc847 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -868,10 +868,10 @@ public class DatabaseDescriptor validateMaxConcurrentAutoUpgradeTasksConf(conf.max_concurrent_automatic_sstable_upgrades); - if (conf.default_keyspace_rf < conf.minimum_keyspace_rf) + if (conf.default_keyspace_rf < conf.minimum_replication_factor_fail_threshold) { - throw new ConfigurationException(String.format("default_keyspace_rf (%d) cannot be less than minimum_keyspace_rf (%d)", - conf.default_keyspace_rf, conf.minimum_keyspace_rf)); + throw new ConfigurationException(String.format("default_keyspace_rf (%d) cannot be less than minimum_replication_factor_fail_threshold (%d)", + conf.default_keyspace_rf, conf.minimum_replication_factor_fail_threshold)); } if (conf.paxos_repair_parallelism <= 0) @@ -4081,30 +4081,14 @@ public class DatabaseDescriptor throw new ConfigurationException("default_keyspace_rf cannot be less than 1"); } - if (value < getMinimumKeyspaceRF()) + if (value < guardrails.getMinimumReplicationFactorFailThreshold()) { - throw new ConfigurationException(String.format("default_keyspace_rf to be set (%d) cannot be less than minimum_keyspace_rf (%d)", value, getMinimumKeyspaceRF())); + throw new ConfigurationException(String.format("default_keyspace_rf to be set (%d) cannot be less than minimum_replication_factor_fail_threshold (%d)", value, guardrails.getMinimumReplicationFactorFailThreshold())); } conf.default_keyspace_rf = value; } - public static int getMinimumKeyspaceRF() { return conf.minimum_keyspace_rf; } - - public static void setMinimumKeyspaceRF(int value) throws ConfigurationException - { - if (value < 0) - { - throw new ConfigurationException("minimum_keyspace_rf cannot be negative"); - } - - if (value > getDefaultKeyspaceRF()) - { - throw new ConfigurationException(String.format("minimum_keyspace_rf to be set (%d) cannot be greater than default_keyspace_rf (%d)", value, getDefaultKeyspaceRF())); - } - - conf.minimum_keyspace_rf = value; - } public static boolean getUseStatementsEnabled() { diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 6654b4fc74..27ad37d3ef 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -61,27 +61,27 @@ public class GuardrailsOptions implements GuardrailsConfig public GuardrailsOptions(Config config) { 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"); + validateMaxIntThreshold(config.keyspaces_warn_threshold, config.keyspaces_fail_threshold, "keyspaces"); + validateMaxIntThreshold(config.tables_warn_threshold, config.tables_fail_threshold, "tables"); + validateMaxIntThreshold(config.columns_per_table_warn_threshold, config.columns_per_table_fail_threshold, "columns_per_table"); + validateMaxIntThreshold(config.secondary_indexes_per_table_warn_threshold, config.secondary_indexes_per_table_fail_threshold, "secondary_indexes_per_table"); + validateMaxIntThreshold(config.materialized_views_per_table_warn_threshold, config.materialized_views_per_table_fail_threshold, "materialized_views_per_table"); config.table_properties_warned = validateTableProperties(config.table_properties_warned, "table_properties_warned"); 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"); - validateIntThreshold(config.partition_keys_in_select_warn_threshold, - config.partition_keys_in_select_fail_threshold, "partition_keys_in_select"); - validateIntThreshold(config.in_select_cartesian_product_warn_threshold, config.in_select_cartesian_product_fail_threshold, "in_select_cartesian_product"); + validateMaxIntThreshold(config.page_size_warn_threshold, config.page_size_fail_threshold, "page_size"); + validateMaxIntThreshold(config.partition_keys_in_select_warn_threshold, config.partition_keys_in_select_fail_threshold, "partition_keys_in_select"); + validateMaxIntThreshold(config.in_select_cartesian_product_warn_threshold, config.in_select_cartesian_product_fail_threshold, "in_select_cartesian_product"); config.read_consistency_levels_warned = validateConsistencyLevels(config.read_consistency_levels_warned, "read_consistency_levels_warned"); config.read_consistency_levels_disallowed = validateConsistencyLevels(config.read_consistency_levels_disallowed, "read_consistency_levels_disallowed"); config.write_consistency_levels_warned = validateConsistencyLevels(config.write_consistency_levels_warned, "write_consistency_levels_warned"); config.write_consistency_levels_disallowed = validateConsistencyLevels(config.write_consistency_levels_disallowed, "write_consistency_levels_disallowed"); validateSizeThreshold(config.collection_size_warn_threshold, config.collection_size_fail_threshold, false, "collection_size"); - validateIntThreshold(config.items_per_collection_warn_threshold, config.items_per_collection_fail_threshold, "items_per_collection"); - validateIntThreshold(config.fields_per_udt_warn_threshold, config.fields_per_udt_fail_threshold, "fields_per_udt"); + validateMaxIntThreshold(config.items_per_collection_warn_threshold, config.items_per_collection_fail_threshold, "items_per_collection"); + validateMaxIntThreshold(config.fields_per_udt_warn_threshold, config.fields_per_udt_fail_threshold, "fields_per_udt"); validatePercentageThreshold(config.data_disk_usage_percentage_warn_threshold, config.data_disk_usage_percentage_fail_threshold, "data_disk_usage_percentage"); validateDataDiskUsageMaxDiskSize(config.data_disk_usage_max_disk_size); + validateMinRFThreshold(config.minimum_replication_factor_warn_threshold, config.minimum_replication_factor_fail_threshold, "minimum_replication_factor"); } @Override @@ -98,7 +98,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setKeyspacesThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "keyspaces"); + validateMaxIntThreshold(warn, fail, "keyspaces"); updatePropertyWithLogging("keyspaces_warn_threshold", warn, () -> config.keyspaces_warn_threshold, @@ -123,7 +123,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setTablesThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "tables"); + validateMaxIntThreshold(warn, fail, "tables"); updatePropertyWithLogging("tables_warn_threshold", warn, () -> config.tables_warn_threshold, @@ -148,7 +148,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setColumnsPerTableThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "columns_per_table"); + validateMaxIntThreshold(warn, fail, "columns_per_table"); updatePropertyWithLogging("columns_per_table_warn_threshold", warn, () -> config.columns_per_table_warn_threshold, @@ -173,7 +173,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setSecondaryIndexesPerTableThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "secondary_indexes_per_table"); + validateMaxIntThreshold(warn, fail, "secondary_indexes_per_table"); updatePropertyWithLogging("secondary_indexes_per_table_warn_threshold", warn, () -> config.secondary_indexes_per_table_warn_threshold, @@ -204,7 +204,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setPartitionKeysInSelectThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "partition_keys_in_select"); + validateMaxIntThreshold(warn, fail, "partition_keys_in_select"); updatePropertyWithLogging("partition_keys_in_select_warn_threshold", warn, () -> config.partition_keys_in_select_warn_threshold, @@ -223,7 +223,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setMaterializedViewsPerTableThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "materialized_views_per_table"); + validateMaxIntThreshold(warn, fail, "materialized_views_per_table"); updatePropertyWithLogging("materialized_views_per_table_warn_threshold", warn, () -> config.materialized_views_per_table_warn_threshold, @@ -248,7 +248,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setPageSizeThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "page_size"); + validateMaxIntThreshold(warn, fail, "page_size"); updatePropertyWithLogging("page_size_warn_threshold", warn, () -> config.page_size_warn_threshold, @@ -427,7 +427,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setInSelectCartesianProductThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "in_select_cartesian_product"); + validateMaxIntThreshold(warn, fail, "in_select_cartesian_product"); updatePropertyWithLogging("in_select_cartesian_product_warn_threshold", warn, () -> config.in_select_cartesian_product_warn_threshold, @@ -534,7 +534,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setItemsPerCollectionThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "items_per_collection"); + validateMaxIntThreshold(warn, fail, "items_per_collection"); updatePropertyWithLogging("items_per_collection_warn_threshold", warn, () -> config.items_per_collection_warn_threshold, @@ -559,7 +559,7 @@ public class GuardrailsOptions implements GuardrailsConfig public void setFieldsPerUDTThreshold(int warn, int fail) { - validateIntThreshold(warn, fail, "fields_per_udt"); + validateMaxIntThreshold(warn, fail, "fields_per_udt"); updatePropertyWithLogging("fields_per_udt_warn_threshold", warn, () -> config.fields_per_udt_warn_threshold, @@ -609,6 +609,31 @@ public class GuardrailsOptions implements GuardrailsConfig x -> config.data_disk_usage_max_disk_size = x); } + @Override + public int getMinimumReplicationFactorWarnThreshold() + { + return config.minimum_replication_factor_warn_threshold; + } + + @Override + public int getMinimumReplicationFactorFailThreshold() + { + return config.minimum_replication_factor_fail_threshold; + } + + public void setMinimumReplicationFactorThreshold(int warn, int fail) + { + validateMinRFThreshold(warn, fail, "minimum_replication_factor"); + updatePropertyWithLogging("minimum_replication_factor_warn_threshold", + warn, + () -> config.minimum_replication_factor_warn_threshold, + x -> config.minimum_replication_factor_warn_threshold = x); + updatePropertyWithLogging("minimum_replication_factor_fail_threshold", + fail, + () -> config.minimum_replication_factor_fail_threshold, + x -> config.minimum_replication_factor_fail_threshold = x); + } + private static void updatePropertyWithLogging(String propertyName, T newValue, Supplier getter, Consumer setter) { T oldValue = getter.get(); @@ -643,18 +668,31 @@ public class GuardrailsOptions implements GuardrailsConfig validatePositiveNumeric(value, 100, name); } - private static void validateIntThreshold(int warn, int fail, String name) + private static void validatePercentageThreshold(int warn, int fail, String name) + { + validatePercentage(warn, name + "_warn_threshold"); + validatePercentage(fail, name + "_fail_threshold"); + validateWarnLowerThanFail(warn, fail, name); + } + + private static void validateMaxIntThreshold(int warn, int fail, String name) { validatePositiveNumeric(warn, Integer.MAX_VALUE, name + "_warn_threshold"); validatePositiveNumeric(fail, Integer.MAX_VALUE, name + "_fail_threshold"); validateWarnLowerThanFail(warn, fail, name); } - private static void validatePercentageThreshold(int warn, int fail, String name) + private static void validateMinIntThreshold(int warn, int fail, String name) { - validatePercentage(warn, name + "_warn_threshold"); - validatePercentage(fail, name + "_fail_threshold"); - validateWarnLowerThanFail(warn, fail, name); + validatePositiveNumeric(warn, Integer.MAX_VALUE, name + "_warn_threshold"); + validatePositiveNumeric(fail, Integer.MAX_VALUE, name + "_fail_threshold"); + validateWarnGreaterThanFail(warn, fail, name); + } + + private static void validateMinRFThreshold(int warn, int fail, String name) + { + validateMinIntThreshold(warn, fail, name); + validateMinRFVersusDefaultRF(fail, name); } private static void validateWarnLowerThanFail(long warn, long fail, String name) @@ -667,6 +705,25 @@ public class GuardrailsOptions implements GuardrailsConfig "than the fail threshold %d", warn, name, fail)); } + private static void validateWarnGreaterThanFail(long warn, long fail, String name) + { + if (warn == -1 || fail == -1) + return; + + if (fail > warn) + throw new IllegalArgumentException(format("The warn threshold %d for %s_warn_threshold should be greater " + + "than the fail threshold %d", warn, name, fail)); + } + + private static void validateMinRFVersusDefaultRF(int fail, String name) throws IllegalArgumentException + { + if (fail > DatabaseDescriptor.getDefaultKeyspaceRF()) + { + throw new IllegalArgumentException(String.format("%s_fail_threshold to be set (%d) cannot be greater than default_keyspace_rf (%d)", + name, fail, DatabaseDescriptor.getDefaultKeyspaceRF())); + } + } + private static void validateSize(DataStorageSpec size, boolean allowZero, String name) { if (size == null) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java index 14c648f70d..87377d70ec 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterKeyspaceStatement.java @@ -79,7 +79,7 @@ public final class AlterKeyspaceStatement extends AlterSchemaStatement if (newKeyspace.params.replication.klass.equals(LocalStrategy.class)) throw ire("Unable to use given strategy class: LocalStrategy is reserved for internal use."); - newKeyspace.params.validate(keyspaceName); + newKeyspace.params.validate(keyspaceName, state); validateNoRangeMovements(); validateTransientReplication(keyspace.createReplicationStrategy(), newKeyspace.createReplicationStrategy()); diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java index 256b33b430..dc82f93a10 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java @@ -80,7 +80,7 @@ public final class CreateKeyspaceStatement extends AlterSchemaStatement if (keyspace.params.replication.klass.equals(LocalStrategy.class)) throw ire("Unable to use given strategy class: LocalStrategy is reserved for internal use."); - keyspace.params.validate(keyspaceName); + keyspace.params.validate(keyspaceName, state); return schema.withAddedOrUpdated(keyspace); } diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 3578ceb1ec..1eb2fbd93f 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -55,51 +55,51 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the total number of user keyspaces. */ - public static final Threshold keyspaces = - new Threshold("keyspaces", - 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) - : format("Cannot have more than %s keyspaces, aborting the creation of keyspace %s", - threshold, what)); + public static final MaxThreshold keyspaces = + new MaxThreshold("keyspaces", + 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) + : format("Cannot have more than %s keyspaces, aborting the creation of keyspace %s", + threshold, what)); /** * Guardrail on the total number of tables on user keyspaces. */ - public static final Threshold tables = - new Threshold("tables", - 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) - : format("Cannot have more than %s tables, aborting the creation of table %s", - threshold, what)); + public static final MaxThreshold tables = + new MaxThreshold("tables", + 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) + : format("Cannot have more than %s tables, aborting the creation of table %s", + threshold, what)); /** * Guardrail on the number of columns per table. */ - public static final Threshold columnsPerTable = - new Threshold("columns_per_table", - 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) - : format("Tables cannot have more than %s columns, but %s provided for table %s", - threshold, value, what)); + public static final MaxThreshold columnsPerTable = + new MaxThreshold("columns_per_table", + 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) + : format("Tables cannot have more than %s columns, but %s provided for table %s", + threshold, value, what)); - public static final Threshold secondaryIndexesPerTable = - new Threshold("secondary_indexes_per_table", - 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) - : format("Tables cannot have more than %s secondary indexes, aborting the creation of secondary index %s", - threshold, what)); + public static final MaxThreshold secondaryIndexesPerTable = + new MaxThreshold("secondary_indexes_per_table", + 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) + : format("Tables cannot have more than %s secondary indexes, aborting the creation of secondary index %s", + threshold, what)); /** * Guardrail disabling user's ability to create secondary indexes @@ -112,15 +112,15 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the number of materialized views per table. */ - public static final Threshold materializedViewsPerTable = - new Threshold("materialized_views_per_table", - 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) - : format("Tables cannot have more than %s materialized views, aborting the creation of materialized view %s", - threshold, what)); + public static final MaxThreshold materializedViewsPerTable = + new MaxThreshold("materialized_views_per_table", + 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) + : format("Tables cannot have more than %s materialized views, aborting the creation of materialized view %s", + threshold, what)); /** * Guardrail warning about, ignoring or rejecting the usage of certain table properties. @@ -169,30 +169,30 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the number of elements returned within page. */ - public static final Threshold pageSize = - new Threshold("page_size", - 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 fail threshold of %s.", - what, value, threshold)); + public static final MaxThreshold pageSize = + new MaxThreshold("page_size", + 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 fail threshold of %s.", + what, value, threshold)); /** * Guardrail on the number of partition keys in the IN clause. */ - public static final Threshold partitionKeysInSelect = - new Threshold("partition_keys_in_select", - state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectFailThreshold(), - (isWarning, what, value, threshold) -> - isWarning ? format("Query with partition keys in IN clause on table %s, with number of " + - "partition keys %s exceeds warning threshold of %s.", - what, value, threshold) - : format("Aborting query with partition keys in IN clause on table %s, " + - "number of partition keys %s exceeds fail threshold of %s.", - what, value, threshold)); + public static final MaxThreshold partitionKeysInSelect = + new MaxThreshold("partition_keys_in_select", + state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("Query with partition keys in IN clause on table %s, with number of " + + "partition keys %s exceeds warning threshold of %s.", + what, value, threshold) + : format("Aborting query with partition keys in IN clause on table %s, " + + "number of partition keys %s exceeds fail threshold of %s.", + what, value, threshold)); /** * Guardrail disabling operations on lists that require read before write. @@ -213,17 +213,17 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the number of restrictions created by a cartesian product of a CQL's {@code IN} query. */ - public static final Threshold inSelectCartesianProduct = - new Threshold("in_select_cartesian_product", - state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductFailThreshold(), - (isWarning, what, value, threshold) -> - isWarning ? format("The cartesian product of the IN restrictions on %s produces %s values, " + - "this exceeds warning threshold of %s.", - what, value, threshold) - : format("Aborting query because the cartesian product of the IN restrictions on %s " + - "produces %s values, this exceeds fail threshold of %s.", - what, value, threshold)); + public static final MaxThreshold inSelectCartesianProduct = + new MaxThreshold("in_select_cartesian_product", + state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("The cartesian product of the IN restrictions on %s produces %s values, " + + "this exceeds warning threshold of %s.", + what, value, threshold) + : format("Aborting query because the cartesian product of the IN restrictions on %s " + + "produces %s values, this exceeds fail threshold of %s.", + what, value, threshold)); /** * Guardrail on read consistency levels. @@ -248,41 +248,41 @@ public final class Guardrails implements GuardrailsMBean /** * Guardrail on the size of a collection. */ - public static final Threshold collectionSize = - new Threshold("collection_size", - state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeWarnThreshold()), - state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeFailThreshold()), - (isWarning, what, value, threshold) -> - isWarning ? format("Detected collection %s of size %s, this exceeds the warning threshold of %s.", - what, value, threshold) - : format("Detected collection %s of size %s, this exceeds the failure threshold of %s.", - what, value, threshold)); + public static final MaxThreshold collectionSize = + new MaxThreshold("collection_size", + state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeWarnThreshold()), + state -> sizeToBytes(CONFIG_PROVIDER.getOrCreate(state).getCollectionSizeFailThreshold()), + (isWarning, what, value, threshold) -> + isWarning ? format("Detected collection %s of size %s, this exceeds the warning threshold of %s.", + what, value, threshold) + : format("Detected collection %s of size %s, this exceeds the failure threshold of %s.", + what, value, threshold)); /** * Guardrail on the number of items of a collection. */ - public static final Threshold itemsPerCollection = - new Threshold("items_per_collection", - state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionFailThreshold(), - (isWarning, what, value, threshold) -> - isWarning ? format("Detected collection %s with %s items, this exceeds the warning threshold of %s.", - what, value, threshold) - : format("Detected collection %s with %s items, this exceeds the failure threshold of %s.", - what, value, threshold)); + public static final MaxThreshold itemsPerCollection = + new MaxThreshold("items_per_collection", + state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getItemsPerCollectionFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("Detected collection %s with %s items, this exceeds the warning threshold of %s.", + what, value, threshold) + : format("Detected collection %s with %s items, this exceeds the failure threshold of %s.", + what, value, threshold)); /** * Guardrail on the number of fields on each UDT. */ - public static final Threshold fieldsPerUDT = - new Threshold("fields_per_udt", - state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTWarnThreshold(), - state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTFailThreshold(), - (isWarning, what, value, threshold) -> - isWarning ? format("The user type %s has %s columns, this exceeds the warning threshold of %s.", - what, value, threshold) - : format("User types cannot have more than %s columns, but %s provided for user type %s.", - threshold, value, what)); + public static final MaxThreshold fieldsPerUDT = + new MaxThreshold("fields_per_udt", + state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getFieldsPerUDTFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("The user type %s has %s columns, this exceeds the warning threshold of %s.", + what, value, threshold) + : format("User types cannot have more than %s columns, but %s provided for user type %s.", + threshold, value, what)); /** * Guardrail on the data disk usage on the local node, used by a periodic task to calculate and propagate that status. @@ -320,6 +320,19 @@ public final class Guardrails implements GuardrailsMBean replicaDiskUsage.minNotifyIntervalInMs(minNotifyInterval); } + /** + * Guardrail on the minimum replication factor. + */ + public static final MinThreshold minimumReplicationFactor = + new MinThreshold("minimum_replication_factor", + state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumReplicationFactorWarnThreshold(), + state -> CONFIG_PROVIDER.getOrCreate(state).getMinimumReplicationFactorFailThreshold(), + (isWarning, what, value, threshold) -> + isWarning ? format("The keyspace %s has a replication factor of %s, below the warning threshold of %s.", + what, value, threshold) + : format("The keyspace %s has a replication factor of %s, below the failure threshold of %s.", + what, value, threshold)); + private Guardrails() { MBeanWrapper.instance.registerMBean(this, MBEAN_NAME); @@ -835,6 +848,24 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setDataDiskUsageMaxDiskSize(sizeFromString(size)); } + @Override + public int getMinimumReplicationFactorWarnThreshold() + { + return DEFAULT_CONFIG.getMinimumReplicationFactorWarnThreshold(); + } + + @Override + public int getMinimumReplicationFactorFailThreshold() + { + return DEFAULT_CONFIG.getMinimumReplicationFactorFailThreshold(); + } + + @Override + public void setMinimumReplicationFactorThreshold(int warn, int fail) + { + DEFAULT_CONFIG.setMinimumReplicationFactorThreshold(warn, fail); + } + private static String toCSV(Set values) { return values == null || values.isEmpty() ? "" : String.join(",", values); diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index cb2bd313b9..8c9cf6b20f 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -266,4 +266,15 @@ public interface GuardrailsConfig */ @Nullable DataStorageSpec getDataDiskUsageMaxDiskSize(); + + /** + * @return The threshold to warn when replication factor is lesser than threshold. + */ + int getMinimumReplicationFactorWarnThreshold(); + + /** + * @return The threshold to fail when replication factor is lesser than threshold. + */ + int getMinimumReplicationFactorFailThreshold(); + } diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 0240f430a4..e109c40873 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -520,4 +520,23 @@ public interface GuardrailsMBean * A {@code null} value means disabled. */ void setDataDiskUsageMaxDiskSize(@Nullable String size); + + /** + * @return The threshold to warn when replication factor is lesser threshold. + */ + int getMinimumReplicationFactorWarnThreshold(); + + /** + * @return The threshold to fail when replication factor is lesser threshold. + */ + int getMinimumReplicationFactorFailThreshold(); + + /** + * @param warn the threshold to warn when the minimum replication factor is lesser than + * threshold -1 means disabled. + * @param fail the threshold to fail when the minimum replication factor is lesser than + * threshold -1 means disabled. + */ + void setMinimumReplicationFactorThreshold (int warn, int fail); + } diff --git a/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java b/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java new file mode 100644 index 0000000000..badaff784b --- /dev/null +++ b/src/java/org/apache/cassandra/db/guardrails/MaxThreshold.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import java.util.function.ToLongFunction; +import org.apache.cassandra.service.ClientState; + +/** + * {@link MaxThreshold} for maximum guardrails, the value is checked to see if it is greater than the warn and fail thresholds. + */ +public class MaxThreshold extends Threshold +{ + /** + * Creates a new threshold guardrail. + * + * @param name the identifying name of the guardrail + * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. + * @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 MaxThreshold(String name, + ToLongFunction warnThreshold, + ToLongFunction failThreshold, + Threshold.ErrorMessageProvider messageProvider) + { + super(name, warnThreshold, failThreshold, messageProvider); + } + + @Override + protected boolean compare(long value, long threshold) + { + return value > threshold; + } + + @Override + protected long failValue(ClientState state) + { + long failValue = failThreshold.applyAsLong(state); + return failValue <= 0 ? Long.MAX_VALUE : failValue; + } + + @Override + protected long warnValue(ClientState state) + { + long warnValue = warnThreshold.applyAsLong(state); + return warnValue <= 0 ? Long.MAX_VALUE : warnValue; + } + +} diff --git a/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java b/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java new file mode 100644 index 0000000000..427f2777e8 --- /dev/null +++ b/src/java/org/apache/cassandra/db/guardrails/MinThreshold.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import java.util.function.ToLongFunction; +import org.apache.cassandra.service.ClientState; + +/** + * {@link MinThreshold} for minimum guardrails, the value is checked to see if it is lesser than the warn and fail thresholds. + */ +public class MinThreshold extends Threshold +{ + /** + * Creates a new minimum threshold guardrail. + * + * @param name the identifying name of the guardrail + * @param warnThreshold a {@link ClientState}-based provider of the value above which a warning should be triggered. + * @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 MinThreshold(String name, + ToLongFunction warnThreshold, + ToLongFunction failThreshold, + Threshold.ErrorMessageProvider messageProvider) + { + super(name, warnThreshold, failThreshold, messageProvider); + } + + @Override + protected boolean compare(long value, long threshold) + { + return value < threshold; + } + + @Override + protected long failValue(ClientState state) + { + long failValue = failThreshold.applyAsLong(state); + return failValue <= 0 ? Long.MIN_VALUE : failValue; + } + + @Override + protected long warnValue(ClientState state) + { + long warnValue = warnThreshold.applyAsLong(state); + return warnValue <= 0 ? Long.MIN_VALUE : warnValue; + } + +} diff --git a/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java b/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java index c08d02641b..6f866c6ec0 100644 --- a/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/PercentageThreshold.java @@ -27,7 +27,7 @@ import org.apache.cassandra.service.ClientState; *

* This works exactly as a {@link Threshold}, but provides slightly more convenient error messages for percentage */ -public class PercentageThreshold extends Threshold +public class PercentageThreshold extends MaxThreshold { /** * Creates a new threshold guardrail. diff --git a/src/java/org/apache/cassandra/db/guardrails/Threshold.java b/src/java/org/apache/cassandra/db/guardrails/Threshold.java index f7e4823448..b671907c62 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Threshold.java +++ b/src/java/org/apache/cassandra/db/guardrails/Threshold.java @@ -26,15 +26,15 @@ import org.apache.cassandra.service.ClientState; /** * A guardrail based on numeric threshold(s). * - *

A {@link Threshold} guardrail defines (up to) 2 thresholds, one at which a warning is issued, and a higher one + *

A {@link Threshold} guardrail defines (up to) 2 thresholds, one at which a warning is issued, and a lower one * at which the operation is aborted with an exception. Only one of those thresholds can be activated if desired. * *

This guardrail only handles guarding positive values. */ -public class Threshold extends Guardrail +public abstract class Threshold extends Guardrail { - private final ToLongFunction warnThreshold; - private final ToLongFunction failThreshold; + protected ToLongFunction warnThreshold; + protected ToLongFunction failThreshold; protected final ErrorMessageProvider messageProvider; /** @@ -56,6 +56,8 @@ public class Threshold extends Guardrail this.messageProvider = messageProvider; } + protected abstract boolean compare(long value, long threshold); + protected String errMsg(boolean isWarning, String what, long value, long thresholdValue) { return messageProvider.createMessage(isWarning, @@ -69,19 +71,10 @@ public class Threshold extends Guardrail return errMsg(isWarning, REDACTED, value, thresholdValue); } - private long failValue(ClientState state) - { - long failValue = failThreshold.applyAsLong(state); - return failValue <= 0 ? Long.MAX_VALUE : failValue; - } + protected abstract long failValue(ClientState state); - private long warnValue(ClientState state) - { - long warnValue = warnThreshold.applyAsLong(state); - return warnValue <= 0 ? Long.MAX_VALUE : warnValue; - } + protected abstract long warnValue(ClientState state); - @Override public boolean enabled(@Nullable ClientState state) { if (!super.enabled(state)) @@ -105,17 +98,17 @@ public class Threshold extends Guardrail */ public boolean triggersOn(long value, @Nullable ClientState state) { - return enabled(state) && (value > Math.min(failValue(state), warnValue(state))); + return enabled(state) && (compare(value, warnValue(state)) || compare(value, failValue(state))); } public boolean warnsOn(long value, @Nullable ClientState state) { - return enabled(state) && (value > warnValue(state) && value <= failValue(state)); + return enabled(state) && compare(value, warnValue(state)); } public boolean failsOn(long value, @Nullable ClientState state) { - return enabled(state) && (value > failValue(state)); + return enabled(state) && compare(value, failValue(state)); } /** @@ -136,14 +129,14 @@ public class Threshold extends Guardrail return; long failValue = failValue(state); - if (value > failValue) + if (compare(value, failValue)) { triggerFail(value, failValue, what, containsUserData, state); return; } long warnValue = warnValue(state); - if (value > warnValue) + if (compare(value, warnValue)) triggerWarn(value, warnValue, what, containsUserData); } @@ -175,4 +168,4 @@ public class Threshold extends Guardrail */ String createMessage(boolean isWarning, String what, String value, String threshold); } -} +} \ No newline at end of file diff --git a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java index 7f33c578ae..c233fc0fbc 100644 --- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java +++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java @@ -41,6 +41,7 @@ import org.apache.cassandra.dht.Token; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.locator.ReplicaCollection.Builder.Conflict; import org.apache.cassandra.service.AbstractWriteResponseHandler; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.DatacenterSyncWriteResponseHandler; import org.apache.cassandra.service.DatacenterWriteResponseHandler; import org.apache.cassandra.service.WriteResponseHandler; @@ -286,7 +287,17 @@ public abstract class AbstractReplicationStrategy public abstract void validateOptions() throws ConfigurationException; - public abstract void maybeWarnOnOptions(); + @Deprecated // use #maybeWarnOnOptions(ClientState) instead + public void maybeWarnOnOptions() + { + // nothing to do here + } + + public void maybeWarnOnOptions(ClientState state) + { + maybeWarnOnOptions(); + } + /* * The options recognized by the strategy. @@ -384,12 +395,13 @@ public abstract class AbstractReplicationStrategy Class strategyClass, TokenMetadata tokenMetadata, IEndpointSnitch snitch, - Map strategyOptions) throws ConfigurationException + Map strategyOptions, + ClientState state) throws ConfigurationException { AbstractReplicationStrategy strategy = createInternal(keyspaceName, strategyClass, tokenMetadata, snitch, strategyOptions); strategy.validateExpectedOptions(); strategy.validateOptions(); - strategy.maybeWarnOnOptions(); + strategy.maybeWarnOnOptions(state); if (strategy.hasTransientReplicas() && !DatabaseDescriptor.isTransientReplicationEnabled()) { throw new ConfigurationException("Transient replication is disabled. Enable in cassandra.yaml to use."); @@ -421,12 +433,7 @@ public abstract class AbstractReplicationStrategy try { ReplicationFactor rf = ReplicationFactor.fromString(s); - - if (rf.fullReplicas < DatabaseDescriptor.getMinimumKeyspaceRF()) - { - throw new ConfigurationException(String.format("Replication factor cannot be less than minimum_keyspace_rf (%d), found %d", DatabaseDescriptor.getMinimumKeyspaceRF(), rf.fullReplicas)); - } - + if (rf.hasTransientReplicas()) { if (DatabaseDescriptor.getNumTokens() > 1) diff --git a/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java b/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java index dd2ec99d9b..9ae034121a 100644 --- a/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java +++ b/src/java/org/apache/cassandra/locator/NetworkTopologyStrategy.java @@ -21,6 +21,7 @@ import java.util.*; import java.util.Map.Entry; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.locator.ReplicaCollection.Builder.Conflict; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +32,7 @@ import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.dht.Token; import org.apache.cassandra.locator.TokenMetadata.Topology; import org.apache.cassandra.schema.SchemaConstants; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.utils.FBUtilities; @@ -336,7 +338,7 @@ public class NetworkTopologyStrategy extends AbstractReplicationStrategy } @Override - public void maybeWarnOnOptions() + public void maybeWarnOnOptions(ClientState state) { if (!SchemaConstants.isSystemKeyspace(keyspaceName)) { @@ -347,6 +349,7 @@ public class NetworkTopologyStrategy extends AbstractReplicationStrategy String dc = e.getKey(); ReplicationFactor rf = getReplicationFactor(dc); + Guardrails.minimumReplicationFactor.guard(rf.fullReplicas, keyspaceName, false, state); int nodeCount = dcsNodes.get(dc).size(); // nodeCount==0 on many tests if (rf.fullReplicas > nodeCount && nodeCount != 0) diff --git a/src/java/org/apache/cassandra/locator/SimpleStrategy.java b/src/java/org/apache/cassandra/locator/SimpleStrategy.java index 1a578e9506..e5b92103b4 100644 --- a/src/java/org/apache/cassandra/locator/SimpleStrategy.java +++ b/src/java/org/apache/cassandra/locator/SimpleStrategy.java @@ -27,10 +27,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.dht.Range; import org.apache.cassandra.dht.Token; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.schema.SchemaConstants; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.StorageService; @@ -100,12 +102,13 @@ public class SimpleStrategy extends AbstractReplicationStrategy } @Override - public void maybeWarnOnOptions() + public void maybeWarnOnOptions(ClientState state) { if (!SchemaConstants.isSystemKeyspace(keyspaceName)) { int nodeCount = StorageService.instance.getHostIdToEndpoint().size(); // nodeCount==0 on many tests + Guardrails.minimumReplicationFactor.guard(rf.fullReplicas, keyspaceName, false, state); if (rf.fullReplicas > nodeCount && nodeCount != 0) { String msg = "Your replication factor " + rf.fullReplicas diff --git a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java index 7978854e31..6d85391d30 100644 --- a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java +++ b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java @@ -308,7 +308,7 @@ public final class KeyspaceMetadata implements SchemaElement name)); } - params.validate(name); + params.validate(name, null); tablesAndViews().forEach(TableMetadata::validate); diff --git a/src/java/org/apache/cassandra/schema/KeyspaceParams.java b/src/java/org/apache/cassandra/schema/KeyspaceParams.java index cc464743ff..539993e2b3 100644 --- a/src/java/org/apache/cassandra/schema/KeyspaceParams.java +++ b/src/java/org/apache/cassandra/schema/KeyspaceParams.java @@ -23,6 +23,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; +import org.apache.cassandra.service.ClientState; + /** * An immutable class representing keyspace parameters (durability and replication). */ @@ -89,9 +91,9 @@ public final class KeyspaceParams return new KeyspaceParams(true, ReplicationParams.nts(args)); } - public void validate(String name) + public void validate(String name, ClientState state) { - replication.validate(name); + replication.validate(name, state); } @Override diff --git a/src/java/org/apache/cassandra/schema/ReplicationParams.java b/src/java/org/apache/cassandra/schema/ReplicationParams.java index 8fbf1cd822..2998aa57ad 100644 --- a/src/java/org/apache/cassandra/schema/ReplicationParams.java +++ b/src/java/org/apache/cassandra/schema/ReplicationParams.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CqlBuilder; import org.apache.cassandra.locator.*; +import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.StorageService; public final class ReplicationParams @@ -70,12 +71,12 @@ public final class ReplicationParams return new ReplicationParams(NetworkTopologyStrategy.class, options); } - public void validate(String name) + public void validate(String name, ClientState state) { // Attempt to instantiate the ARS, which will throw a ConfigurationException if the options aren't valid. TokenMetadata tmd = StorageService.instance.getTokenMetadata(); IEndpointSnitch eps = DatabaseDescriptor.getEndpointSnitch(); - AbstractReplicationStrategy.validateReplicationStrategy(name, klass, tmd, eps, options); + AbstractReplicationStrategy.validateReplicationStrategy(name, klass, tmd, eps, options, state); } public static ReplicationParams fromMap(Map map) { diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 60df67e934..d5cd898c35 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -6505,17 +6505,6 @@ public class StorageService extends NotificationBroadcasterSupport implements IE return DatabaseDescriptor.getDefaultKeyspaceRF(); } - public void setMinimumKeyspaceReplicationFactor(int value) - { - DatabaseDescriptor.setMinimumKeyspaceRF(value); - logger.info("set minimum keyspace rf to {}", value); - } - - public int getMinimumKeyspaceReplicationFactor() - { - return DatabaseDescriptor.getMinimumKeyspaceRF(); - } - public boolean getSkipPaxosRepairOnTopologyChange() { return DatabaseDescriptor.skipPaxosRepairOnTopologyChange(); diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java index dd8f93fd31..fc198c817d 100644 --- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java +++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java @@ -945,8 +945,6 @@ public interface StorageServiceMBean extends NotificationEmitter public void setDefaultKeyspaceReplicationFactor(int value); public int getDefaultKeyspaceReplicationFactor(); - public void setMinimumKeyspaceReplicationFactor(int value); - public int getMinimumKeyspaceReplicationFactor(); boolean getSkipPaxosRepairOnTopologyChange(); void setSkipPaxosRepairOnTopologyChange(boolean v); diff --git a/src/java/org/apache/cassandra/tools/NodeProbe.java b/src/java/org/apache/cassandra/tools/NodeProbe.java index c9c81a874a..e25938aa26 100644 --- a/src/java/org/apache/cassandra/tools/NodeProbe.java +++ b/src/java/org/apache/cassandra/tools/NodeProbe.java @@ -2038,16 +2038,6 @@ public class NodeProbe implements AutoCloseable { return ssProxy.getDefaultKeyspaceReplicationFactor(); } - - public void setMinimumKeyspaceReplicationFactor(int value) - { - ssProxy.setMinimumKeyspaceReplicationFactor(value); - } - - public int getMinimumKeyspaceReplicationFactor() - { - return ssProxy.getMinimumKeyspaceReplicationFactor(); - } } class ColumnFamilyStoreMBeanIterator implements Iterator> diff --git a/src/java/org/apache/cassandra/tools/NodeTool.java b/src/java/org/apache/cassandra/tools/NodeTool.java index 476353fee0..8d87c88906 100644 --- a/src/java/org/apache/cassandra/tools/NodeTool.java +++ b/src/java/org/apache/cassandra/tools/NodeTool.java @@ -144,7 +144,6 @@ public class NodeTool GetInterDCStreamThroughput.class, GetLoggingLevels.class, GetMaxHintWindow.class, - GetMinimumKeyspaceRF.class, GetSSTables.class, GetSeeds.class, GetSnapshotThrottle.class, @@ -204,7 +203,6 @@ public class NodeTool SetInterDCStreamThroughput.class, SetLoggingLevel.class, SetMaxHintWindow.class, - SetMinimumKeyspaceRF.class, SetSnapshotThrottle.class, SetStreamThroughput.class, SetTimeout.class, diff --git a/src/java/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRF.java b/src/java/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRF.java deleted file mode 100644 index 2de94f5f1a..0000000000 --- a/src/java/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRF.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.cassandra.tools.nodetool; - -import io.airlift.airline.Arguments; -import io.airlift.airline.Command; -import org.apache.cassandra.tools.NodeProbe; -import org.apache.cassandra.tools.NodeTool; - -@Command(name = "getminimumrf", description = "Gets minimum keyspace replication factor.") -public class GetMinimumKeyspaceRF extends NodeTool.NodeToolCmd -{ - protected void execute(NodeProbe probe) - { - probe.output().out.println(probe.getMinimumKeyspaceReplicationFactor()); - } -} diff --git a/src/java/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRF.java b/src/java/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRF.java deleted file mode 100644 index 92ab7b72f5..0000000000 --- a/src/java/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRF.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.cassandra.tools.nodetool; - -import io.airlift.airline.Arguments; -import io.airlift.airline.Command; -import org.apache.cassandra.tools.NodeProbe; -import org.apache.cassandra.tools.NodeTool; - -@Command(name = "setminimumrf", description = "Sets minimum keyspace replication factor.") -public class SetMinimumKeyspaceRF extends NodeTool.NodeToolCmd -{ - @Arguments(title = "minimum_rf", usage = "", description = "Minimum replication factor", required = true) - private Integer minimumRF = null; - - protected void execute(NodeProbe probe) - { - probe.setMinimumKeyspaceReplicationFactor(minimumRF); - } -} diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java index c1e771be9f..011b322cba 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java @@ -98,14 +98,11 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.db.guardrails.GuardrailsConfig", "org.apache.cassandra.db.guardrails.GuardrailsConfigMBean", "org.apache.cassandra.db.guardrails.GuardrailsConfig$ConsistencyLevels", - "org.apache.cassandra.db.guardrails.GuardrailsConfig$IntThreshold", "org.apache.cassandra.db.guardrails.GuardrailsConfig$TableProperties", "org.apache.cassandra.config.GuardrailsOptions", "org.apache.cassandra.config.GuardrailsOptions$Config", "org.apache.cassandra.config.GuardrailsOptions$ConsistencyLevels", - "org.apache.cassandra.config.GuardrailsOptions$IntThreshold", "org.apache.cassandra.config.GuardrailsOptions$TableProperties", - "org.apache.cassandra.config.GuardrailsOptions$Threshold", "org.apache.cassandra.config.ReplicaFilteringProtectionOptions", "org.apache.cassandra.config.YamlConfigurationLoader", "org.apache.cassandra.config.YamlConfigurationLoader$PropertiesChecker", @@ -114,8 +111,6 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.config.TransparentDataEncryptionOptions", "org.apache.cassandra.config.SubnetGroups", "org.apache.cassandra.config.TrackWarnings", - "org.apache.cassandra.config.TrackWarnings$LongByteThreshold", - "org.apache.cassandra.config.TrackWarnings$IntByteThreshold", "org.apache.cassandra.db.ConsistencyLevel", "org.apache.cassandra.db.commitlog.CommitLogSegmentManagerFactory", "org.apache.cassandra.db.commitlog.DefaultCommitLogSegmentMgrFactory", @@ -124,7 +119,6 @@ public class DatabaseDescriptorRefTest "org.apache.cassandra.db.commitlog.CommitLogSegmentManagerStandard", "org.apache.cassandra.db.commitlog.CommitLog", "org.apache.cassandra.db.commitlog.CommitLogMBean", - "org.apache.cassandra.db.guardrails.Threshold$Config", "org.apache.cassandra.db.guardrails.Values$Config", "org.apache.cassandra.dht.IPartitioner", "org.apache.cassandra.distributed.api.IInstance", diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index aedaf49991..32f80579e3 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java @@ -756,24 +756,4 @@ public class DatabaseDescriptorTest { DatabaseDescriptor.setDefaultKeyspaceRF(0); } - - @Test (expected = ConfigurationException.class) - public void testInvalidSub0MinimumRFs() throws ConfigurationException - { - DatabaseDescriptor.setMinimumKeyspaceRF(-1); - } - - @Test (expected = ConfigurationException.class) - public void testDefaultRfLessThanMinRF() - { - DatabaseDescriptor.setMinimumKeyspaceRF(2); - DatabaseDescriptor.setDefaultKeyspaceRF(1); - } - - @Test (expected = ConfigurationException.class) - public void testMinimumRfGreaterThanDefaultRF() - { - DatabaseDescriptor.setDefaultKeyspaceRF(1); - DatabaseDescriptor.setMinimumKeyspaceRF(2); - } } diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java index a394d9593b..c4a376648d 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java @@ -435,30 +435,6 @@ public class AlterTest extends CQLTester execute(String.format("DROP KEYSPACE IF EXISTS %s", ks6)); } - @Test - public void testMinimumRF() throws Throwable - { - DatabaseDescriptor.setDefaultKeyspaceRF(3); - DatabaseDescriptor.setMinimumKeyspaceRF(2); - - String ks1 = createKeyspace("CREATE KEYSPACE %s WITH replication={ 'class' : 'SimpleStrategy' }"); - String ks2 = createKeyspace("CREATE KEYSPACE %s WITH replication={ 'class' : 'NetworkTopologyStrategy' }"); - - assertAlterTableThrowsException(ConfigurationException.class, - String.format("Replication factor cannot be less than minimum_keyspace_rf (%s), found %s", DatabaseDescriptor.getMinimumKeyspaceRF(), "1"), - String.format("ALTER KEYSPACE %s WITH replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }", ks1)); - assertAlterTableThrowsException(ConfigurationException.class, - String.format("Replication factor cannot be less than minimum_keyspace_rf (%s), found %s", DatabaseDescriptor.getMinimumKeyspaceRF(), "1"), - String.format("ALTER KEYSPACE %s WITH replication={ 'class' : 'NetworkTopologyStrategy', '" + DATA_CENTER + "' : '1' }", ks2)); - - //clean up config change - DatabaseDescriptor.setMinimumKeyspaceRF(0); - DatabaseDescriptor.setDefaultKeyspaceRF(1); - - //clean up keyspaces - execute(String.format("DROP KEYSPACE IF EXISTS %s", ks1)); - execute(String.format("DROP KEYSPACE IF EXISTS %s", ks2)); - } /** * Test {@link ConfigurationException} thrown when altering a keyspace to invalid DC option in replication configuration. diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java index b1a5c6b86c..0773c97531 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java @@ -738,26 +738,4 @@ public class CreateTest extends CQLTester } } - @Test - public void testMinimumRF() - { - try - { - DatabaseDescriptor.setDefaultKeyspaceRF(3); - DatabaseDescriptor.setMinimumKeyspaceRF(2); - - assertThrowsConfigurationException( - String.format("Replication factor cannot be less than minimum_keyspace_rf (%s), found %s", DatabaseDescriptor.getMinimumKeyspaceRF(), "1"), - "CREATE KEYSPACE ks1 WITH replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }"); - - assertThrowsConfigurationException( - String.format("Replication factor cannot be less than minimum_keyspace_rf (%s), found %s", DatabaseDescriptor.getMinimumKeyspaceRF(), "1"), - "CREATE KEYSPACE ks2 WITH replication={ 'class' : 'NetworkTopologyStrategy', 'replication_factor' : 1 }"); - } - finally - { - DatabaseDescriptor.setMinimumKeyspaceRF(0); - DatabaseDescriptor.setDefaultKeyspaceRF(1); - } - } } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java index ca0c41d724..0c473353ee 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java @@ -140,7 +140,7 @@ public class GuardrailColumnsPerTableTest extends ThresholdTester private void assertCreateTableValid(String query) throws Throwable { - assertThresholdValid(format(query, keyspace() + '.' + createTableName())); + assertMaxThresholdValid(format(query, keyspace() + '.' + createTableName())); } private void assertDropColumnValid(String query) throws Throwable diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java index 2c5b3d5fba..6c600933b7 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java @@ -82,7 +82,7 @@ public class GuardrailKeyspacesTest extends ThresholdTester private String assertCreateKeyspaceValid() throws Throwable { String keyspaceName = createKeyspaceName(); - assertThresholdValid(createKeyspaceQuery(keyspaceName)); + assertMaxThresholdValid(createKeyspaceQuery(keyspaceName)); return keyspaceName; } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailMinimumReplicationFactorTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailMinimumReplicationFactorTest.java new file mode 100644 index 0000000000..16d201e1df --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailMinimumReplicationFactorTest.java @@ -0,0 +1,277 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import org.apache.cassandra.ServerTestUtils; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.locator.AbstractEndpointSnitch; +import org.apache.cassandra.locator.IEndpointSnitch; +import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.locator.Replica; +import org.apache.cassandra.service.ClientWarn; +import org.apache.cassandra.service.StorageService; +import org.assertj.core.api.Assertions; + +import static java.lang.String.format; +import static org.junit.Assert.assertNotNull; + +public class GuardrailMinimumReplicationFactorTest extends ThresholdTester +{ + private static final int MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD = 4; + private static int MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD = 1; + private static final int DEFAULT_REPLICATION_FACTOR = 2; + private static final int DISABLED_GUARDRAIL = -1; + private static final String WHAT = "minimum_replication_factor"; + private static final String DATACENTER1 = "datacenter1"; + private static final String KS = "ks"; + private final TriConsumer setter; + + public GuardrailMinimumReplicationFactorTest() + { + super(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, + MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD, + Guardrails.minimumReplicationFactor, + Guardrails::setMinimumReplicationFactorThreshold, + Guardrails::getMinimumReplicationFactorWarnThreshold, + Guardrails::getMinimumReplicationFactorFailThreshold); + + this.setter = Guardrails::setMinimumReplicationFactorThreshold; + } + + @Before + public void setupTest() throws Throwable + { + DatabaseDescriptor.setDefaultKeyspaceRF(DEFAULT_REPLICATION_FACTOR); + MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD = 2; + } + + @After + public void cleanupTest() throws Throwable + { + execute("DROP KEYSPACE IF EXISTS ks"); + } + + @Override + protected long currentValue() + { + return Long.parseLong((Keyspace.open(KS).getReplicationStrategy()).configOptions.get(DATACENTER1)); + } + + @Override + protected List getWarnings() + { + List warnings = ClientWarn.instance.getWarnings(); + + return warnings == null + ? Collections.emptyList() + : warnings.stream() + .filter(w -> !w.contains("keyspace ks is higher than the number of nodes 1 for datacenter") && + !w.contains("When increasing replication factor you need to run a full (-full) repair to distribute the data") && + !w.contains("keyspace ks is higher than the number of nodes") && + !w.contains("Your replication factor 4 for keyspace ks is higher than the number of nodes 2 for datacenter datacenter2")) + .collect(Collectors.toList()); + } + + @Test + public void testConfigValidation() + { + assertNotNull(guardrail); + setter.accept(guardrails(), DISABLED_GUARDRAIL, DISABLED_GUARDRAIL); + + assertInvalidPositiveIntProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), Integer.MIN_VALUE, Integer.MAX_VALUE, WHAT + "_fail_threshold"); + assertInvalidPositiveIntProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), -2, Integer.MAX_VALUE, WHAT + "_fail_threshold"); + assertValidProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), DISABLED_GUARDRAIL); + assertInvalidPositiveIntProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), 0, Integer.MAX_VALUE, WHAT + "_fail_threshold"); + assertValidProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), 1); + assertValidProperty((g, a) -> setter.accept(g, DISABLED_GUARDRAIL, a), 2); + + assertInvalidPositiveIntProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), Integer.MIN_VALUE, Integer.MAX_VALUE, WHAT + "_warn_threshold"); + assertInvalidPositiveIntProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), -2, Integer.MAX_VALUE, WHAT + "_warn_threshold"); + assertValidProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), DISABLED_GUARDRAIL); + assertInvalidPositiveIntProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), 0, Integer.MAX_VALUE, WHAT + "_warn_threshold"); + assertValidProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), 1); + assertValidProperty((g, w) -> setter.accept(g, w, DISABLED_GUARDRAIL), 2); + + Assertions.assertThatThrownBy(() -> setter.accept(guardrails(), 1, 2)) + .hasMessageContaining(guardrail.name + "_warn_threshold should be greater than the fail threshold"); + } + + @Test + public void testMinKeyspaceRFDisabled() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(DISABLED_GUARDRAIL, DISABLED_GUARDRAIL); + assertMinThresholdValid("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 1}"); + assertMinThresholdValid("ALTER KEYSPACE ks WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 3}"); + } + + @Test + public void testSimpleStrategy() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertWarns("CREATE KEYSPACE ks WITH replication = { 'class': 'SimpleStrategy', 'replication_factor': 3}", + format("The keyspace %s has a replication factor of 3, below the warning threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + assertFails("ALTER KEYSPACE ks WITH replication = { 'class': 'SimpleStrategy', 'replication_factor': 1}", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + } + + @Test + public void testMultipleDatacenter() throws Throwable + { + IEndpointSnitch snitch = DatabaseDescriptor.getEndpointSnitch(); + DatabaseDescriptor.setEndpointSnitch(new AbstractEndpointSnitch() + { + public static final String RACK1 = ServerTestUtils.RACK1; + + @Override + public String getRack(InetAddressAndPort endpoint) { return RACK1; } + + @Override + public String getDatacenter(InetAddressAndPort endpoint) { return "datacenter2"; } + + @Override + public int compareEndpoints(InetAddressAndPort target, Replica a1, Replica a2) { return 0; } + }); + + List twoWarnings = Arrays.asList(format("The keyspace %s has a replication factor of 2, below the warning threshold of %d.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD), + format("The keyspace %s has a replication factor of 2, below the warning threshold of %d.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + + StorageService.instance.getTokenMetadata().updateHostId(UUID.randomUUID(), InetAddressAndPort.getByName("127.0.0.255")); + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertValid("CREATE KEYSPACE ks WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 4, 'datacenter2' : 4 };"); + assertWarns("ALTER KEYSPACE ks WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 4, 'datacenter2' : 2 };", + format("The keyspace %s has a replication factor of 2, below the warning threshold of %d.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + assertWarns("ALTER KEYSPACE ks WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 2, 'datacenter2' : 2 };", twoWarnings); + assertFails("ALTER KEYSPACE ks WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 4, 'datacenter2' : 1 };", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %d.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + assertFails("CREATE KEYSPACE ks1 WITH replication = { 'class' : 'NetworkTopologyStrategy', 'datacenter1': 1, 'datacenter2' : 1 };", + format("The keyspace ks1 has a replication factor of 1, below the failure threshold of %d.", MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + + DatabaseDescriptor.setEndpointSnitch(snitch); + execute("DROP KEYSPACE IF EXISTS ks1"); + } + + @Test + public void testMinKeyspaceRFOnlyWarnAbove() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, DISABLED_GUARDRAIL); + assertMinThresholdValid("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 6}"); + assertMinThresholdValid("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 5}"); + } + + @Test + public void testMinKeyspaceRFOnlyWarnBelow() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, DISABLED_GUARDRAIL); + assertWarns("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 3}", + format("The keyspace %s has a replication factor of 3, below the warning threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + assertWarns("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 2}", + format("The keyspace %s has a replication factor of 2, below the warning threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + } + + @Test + public void testMinKeyspaceRFOnlyFailAbove() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(DISABLED_GUARDRAIL, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertMinThresholdValid("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 4}"); + assertMinThresholdValid("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 2}"); + } + + @Test + public void testMinKeyspaceRFOnlyFailBelow() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(DISABLED_GUARDRAIL, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertFails("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 1}", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + } + + @Test + public void testMinKeyspaceRFOnlyFailBelowAlter() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(DISABLED_GUARDRAIL, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + execute("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 3}"); + assertFails("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 1}", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + } + + @Test + public void testMinKeyspaceRFWarnAbove() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertMinThresholdValid("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 6}"); + assertMinThresholdValid("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 5}"); + } + + @Test + public void testMinKeyspaceRFWarnFailBetween() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertWarns("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 3}", + format("The keyspace %s has a replication factor of 3, below the warning threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + assertWarns("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 2}", + format("The keyspace %s has a replication factor of 2, below the warning threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD)); + } + + @Test + public void testMinKeyspaceRFFailBelow() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + assertFails("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 1}", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + } + + @Test + public void testMinKeyspaceRFFailBelowAlter() throws Throwable + { + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + execute("CREATE KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 4}"); + assertFails("ALTER KEYSPACE ks WITH replication = { 'class': 'NetworkTopologyStrategy', 'datacenter1': 1}", + format("The keyspace %s has a replication factor of 1, below the failure threshold of %s.", KS, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD)); + } + + @Test + public void testMinRFGreaterThanDefaultRF() + { + try + { + DatabaseDescriptor.setDefaultKeyspaceRF(1); + guardrails().setMinimumReplicationFactorThreshold(MINIMUM_REPLICATION_FACTOR_WARN_THRESHOLD, MINIMUM_REPLICATION_FACTOR_FAIL_THRESHOLD); + } + catch (ConfigurationException e) + { + String expectedMessage = ""; + + if(guardrails().getMinimumReplicationFactorFailThreshold() > DatabaseDescriptor.getDefaultKeyspaceRF()) + expectedMessage = format("%s_fail_threshold to be set (%d) cannot be greater than default_keyspace_rf (%d)", + WHAT, guardrails().getMinimumReplicationFactorFailThreshold(), DatabaseDescriptor.getDefaultKeyspaceRF()); + Assertions.assertThat(e.getMessage()).contains(expectedMessage); + } + } +} diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java index 102da71c8a..8591f823af 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java @@ -92,7 +92,7 @@ public class GuardrailSecondaryIndexesPerTable extends ThresholdTester private void assertCreateIndexSucceeds(String column, String indexName) throws Throwable { - assertThresholdValid(format("CREATE INDEX %s ON %s.%s(%s)", indexName, keyspace(), currentTable(), column)); + assertMaxThresholdValid(format("CREATE INDEX %s ON %s.%s(%s)", indexName, keyspace(), currentTable(), column)); } private void assertCreateIndexWarns(String column, String indexName) throws Throwable diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java index 969b50c668..7e69636600 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java @@ -83,7 +83,7 @@ public class GuardrailTablesTest extends ThresholdTester private String assertCreateTableValid() throws Throwable { String tableName = createTableName(); - assertThresholdValid(createTableQuery(tableName)); + assertMaxThresholdValid(createTableQuery(tableName)); return tableName; } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java index 986e1d698f..7c94702a21 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java @@ -416,7 +416,7 @@ public abstract class GuardrailTester extends CQLTester assertTrue(format("Expect no warning messages but got %s", warnings), warnings.isEmpty()); } - private List getWarnings() + protected List getWarnings() { List warnings = ClientWarn.instance.getWarnings(); diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java index edff3172f7..3be58b00cb 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java @@ -101,7 +101,7 @@ public class GuardrailViewsPerTableTest extends ThresholdTester private String assertCreateViewSucceeds() throws Throwable { String viewName = createViewName(); - assertThresholdValid(format(CREATE_VIEW, viewName)); + assertMaxThresholdValid(format(CREATE_VIEW, viewName)); return viewName; } diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java index 991dcf7cd4..e99b736b03 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java @@ -36,7 +36,7 @@ public class GuardrailsConfigProviderTest extends GuardrailTester { String name = getClass().getCanonicalName() + '$' + CustomProvider.class.getSimpleName(); GuardrailsConfigProvider provider = GuardrailsConfigProvider.build(name); - Threshold guard = new Threshold("test_guardrail", + MaxThreshold guard = new MaxThreshold("test_guardrail", state -> provider.getOrCreate(state).getTablesWarnThreshold(), state -> provider.getOrCreate(state).getTablesFailThreshold(), (isWarn, what, v, t) -> format("%s: for %s, %s > %s", diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java index b278e7f6f8..a0a5823b01 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java @@ -39,12 +39,6 @@ public class GuardrailsTest extends GuardrailTester { public static final int DISABLED = -1; - @Test - public void testDisabledThreshold() throws Throwable - { - Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger"; - testDisabledThreshold(new Threshold("x", state -> DISABLED, state -> DISABLED, errorMessageProvider)); - } private void testDisabledThreshold(Threshold guard) throws Throwable { @@ -61,9 +55,16 @@ public class GuardrailsTest extends GuardrailTester } @Test - public void testThreshold() throws Throwable + public void testDisabledMaxThreshold() throws Throwable { - Threshold guard = new Threshold("x", + Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger"; + testDisabledThreshold(new MaxThreshold("x", state -> DISABLED, state -> DISABLED, errorMessageProvider)); + } + + @Test + public void testMaxThreshold() throws Throwable + { + MaxThreshold guard = new MaxThreshold("x", state -> 10, state -> 100, (isWarn, what, v, t) -> format("%s: for %s, %s > %s", @@ -87,9 +88,9 @@ public class GuardrailsTest extends GuardrailTester } @Test - public void testWarnOnlyThreshold() throws Throwable + public void testWarnOnlyMaxThreshold() throws Throwable { - Threshold guard = new Threshold("x", + MaxThreshold guard = new MaxThreshold("x", state -> 10, state -> DISABLED, (isWarn, what, v, t) -> format("%s: for %s, %s > %s", @@ -105,9 +106,9 @@ public class GuardrailsTest extends GuardrailTester } @Test - public void testFailOnlyThreshold() throws Throwable + public void testFailOnlyMaxThreshold() throws Throwable { - Threshold guard = new Threshold("x", + MaxThreshold guard = new MaxThreshold("x", state -> DISABLED, state -> 10, (isWarn, what, v, t) -> format("%s: for %s, %s > %s", @@ -123,9 +124,9 @@ public class GuardrailsTest extends GuardrailTester } @Test - public void testThresholdUsers() throws Throwable + public void testMaxThresholdUsers() throws Throwable { - Threshold guard = new Threshold("x", + MaxThreshold guard = new MaxThreshold("x", state -> 10, state -> 100, (isWarn, what, v, t) -> format("%s: for %s, %s > %s", @@ -151,6 +152,104 @@ public class GuardrailsTest extends GuardrailTester assertValid(() -> guard.guard(101, "z", false, superClientState)); } + @Test + public void testDisabledMinThreshold() throws Throwable + { + Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what, v, t) -> "Should never trigger"; + testDisabledThreshold(new MinThreshold("x", state -> DISABLED, state -> DISABLED, errorMessageProvider)); + } + + @Test + public void testMinThreshold() throws Throwable + { + MinThreshold guard = new MinThreshold("x", + state -> 100, + state -> 10, + (isWarn, what, v, t) -> format("%s: for %s, %s < %s", + isWarn ? "Warning" : "Aborting", what, v, t)); + + assertTrue(guard.enabled(userClientState)); + + assertValid(() -> guard.guard(200, "Z", false, userClientState)); + assertWarns(() -> guard.guard(25, "A", false, userClientState), "Warning: for A, 25 < 100"); + assertWarns(() -> guard.guard(10, "B", false, userClientState), "Warning: for B, 10 < 100"); + assertFails(() -> guard.guard(9, "X", false, userClientState), "Aborting: for X, 9 < 10"); + assertFails(() -> guard.guard(1, "Y", false, userClientState), "Aborting: for Y, 1 < 10"); + assertValid(() -> guard.guard(200, "Z", false, userClientState)); + + assertValid(() -> guard.guard(200, "Z", true, userClientState)); + assertWarns(() -> guard.guard(25, "A", true, userClientState), "Warning: for A, 25 < 100", "Warning: for , 25 < 100"); + assertWarns(() -> guard.guard(10, "B", true, userClientState), "Warning: for B, 10 < 100", "Warning: for , 10 < 100"); + assertFails(() -> guard.guard(9, "X", true, userClientState), "Aborting: for X, 9 < 10", "Aborting: for , 9 < 10"); + assertFails(() -> guard.guard(1, "Y", true, userClientState), "Aborting: for Y, 1 < 10", "Aborting: for , 1 < 10"); + assertValid(() -> guard.guard(200, "Z", true, userClientState)); + } + + @Test + public void testWarnOnlyMinThreshold() throws Throwable + { + MinThreshold guard = new MinThreshold("x", + state -> 10, + state -> DISABLED, + (isWarn, what, v, t) -> format("%s: for %s, %s < %s", + isWarn ? "Warning" : "Aborting", what, v, t)); + + assertTrue(guard.enabled(userClientState)); + + assertValid(() -> guard.guard(11, "Z", false, userClientState)); + assertWarns(() -> guard.guard(5, "A", false, userClientState), "Warning: for A, 5 < 10"); + + assertValid(() -> guard.guard(11, "Z", true, userClientState)); + assertWarns(() -> guard.guard(5, "A", true, userClientState), "Warning: for A, 5 < 10", "Warning: for , 5 < 10"); + } + + @Test + public void testFailOnlyMinThreshold() throws Throwable + { + MinThreshold guard = new MinThreshold("x", + state -> DISABLED, + state -> 10, + (isWarn, what, v, t) -> format("%s: for %s, %s < %s", + isWarn ? "Warning" : "Aborting", what, v, t)); + + assertTrue(guard.enabled(userClientState)); + + assertValid(() -> guard.guard(11, "Z", false, userClientState)); + assertFails(() -> guard.guard(5, "A", false, userClientState), "Aborting: for A, 5 < 10"); + + assertValid(() -> guard.guard(11, "Z", true, userClientState)); + assertFails(() -> guard.guard(5, "A", true, userClientState), "Aborting: for A, 5 < 10", "Aborting: for , 5 < 10"); + } + + @Test + public void testMinThresholdUsers() throws Throwable + { + MinThreshold guard = new MinThreshold("x", + state -> 100, + state -> 10, + (isWarn, what, v, t) -> format("%s: for %s, %s < %s", + isWarn ? "Warning" : "Failure", what, v, t)); + + // value above both thresholds + assertValid(() -> guard.guard(200, "x", false, null)); + assertValid(() -> guard.guard(200, "x", false, userClientState)); + assertValid(() -> guard.guard(200, "x", false, systemClientState)); + assertValid(() -> guard.guard(200, "x", false, superClientState)); + + // value under warning threshold + assertWarns(() -> guard.guard(10, "y", false, null), "Warning: for y, 10 < 100"); + assertWarns(() -> guard.guard(10, "y", false, userClientState), "Warning: for y, 10 < 100"); + assertValid(() -> guard.guard(10, "y", false, systemClientState)); + assertValid(() -> guard.guard(10, "y", false, superClientState)); + + // value under fail threshold. An undefined user means that the check comes from a background process, so we + // still emit failure messages and events, but we don't throw an exception to prevent interrupting that process. + assertFails(() -> guard.guard(9, "z", false, null), false, "Failure: for z, 9 < 10"); + assertFails(() -> guard.guard(9, "z", false, userClientState), "Failure: for z, 9 < 10"); + assertValid(() -> guard.guard(9, "z", false, systemClientState)); + assertValid(() -> guard.guard(9, "z", false, superClientState)); + } + @Test public void testDisableFlag() throws Throwable { diff --git a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java index a04cc9933b..13ce67d142 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java +++ b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java @@ -136,15 +136,41 @@ public abstract class ThresholdTester extends GuardrailTester .hasMessageContaining(guardrail.name + "_warn_threshold should be lower than the fail threshold"); } - protected void assertThresholdValid(String query) throws Throwable + protected void assertMaxThresholdValid(String query) throws Throwable { assertValid(query); - Assertions.assertThat(currentValue()) - .isLessThanOrEqualTo(warnGetter.applyAsLong(guardrails())) - .isLessThanOrEqualTo(failGetter.applyAsLong(guardrails())); + long warnValue = warnGetter.applyAsLong(guardrails()); + long failValue = failGetter.applyAsLong(guardrails()); + long current = currentValue(); + + if (warnValue != disabledValue) + Assertions.assertThat(current) + .isLessThanOrEqualTo(warnValue); + + if (failValue != disabledValue) + Assertions.assertThat(current) + .isLessThanOrEqualTo(failValue); } + protected void assertMinThresholdValid(String query) throws Throwable + { + assertValid(query); + + long warnValue = warnGetter.applyAsLong(guardrails()); + long failValue = failGetter.applyAsLong(guardrails()); + long current = currentValue(); + + if (warnValue != disabledValue) + Assertions.assertThat(current) + .isGreaterThanOrEqualTo(warnValue); + + if (failValue != disabledValue) + Assertions.assertThat(current) + .isGreaterThanOrEqualTo(failValue); + } + + protected void assertThresholdWarns(String query, String message) throws Throwable { assertThresholdWarns(query, message, message); @@ -193,7 +219,7 @@ public abstract class ThresholdTester extends GuardrailTester .isEqualTo(failGetter.applyAsLong(guardrails())); } - private void assertInvalidPositiveProperty(BiConsumer setter, + protected void assertInvalidPositiveProperty(BiConsumer setter, long value, long maxValue, String name) @@ -238,6 +264,13 @@ public abstract class ThresholdTester extends GuardrailTester assertInvalidPositiveProperty(setter, value, maxValue, name); } + protected void assertInvalidPositiveIntProperty (BiConsumer setter, int value, + int maxValue, + String name) + { + assertInvalidPositiveProperty((g, l) -> setter.accept(g, l.intValue()), (long) value, maxValue, name); + } + protected void testValidationOfStrictlyPositiveProperty(BiConsumer setter, String name) { assertInvalidStrictlyPositiveProperty(setter, Integer.MIN_VALUE, name); diff --git a/test/unit/org/apache/cassandra/locator/NetworkTopologyStrategyTest.java b/test/unit/org/apache/cassandra/locator/NetworkTopologyStrategyTest.java index 8ab1bfa953..45ba5d8f1d 100644 --- a/test/unit/org/apache/cassandra/locator/NetworkTopologyStrategyTest.java +++ b/test/unit/org/apache/cassandra/locator/NetworkTopologyStrategyTest.java @@ -480,7 +480,7 @@ public class NetworkTopologyStrategyTest StorageService.instance.getTokenMetadata().updateHostId(UUID.randomUUID(), FBUtilities.getBroadcastAddressAndPort()); ClientWarn.instance.captureWarnings(); - strategy.maybeWarnOnOptions(); + strategy.maybeWarnOnOptions(null); assertTrue(ClientWarn.instance.getWarnings().stream().anyMatch(s -> s.contains("Your replication factor"))); } } diff --git a/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java b/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java index 1665d01a33..809b6fc989 100644 --- a/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java +++ b/test/unit/org/apache/cassandra/locator/SimpleStrategyTest.java @@ -349,7 +349,7 @@ public class SimpleStrategyTest StorageService.instance.getTokenMetadata().updateHostId(UUID.randomUUID(), FBUtilities.getBroadcastAddressAndPort()); ClientWarn.instance.captureWarnings(); - strategy.maybeWarnOnOptions(); + strategy.maybeWarnOnOptions(null); assertTrue(ClientWarn.instance.getWarnings().stream().anyMatch(s -> s.contains("Your replication factor"))); } diff --git a/test/unit/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRFTest.java b/test/unit/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRFTest.java deleted file mode 100644 index d2ff34b912..0000000000 --- a/test/unit/org/apache/cassandra/tools/nodetool/GetMinimumKeyspaceRFTest.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.cassandra.tools.nodetool; - -import org.junit.BeforeClass; -import org.junit.Test; - -import org.apache.cassandra.config.DatabaseDescriptor; -import org.apache.cassandra.cql3.CQLTester; -import org.apache.cassandra.tools.ToolRunner; -import org.assertj.core.api.Assertions; - -import static org.assertj.core.api.Assertions.assertThat; - -public class GetMinimumKeyspaceRFTest extends CQLTester -{ - @BeforeClass - public static void setup() throws Exception - { - requireNetwork(); - startJMXServer(); - } - - @Test - @SuppressWarnings("SingleCharacterStringConcatenation") - public void testMaybeChangeDocs() - { - // If you added, modified options or help, please update docs if necessary - ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("help", "getminimumrf"); - tool.assertOnCleanExit(); - - String help = "NAME\n" + - " nodetool getminimumrf - Gets minimum keyspace replication factor.\n" + - "\n" + - "SYNOPSIS\n" + - " nodetool [(-h | --host )] [(-p | --port )]\n" + - " [(-pp | --print-port)] [(-pw | --password )]\n" + - " [(-pwf | --password-file )]\n" + - " [(-u | --username )] getminimumrf\n" + - "\n" + - "OPTIONS\n" + - " -h , --host \n" + - " Node hostname or ip address\n" + - "\n" + - " -p , --port \n" + - " Remote jmx agent port number\n" + - "\n" + - " -pp, --print-port\n" + - " Operate in 4.0 mode with hosts disambiguated by port number\n" + - "\n" + - " -pw , --password \n" + - " Remote jmx agent password\n" + - "\n" + - " -pwf , --password-file \n" + - " Path to the JMX password file\n" + - "\n" + - " -u , --username \n" + - " Remote jmx agent username\n" + - "\n" + - "\n"; - assertThat(tool.getStdout()).isEqualTo(help); - } - - @Test - public void testGetMinimumRF() - { - ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("getminimumrf"); - tool.assertOnCleanExit(); - assertThat(tool.getStdout().trim()).isEqualTo(Integer.toString(DatabaseDescriptor.getMinimumKeyspaceRF())); - } -} \ No newline at end of file diff --git a/test/unit/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRFTest.java b/test/unit/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRFTest.java deleted file mode 100644 index 09d3625d50..0000000000 --- a/test/unit/org/apache/cassandra/tools/nodetool/SetMinimumKeyspaceRFTest.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.cassandra.tools.nodetool; - -import org.junit.BeforeClass; -import org.junit.Test; - -import org.apache.cassandra.config.DatabaseDescriptor; -import org.apache.cassandra.cql3.CQLTester; -import org.apache.cassandra.tools.ToolRunner; - -import static org.assertj.core.api.Assertions.assertThat; - -public class SetMinimumKeyspaceRFTest extends CQLTester -{ - @BeforeClass - public static void setup() throws Exception - { - requireNetwork(); - startJMXServer(); - } - - @Test - @SuppressWarnings("SingleCharacterStringConcatenation") - public void testMaybeChangeDocs() - { - // If you added, modified options or help, please update docs if necessary - ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("help", "setminimumrf"); - tool.assertOnCleanExit(); - - String help = "NAME\n" + - " nodetool setminimumrf - Sets minimum keyspace replication factor.\n" + - "\n" + - "SYNOPSIS\n" + - " nodetool [(-h | --host )] [(-p | --port )]\n" + - " [(-pp | --print-port)] [(-pw | --password )]\n" + - " [(-pwf | --password-file )]\n" + - " [(-u | --username )] setminimumrf [--] \n" + - "\n" + - "OPTIONS\n" + - " -h , --host \n" + - " Node hostname or ip address\n" + - "\n" + - " -p , --port \n" + - " Remote jmx agent port number\n" + - "\n" + - " -pp, --print-port\n" + - " Operate in 4.0 mode with hosts disambiguated by port number\n" + - "\n" + - " -pw , --password \n" + - " Remote jmx agent password\n" + - "\n" + - " -pwf , --password-file \n" + - " Path to the JMX password file\n" + - "\n" + - " -u , --username \n" + - " Remote jmx agent username\n" + - "\n" + - " --\n" + - " This option can be used to separate command-line options from the\n" + - " list of argument, (useful when arguments might be mistaken for\n" + - " command-line options\n" + - "\n" + - " \n" + - " Minimum replication factor\n" + - "\n" + - "\n"; - assertThat(tool.getStdout()).isEqualTo(help); - } - - @Test - public void testSetMinimumRF() - { - ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("setminimumrf", "1"); - tool.assertOnCleanExit(); - assertThat(DatabaseDescriptor.getMinimumKeyspaceRF()).isEqualTo(1); - } -}