This is an automated email from the ASF dual-hosted git repository.
adelapena pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 20175bf77e Remove guardrails global enable flag
20175bf77e is described below
commit 20175bf77e2c6f72c25240ee445b583805a37630
Author: Savni Nagarkar <savni_nagar...@apple.com>
AuthorDate: Tue Mar 29 14:40:55 2022 -0500
Remove guardrails global enable flag
patch by Savni Nagarkar; reviewed by Andrés de la Peña and Joshua McKenzie
for CASSANDRA-17499
---
CHANGES.txt | 1 +
conf/cassandra.yaml | 2 -
src/java/org/apache/cassandra/config/Config.java | 1 -
.../apache/cassandra/config/GuardrailsOptions.java | 22 -----------
.../statements/schema/CreateTableStatement.java | 45 ++++++++++------------
.../apache/cassandra/db/guardrails/Guardrail.java | 8 ++--
.../apache/cassandra/db/guardrails/Guardrails.java | 24 ------------
.../cassandra/db/guardrails/GuardrailsConfig.java | 10 -----
.../cassandra/db/guardrails/GuardrailsMBean.java | 14 -------
.../GuardrailCollectionSizeOnSSTableWriteTest.java | 1 -
...rdrailItemsPerCollectionOnSSTableWriteTest.java | 3 +-
.../cassandra/db/guardrails/GuardrailTester.java | 1 -
12 files changed, 26 insertions(+), 106 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index d450f81726..114859c755 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.1
+ * Remove guardrails global enable flag (CASSANDRA-17499)
* Clients using JMX are unable to handle non-standard java types but we leak
this into our interfaces (CASSANDRA-17527)
* Remove stress server functionality (CASSANDRA-17535)
* Reduce histogram snapshot long[] allocation overhead during speculative
read and write threshold updates (CASSANDRA-17523)
diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml
index 1a876ca934..668ab83db4 100644
--- a/conf/cassandra.yaml
+++ b/conf/cassandra.yaml
@@ -1599,8 +1599,6 @@ drop_compact_storage_enabled: false
# warn_threshold_kb: 0
# abort_threshold_kb: 0
-# Whether guardrails are enabled or not. Guardrails are disabled by default.
-# guardrails_enabled: false
# Guardrail to warn or fail when creating more user keyspaces than threshold.
# The two thresholds default to -1 to disable.
# keyspaces_warn_threshold: -1
diff --git a/src/java/org/apache/cassandra/config/Config.java
b/src/java/org/apache/cassandra/config/Config.java
index e0dc5767c6..9717dbc135 100644
--- a/src/java/org/apache/cassandra/config/Config.java
+++ b/src/java/org/apache/cassandra/config/Config.java
@@ -775,7 +775,6 @@ public class Config
public static final int DISABLED_GUARDRAIL = -1;
public static final DataStorageSpec DISABLED_SIZE_GUARDRAIL =
DataStorageSpec.inBytes(0);
- public volatile boolean guardrails_enabled = false;
public volatile int keyspaces_warn_threshold = DISABLED_GUARDRAIL;
public volatile int keyspaces_fail_threshold = DISABLED_GUARDRAIL;
public volatile int tables_warn_threshold = DISABLED_GUARDRAIL;
diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
index d08f8c555a..5047cbaa2b 100644
--- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
+++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
@@ -42,9 +42,6 @@ import static java.util.stream.Collectors.toSet;
* code checking each guarded constraint. That code should use the higher
level abstractions defined in
* {@link Guardrails}).
*
- * <p>This contains a main setting, {@code enabled}, controlling if guardrails
are globally active or not, and
- * individual settings to control each guardrail.
- *
* <p>We have 2 variants of guardrails, soft (warn) and hard (fail) limits,
each guardrail having either one of the
* variants or both. Note in particular that hard limits only make sense for
guardrails triggering during query
* execution. For other guardrails, say one triggering during compaction,
aborting that compaction does not make sense.
@@ -82,25 +79,6 @@ public class GuardrailsOptions implements GuardrailsConfig
validateIntThreshold(config.fields_per_udt_warn_threshold,
config.fields_per_udt_fail_threshold, "fields_per_udt");
}
- @Override
- public boolean getEnabled()
- {
- return config.guardrails_enabled;
- }
-
- /**
- * Enable/disable guardrails.
- *
- * @param enabled {@code true} for enabling guardrails, {@code false} for
disabling them.
- */
- public void setEnabled(boolean enabled)
- {
- updatePropertyWithLogging("guardrails_enabled",
- enabled,
- () -> config.guardrails_enabled,
- x -> config.guardrails_enabled = x);
- }
-
@Override
public int getKeyspacesWarnThreshold()
{
diff --git
a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
index 869a062146..1a625380cb 100644
---
a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
+++
b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
@@ -128,32 +128,27 @@ public final class CreateTableStatement extends
AlterSchemaStatement
{
super.validate(state);
- // since there are multiple guardrails guarding this, we first check
that guardrails are globally enabled as an
- // optimization for the case where they are disabled, so we don't have
to do the same check on every guardrail
- if (Guardrails.enabled(state))
- {
- // Guardrail on table properties
- Guardrails.tableProperties.guard(attrs.updatedProperties(),
attrs::removeProperty, state);
-
- // Guardrail on columns per table
- Guardrails.columnsPerTable.guard(rawColumns.size(), tableName,
false, state);
-
- // Guardrail on number of tables
- if (Guardrails.tables.enabled(state))
- {
- int totalUserTables = Schema.instance.getUserKeyspaces()
- .stream()
- .map(ksm -> ksm.name)
- .map(Keyspace::open)
- .mapToInt(keyspace ->
keyspace.getColumnFamilyStores().size())
- .sum();
- Guardrails.tables.guard(totalUserTables + 1, tableName, false,
state);
- }
-
- // Guardrail to check whether creation of new COMPACT STORAGE
tables is allowed
- if (useCompactStorage)
- Guardrails.compactTablesEnabled.ensureEnabled(state);
+ // Guardrail on table properties
+ Guardrails.tableProperties.guard(attrs.updatedProperties(),
attrs::removeProperty, state);
+
+ // Guardrail on columns per table
+ Guardrails.columnsPerTable.guard(rawColumns.size(), tableName, false,
state);
+
+ // Guardrail on number of tables
+ if (Guardrails.tables.enabled(state))
+ {
+ int totalUserTables = Schema.instance.getUserKeyspaces()
+ .stream()
+ .map(ksm -> ksm.name)
+ .map(Keyspace::open)
+ .mapToInt(keyspace ->
keyspace.getColumnFamilyStores().size())
+ .sum();
+ Guardrails.tables.guard(totalUserTables + 1, tableName, false,
state);
}
+
+ // Guardrail to check whether creation of new COMPACT STORAGE tables
is allowed
+ if (useCompactStorage)
+ Guardrails.compactTablesEnabled.ensureEnabled(state);
}
SchemaChange schemaChangeEvent(KeyspacesDiff diff)
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
index d49131d78e..6609760109 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
@@ -24,6 +24,7 @@ import javax.annotation.Nullable;
import com.google.common.annotations.VisibleForTesting;
import org.slf4j.LoggerFactory;
+import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.service.ClientWarn;
@@ -55,9 +56,8 @@ public abstract class Guardrail
}
/**
- * Checks whether this guardrail is enabled or not. This will be enabled
if guardrails are enabled
- * ({@link Guardrails#enabled(ClientState)}) and if the authenticated user
(if specified) is not system nor
- * superuser.
+ * Checks whether this guardrail is enabled or not. This will be enabled
if the database is initialized and the
+ * authenticated user (if specified) is not system nor superuser.
*
* @param state the client state, used to skip the check if the query is
internal or is done by a superuser.
* A {@code null} value means that the check should be done
regardless of the query.
@@ -65,7 +65,7 @@ public abstract class Guardrail
*/
public boolean enabled(@Nullable ClientState state)
{
- return Guardrails.enabled(state) && (state == null ||
state.isOrdinaryUser());
+ return DatabaseDescriptor.isDaemonInitialized() && (state == null ||
state.isOrdinaryUser());
}
protected void warn(String message)
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
index 88fc0f0c1a..baac800a0e 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
@@ -31,7 +31,6 @@ import org.apache.cassandra.config.DataStorageSpec;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.config.GuardrailsOptions;
import org.apache.cassandra.db.ConsistencyLevel;
-import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.utils.MBeanWrapper;
import static java.lang.String.format;
@@ -268,29 +267,6 @@ public final class Guardrails implements GuardrailsMBean
MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
}
- /**
- * Whether guardrails are enabled.
- *
- * @return {@code true} if guardrails are enabled and daemon is
initialized,
- * {@code false} otherwise (in which case no guardrail will trigger).
- */
- public static boolean enabled(ClientState state)
- {
- return DatabaseDescriptor.isDaemonInitialized() &&
CONFIG_PROVIDER.getOrCreate(state).getEnabled();
- }
-
- @Override
- public boolean getEnabled()
- {
- return DEFAULT_CONFIG.getEnabled();
- }
-
- @Override
- public void setEnabled(boolean enabled)
- {
- DEFAULT_CONFIG.setEnabled(enabled);
- }
-
@Override
public int getKeyspacesWarnThreshold()
{
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
index f95989fcf7..7ad9d0afb1 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
@@ -30,9 +30,6 @@ import org.apache.cassandra.db.ConsistencyLevel;
* checking each guarded constraint (which, again, should use the higher level
abstractions defined in
* {@link Guardrails}).
*
- * <p>This contains a main setting, {@code enabled}, controlling if guardrails
are globally active or not, and
- * individual settings to control each guardrail.
- *
* <p>We have 2 variants of guardrails, soft (warn) and hard (fail) limits,
each guardrail having either one of the
* variants or both. Note in particular that hard limits only make sense for
guardrails triggering during query
* execution. For other guardrails, say one triggering during compaction,
aborting that compaction does not make sense.
@@ -46,13 +43,6 @@ import org.apache.cassandra.db.ConsistencyLevel;
*/
public interface GuardrailsConfig
{
- /**
- * Whether guardrails are enabled or not.
- *
- * @return {@code true} if guardrails are enabled, {@code false} otherwise
- */
- boolean getEnabled();
-
/**
* @return The threshold to warn when creating more user keyspaces than
threshold.
*/
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
index ca4ea98e76..2cc1e03dbe 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
@@ -34,20 +34,6 @@ import java.util.Set;
*/
public interface GuardrailsMBean
{
- /**
- * Whether guardrails are enabled or not.
- *
- * @return {@code true} if guardrails are enabled, {@code false} otherwise
- */
- boolean getEnabled();
-
- /**
- * Enable/disable guardrails.
- *
- * @param enabled {@code true} for enabling guardrails, {@code false} for
disabling them.
- */
- void setEnabled(boolean enabled);
-
/**
* @return The threshold to warn when creating more user keyspaces than
threshold.
* -1 means disabled.
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailCollectionSizeOnSSTableWriteTest.java
b/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailCollectionSizeOnSSTableWriteTest.java
index e469bc3d56..6b7daef26f 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailCollectionSizeOnSSTableWriteTest.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailCollectionSizeOnSSTableWriteTest.java
@@ -54,7 +54,6 @@ public class GuardrailCollectionSizeOnSSTableWriteTest
extends GuardrailTester
{
cluster = init(Cluster.build(NUM_NODES)
.withConfig(c -> c.with(Feature.GOSSIP,
Feature.NATIVE_PROTOCOL)
- .set("guardrails_enabled",
true)
.set("collection_size_warn_threshold", WARN_THRESHOLD + "B")
.set("collection_size_fail_threshold", FAIL_THRESHOLD + "B"))
.start());
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailItemsPerCollectionOnSSTableWriteTest.java
b/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailItemsPerCollectionOnSSTableWriteTest.java
index 742fa6270d..260752e9d9 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailItemsPerCollectionOnSSTableWriteTest.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailItemsPerCollectionOnSSTableWriteTest.java
@@ -49,8 +49,7 @@ public class GuardrailItemsPerCollectionOnSSTableWriteTest
extends GuardrailTest
public static void setupCluster() throws IOException
{
cluster = init(Cluster.build(NUM_NODES)
- .withConfig(c -> c.set("guardrails_enabled",
true)
-
.set("items_per_collection_warn_threshold", WARN_THRESHOLD)
+ .withConfig(c ->
c.set("items_per_collection_warn_threshold", WARN_THRESHOLD)
.set("items_per_collection_fail_threshold", FAIL_THRESHOLD))
.start());
cluster.disableAutoCompaction(KEYSPACE);
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
index 4540a51bce..74747bbe5a 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
@@ -103,7 +103,6 @@ public abstract class GuardrailTester extends CQLTester
CQLTester.setUpClass();
requireAuthentication();
requireNetwork();
- guardrails().setEnabled(true);
DatabaseDescriptor.setDiagnosticEventsEnabled(true);
systemClientState = ClientState.forInternalCalls();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
