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 0ecd480 Improve guardrails messages
0ecd480 is described below
commit 0ecd48001fab88d2d53c717ef47cd3bc0d1369d0
Author: Andrés de la Peña <[email protected]>
AuthorDate: Fri Mar 11 17:49:17 2022 +0000
Improve guardrails messages
Guardrail messages include a specific mention of the guardrail framework
and the name of the violated guardrail,
so users know what is producing the wanrning or failure. Also, guardrails
throw their own type of exception.
patch by Andrés de la Peña; reviewed by Josh McKenzie for CASSANDRA-17430
---
CHANGES.txt | 1 +
.../cassandra/db/guardrails/DisableFlag.java | 4 ++-
.../apache/cassandra/db/guardrails/Guardrail.java | 22 +++++++++++-
.../db/guardrails/GuardrailViolatedException.java | 29 +++++++++++++++
.../apache/cassandra/db/guardrails/Guardrails.java | 39 +++++++++++++-------
.../apache/cassandra/db/guardrails/Threshold.java | 5 ++-
.../org/apache/cassandra/db/guardrails/Values.java | 5 ++-
.../guardrails/GuardrailColumnsPerTableTest.java | 2 +-
.../GuardrailConsistencyLevelsTester.java | 2 ++
.../GuardrailInSelectCartesianProductTest.java | 2 +-
.../db/guardrails/GuardrailKeyspacesTest.java | 2 +-
.../db/guardrails/GuardrailPageSizeTest.java | 2 +-
.../GuardrailPartitionKeysInSelectTest.java | 2 +-
...GuardrailReadBeforeWriteListOperationsTest.java | 5 +++
.../GuardrailReadConsistencyLevelsTest.java | 1 +
.../GuardrailSecondaryIndexesPerTable.java | 2 +-
.../db/guardrails/GuardrailTablesTest.java | 2 +-
.../cassandra/db/guardrails/GuardrailTester.java | 41 +++++++++++++++++++---
.../db/guardrails/GuardrailUserTimestampsTest.java | 5 +++
.../db/guardrails/GuardrailViewsPerTableTest.java | 10 +++---
.../GuardrailWriteConsistencyLevelsTest.java | 1 +
.../guardrails/GuardrailsConfigProviderTest.java | 3 +-
.../cassandra/db/guardrails/GuardrailsTest.java | 38 ++++++++++++--------
.../cassandra/db/guardrails/ThresholdTester.java | 15 ++++----
24 files changed, 182 insertions(+), 58 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index e99d8d8..0726f04 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.1
+ * Improve guardrails messages (CASSANDRA-17430)
* Remove all usages of junit.framework and ban them via Checkstyle
(CASSANDRA-17316)
* Add guardrails for read/write consistency levels (CASSANDRA-17188)
* Add guardrail for SELECT IN terms and their cartesian product
(CASSANDRA-17187)
diff --git a/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java
b/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java
index e8a3a2c..b6a8ed9 100644
--- a/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java
+++ b/src/java/org/apache/cassandra/db/guardrails/DisableFlag.java
@@ -38,13 +38,15 @@ public class DisableFlag extends Guardrail
/**
* Creates a new {@link DisableFlag} guardrail.
*
+ * @param name the identifying name of the guardrail
* @param disabled a {@link ClientState}-based supplier of boolean
indicating whether the feature guarded by this
* guardrail must be disabled.
* @param what The feature that is guarded by this guardrail (for
reporting in error messages),
* {@link DisableFlag#ensureEnabled(String, ClientState)}
can specify a different {@code what}.
*/
- public DisableFlag(Predicate<ClientState> disabled, String what)
+ public DisableFlag(String name, Predicate<ClientState> disabled, String
what)
{
+ super(name);
this.disabled = disabled;
this.what = what;
}
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
index 74496e3..33dd8af 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrail.java
@@ -21,6 +21,7 @@ package org.apache.cassandra.db.guardrails;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
+import com.google.common.annotations.VisibleForTesting;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.exceptions.InvalidRequestException;
@@ -44,6 +45,14 @@ public abstract class Guardrail
protected static final NoSpamLogger logger =
NoSpamLogger.getLogger(LoggerFactory.getLogger(Guardrail.class),
10,
TimeUnit.MINUTES);
+ /** A name identifying the guardrail (mainly for shipping with diagnostic
events). */
+ public final String name;
+
+ Guardrail(String name)
+ {
+ this.name = name;
+ }
+
/**
* 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
@@ -60,6 +69,8 @@ public abstract class Guardrail
protected void warn(String message)
{
+ message = decorateMessage(message);
+
logger.warn(message);
// Note that ClientWarn will simply ignore the message if we're not
running this as part of a user query
// (the internal "state" will be null)
@@ -70,6 +81,8 @@ public abstract class Guardrail
protected void fail(String message)
{
+ message = decorateMessage(message);
+
logger.error(message);
// Note that ClientWarn will simply ignore the message if we're not
running this as part of a user query
// (the internal "state" will be null)
@@ -77,6 +90,13 @@ public abstract class Guardrail
// Similarly, tracing will also ignore the message if we're not
running tracing on the current thread.
Tracing.trace(message);
- throw new InvalidRequestException(message);
+ throw new GuardrailViolatedException(message);
+ }
+
+ @VisibleForTesting
+ String decorateMessage(String message)
+ {
+ // Add a prefix to error message so user knows what threw the warning
or cause the failure
+ return String.format("Guardrail %s violated: %s", name, message);
}
}
diff --git
a/src/java/org/apache/cassandra/db/guardrails/GuardrailViolatedException.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailViolatedException.java
new file mode 100644
index 0000000..e8a4795
--- /dev/null
+++
b/src/java/org/apache/cassandra/db/guardrails/GuardrailViolatedException.java
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.db.guardrails;
+
+import org.apache.cassandra.exceptions.InvalidRequestException;
+
+public class GuardrailViolatedException extends InvalidRequestException
+{
+ GuardrailViolatedException(String message)
+ {
+ super(message);
+ }
+}
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
index b100df5..5adab5a 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
@@ -52,7 +52,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the total number of user keyspaces.
*/
public static final Threshold keyspaces =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getKeyspacesWarnThreshold(),
+ 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.",
@@ -64,7 +65,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the total number of tables on user keyspaces.
*/
public static final Threshold tables =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getTablesWarnThreshold(),
+ 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.",
@@ -76,7 +78,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of columns per table.
*/
public static final Threshold columnsPerTable =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getColumnsPerTableWarnThreshold(),
+ 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.",
@@ -85,7 +88,8 @@ public final class Guardrails implements GuardrailsMBean
threshold, value, what));
public static final Threshold secondaryIndexesPerTable =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getSecondaryIndexesPerTableWarnThreshold(),
+ 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.",
@@ -97,7 +101,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of materialized views per table.
*/
public static final Threshold materializedViewsPerTable =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getMaterializedViewsPerTableWarnThreshold(),
+ 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.",
@@ -109,7 +114,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail warning about, ignoring or rejecting the usage of certain
table properties.
*/
public static final Values<String> tableProperties =
- new Values<>(state ->
CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesWarned(),
+ new Values<>("table_properties",
+ state ->
CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesWarned(),
state ->
CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesIgnored(),
state ->
CONFIG_PROVIDER.getOrCreate(state).getTablePropertiesDisallowed(),
"Table Properties");
@@ -118,14 +124,16 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail disabling user-provided timestamps.
*/
public static final DisableFlag userTimestampsEnabled =
- new DisableFlag(state ->
!CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(),
+ new DisableFlag("user_timestamps",
+ state ->
!CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(),
"User provided timestamps (USING TIMESTAMP)");
/**
* Guardrail on the number of elements returned within page.
*/
public static final Threshold pageSize =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getPageSizeWarnThreshold(),
+ 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.",
@@ -137,7 +145,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on the number of partition keys in the IN clause.
*/
public static final Threshold partitionKeysInSelect =
- new Threshold(state ->
CONFIG_PROVIDER.getOrCreate(state).getPartitionKeysInSelectWarnThreshold(),
+ 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 " +
@@ -151,14 +160,16 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail disabling operations on lists that require read before write.
*/
public static final DisableFlag readBeforeWriteListOperationsEnabled =
- new DisableFlag(state ->
!CONFIG_PROVIDER.getOrCreate(state).getReadBeforeWriteListOperationsEnabled(),
+ new DisableFlag("read_before_write_list_operations",
+ state ->
!CONFIG_PROVIDER.getOrCreate(state).getReadBeforeWriteListOperationsEnabled(),
"List operation requiring read before write");
/**
* 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(state ->
CONFIG_PROVIDER.getOrCreate(state).getInSelectCartesianProductWarnThreshold(),
+ 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 %d values, " +
@@ -171,7 +182,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on read consistency levels.
*/
public static final Values<ConsistencyLevel> readConsistencyLevels =
- new Values<>(state ->
CONFIG_PROVIDER.getOrCreate(state).getReadConsistencyLevelsWarned(),
+ new Values<>("read_consistency_levels",
+ state ->
CONFIG_PROVIDER.getOrCreate(state).getReadConsistencyLevelsWarned(),
state -> Collections.emptySet(),
state ->
CONFIG_PROVIDER.getOrCreate(state).getReadConsistencyLevelsDisallowed(),
"read consistency levels");
@@ -180,7 +192,8 @@ public final class Guardrails implements GuardrailsMBean
* Guardrail on write consistency levels.
*/
public static final Values<ConsistencyLevel> writeConsistencyLevels =
- new Values<>(state ->
CONFIG_PROVIDER.getOrCreate(state).getWriteConsistencyLevelsWarned(),
+ new Values<>("write_consistency_levels",
+ state ->
CONFIG_PROVIDER.getOrCreate(state).getWriteConsistencyLevelsWarned(),
state -> Collections.emptySet(),
state ->
CONFIG_PROVIDER.getOrCreate(state).getWriteConsistencyLevelsDisallowed(),
"write consistency levels");
diff --git a/src/java/org/apache/cassandra/db/guardrails/Threshold.java
b/src/java/org/apache/cassandra/db/guardrails/Threshold.java
index ccf7ec5..85fddce 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Threshold.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Threshold.java
@@ -40,14 +40,17 @@ public class Threshold extends Guardrail
/**
* 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 Threshold(ToLongFunction<ClientState> warnThreshold,
+ public Threshold(String name,
+ ToLongFunction<ClientState> warnThreshold,
ToLongFunction<ClientState> failThreshold,
ErrorMessageProvider messageProvider)
{
+ super(name);
this.warnThreshold = warnThreshold;
this.failThreshold = failThreshold;
this.messageProvider = messageProvider;
diff --git a/src/java/org/apache/cassandra/db/guardrails/Values.java
b/src/java/org/apache/cassandra/db/guardrails/Values.java
index 36bb171..edb5d2a 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Values.java
+++ b/src/java/org/apache/cassandra/db/guardrails/Values.java
@@ -46,16 +46,19 @@ public class Values<T> extends Guardrail
/**
* Creates a new values guardrail.
*
+ * @param name the identifying name of the guardrail
* @param warnedValues a {@link ClientState}-based provider of the
values for which a warning is triggered.
* @param ignoredValues a {@link ClientState}-based provider of the
values that are ignored.
* @param disallowedValues a {@link ClientState}-based provider of the
values that are disallowed.
* @param what The feature that is guarded by this guardrail
(for reporting in error messages).
*/
- public Values(Function<ClientState, Set<T>> warnedValues,
+ public Values(String name,
+ Function<ClientState, Set<T>> warnedValues,
Function<ClientState, Set<T>> ignoredValues,
Function<ClientState, Set<T>> disallowedValues,
String what)
{
+ super(name);
this.warnedValues = warnedValues;
this.ignoredValues = ignoredValues;
this.disallowedValues = disallowedValues;
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java
index 0c32f49..ca0c41d 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailColumnsPerTableTest.java
@@ -34,7 +34,7 @@ public class GuardrailColumnsPerTableTest extends
ThresholdTester
{
super(COLUMNS_PER_TABLE_WARN_THRESHOLD,
COLUMNS_PER_TABLE_FAIL_THRESHOLD,
- "columns_per_table",
+ Guardrails.columnsPerTable,
Guardrails::setColumnsPerTableThreshold,
Guardrails::getColumnsPerTableWarnThreshold,
Guardrails::getColumnsPerTableFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
index c81f49d..5ca6d8a 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailConsistencyLevelsTester.java
@@ -49,6 +49,7 @@ public abstract class GuardrailConsistencyLevelsTester
extends GuardrailTester
public GuardrailConsistencyLevelsTester(String warnedPropertyName,
String disallowePropertyName,
+ Values<ConsistencyLevel> guardrail,
Function<Guardrails,
Set<ConsistencyLevel>> warnedGetter,
Function<Guardrails,
Set<ConsistencyLevel>> disallowedGetter,
Function<Guardrails, String>
warnedCSVGetter,
@@ -58,6 +59,7 @@ public abstract class GuardrailConsistencyLevelsTester
extends GuardrailTester
BiConsumer<Guardrails, String>
warnedCSVSetter,
BiConsumer<Guardrails, String>
disallowedCSVSetter)
{
+ super(guardrail);
this.warnedPropertyName = warnedPropertyName;
this.disallowePropertyName = disallowePropertyName;
this.warnedGetter = warnedGetter;
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java
index 4682904..6575084 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailInSelectCartesianProductTest.java
@@ -46,7 +46,7 @@ public class GuardrailInSelectCartesianProductTest extends
ThresholdTester
{
super(WARN_THRESHOLD,
FAIL_THRESHOLD,
- "in_select_cartesian_product",
+ Guardrails.inSelectCartesianProduct,
Guardrails::setInSelectCartesianProductThreshold,
Guardrails::getInSelectCartesianProductWarnThreshold,
Guardrails::getInSelectCartesianProductFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
index 2ccbad1..2c5b3d5 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailKeyspacesTest.java
@@ -36,7 +36,7 @@ public class GuardrailKeyspacesTest extends ThresholdTester
{
super(WARN_THRESHOLD,
FAIL_THRESHOLD,
- "keyspaces",
+ Guardrails.keyspaces,
Guardrails::setKeyspacesThreshold,
Guardrails::getKeyspacesWarnThreshold,
Guardrails::getKeyspacesFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java
index ba53a17..ba4c2d0 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPageSizeTest.java
@@ -45,7 +45,7 @@ public class GuardrailPageSizeTest extends ThresholdTester
{
super(PAGE_SIZE_WARN_THRESHOLD,
PAGE_SIZE_FAIL_THRESHOLD,
- "page_size",
+ Guardrails.pageSize,
Guardrails::setPageSizeThreshold,
Guardrails::getPageSizeWarnThreshold,
Guardrails::getPageSizeFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java
index af0728a..9acef56 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailPartitionKeysInSelectTest.java
@@ -31,7 +31,7 @@ public class GuardrailPartitionKeysInSelectTest extends
ThresholdTester
{
super(PARTITION_KEYS_SELECT_WARN_THRESHOLD,
PARTITION_KEYS_SELECT_FAIL_THRESHOLD,
- "partition_keys_in_select",
+ Guardrails.partitionKeysInSelect,
Guardrails::setPartitionKeysInSelectThreshold,
Guardrails::getPartitionKeysInSelectWarnThreshold,
Guardrails::getPartitionKeysInSelectFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java
index 98a7d09..8184fa6 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadBeforeWriteListOperationsTest.java
@@ -42,6 +42,11 @@ public class GuardrailReadBeforeWriteListOperationsTest
extends GuardrailTester
return Arrays.asList(false, true);
}
+ public GuardrailReadBeforeWriteListOperationsTest()
+ {
+ super(Guardrails.readBeforeWriteListOperationsEnabled);
+ }
+
@Before
public void before()
{
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadConsistencyLevelsTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadConsistencyLevelsTest.java
index dccd306..2d189a2 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadConsistencyLevelsTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailReadConsistencyLevelsTest.java
@@ -39,6 +39,7 @@ public class GuardrailReadConsistencyLevelsTest extends
GuardrailConsistencyLeve
{
super("read_consistency_levels_warned",
"read_consistency_levels_disallowed",
+ Guardrails.readConsistencyLevels,
Guardrails::getReadConsistencyLevelsWarned,
Guardrails::getReadConsistencyLevelsDisallowed,
Guardrails::getReadConsistencyLevelsWarnedCSV,
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
index c7a3763..102da71 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailSecondaryIndexesPerTable.java
@@ -35,7 +35,7 @@ public class GuardrailSecondaryIndexesPerTable extends
ThresholdTester
{
super(INDEXES_PER_TABLE_WARN_THRESHOLD,
INDEXES_PER_TABLE_FAIL_THRESHOLD,
- "secondary_indexes_per_table",
+ Guardrails.secondaryIndexesPerTable,
Guardrails::setSecondaryIndexesPerTableThreshold,
Guardrails::getSecondaryIndexesPerTableWarnThreshold,
Guardrails::getSecondaryIndexesPerTableFailThreshold);
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
index c56ae00..969b50c 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTablesTest.java
@@ -36,7 +36,7 @@ public class GuardrailTablesTest extends ThresholdTester
{
super(TABLES_LIMIT_WARN_THRESHOLD,
TABLES_LIMIT_FAIL_THRESHOLD,
- "tables",
+ Guardrails.tables,
Guardrails::setTablesThreshold,
Guardrails::getTablesWarnThreshold,
Guardrails::getTablesFailThreshold);
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
index 797476d..2592f85 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java
@@ -28,13 +28,13 @@ import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import com.google.common.collect.ImmutableSet;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
-import com.datastax.driver.core.exceptions.InvalidQueryException;
import org.apache.cassandra.auth.AuthenticatedUser;
import org.apache.cassandra.auth.CassandraRoleManager;
import org.apache.cassandra.cql3.CQLStatement;
@@ -43,7 +43,6 @@ import org.apache.cassandra.cql3.QueryOptions;
import org.apache.cassandra.cql3.QueryProcessor;
import org.apache.cassandra.db.ConsistencyLevel;
import org.apache.cassandra.db.view.View;
-import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.index.sasi.SASIIndex;
import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.service.ClientWarn;
@@ -71,6 +70,22 @@ public abstract class GuardrailTester extends CQLTester
protected static ClientState systemClientState, userClientState,
superClientState;
+ /**
+ * The tested guardrail, if we are testing a specific one.
+ */
+ @Nullable
+ protected final Guardrail guardrail;
+
+ public GuardrailTester()
+ {
+ this(null);
+ }
+
+ public GuardrailTester(@Nullable Guardrail guardrail)
+ {
+ this.guardrail = guardrail;
+ }
+
@BeforeClass
public static void setUpClass()
{
@@ -162,7 +177,7 @@ public abstract class GuardrailTester extends CQLTester
function.apply();
assertEmptyWarnings();
}
- catch (InvalidRequestException e)
+ catch (GuardrailViolatedException e)
{
fail("Expected not to fail, but failed with error message: " +
e.getMessage());
}
@@ -213,12 +228,20 @@ public abstract class GuardrailTester extends CQLTester
if (thrown)
fail("Expected to fail, but it did not");
}
- catch (InvalidRequestException | InvalidQueryException e)
+ catch (GuardrailViolatedException e)
{
assertTrue("Expect no exception thrown", thrown);
// the last message is the one raising the guardrail failure, the
previous messages are warnings
String failMessage = messages[messages.length - 1];
+
+ if (guardrail != null)
+ {
+ String prefix = guardrail.decorateMessage("");
+ assertTrue(format("Full error message '%s' doesn't start with
the prefix '%s'", e.getMessage(), prefix),
+ e.getMessage().startsWith(prefix));
+ }
+
assertTrue(format("Full error message '%s' does not contain
expected message '%s'", e.getMessage(), failMessage),
e.getMessage().contains(failMessage));
@@ -263,8 +286,16 @@ public abstract class GuardrailTester extends CQLTester
for (int i = 0; i < messages.length; i++)
{
- String message = messages[i];
String warning = warnings.get(i);
+
+ String message = messages[i];
+ if (guardrail != null)
+ {
+ String prefix = guardrail.decorateMessage("");
+ assertTrue(format("Warning log message '%s' doesn't start with
the prefix '%s'", warning, prefix),
+ warning.startsWith(prefix));
+ }
+
assertTrue(format("Warning log message '%s' does not contain
expected message '%s'", warning, message),
warning.contains(message));
}
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java
index 72d8894..d7c0c7b 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailUserTimestampsTest.java
@@ -26,6 +26,11 @@ import org.junit.Test;
*/
public class GuardrailUserTimestampsTest extends GuardrailTester
{
+ public GuardrailUserTimestampsTest()
+ {
+ super(Guardrails.userTimestampsEnabled);
+ }
+
@Before
public void setupTest()
{
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java
index 3cc3ab9..edff317 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailViewsPerTableTest.java
@@ -24,7 +24,7 @@ import org.junit.Test;
import static java.lang.String.format;
/**
- * Tests the guardrail for the number of secondary indexes in a table, {@link
Guardrails#secondaryIndexesPerTable}.
+ * Tests the guardrail for the number of materialized views in a table, {@link
Guardrails#materializedViewsPerTable}.
*/
public class GuardrailViewsPerTableTest extends ThresholdTester
{
@@ -39,7 +39,7 @@ public class GuardrailViewsPerTableTest extends
ThresholdTester
{
super(VIEWS_PER_TABLE_WARN_THRESHOLD,
VIEWS_PER_TABLE_FAIL_THRESHOLD,
- "materialized_views_per_table",
+ Guardrails.materializedViewsPerTable,
Guardrails::setMaterializedViewsPerTableThreshold,
Guardrails::getMaterializedViewsPerTableWarnThreshold,
Guardrails::getMaterializedViewsPerTableFailThreshold);
@@ -110,8 +110,7 @@ public class GuardrailViewsPerTableTest extends
ThresholdTester
String viewName = createViewName();
assertThresholdWarns(format(CREATE_VIEW, viewName),
format("Creating materialized view %s on table
%s, current number of views %s exceeds warning threshold of %s.",
- viewName, currentTable(), currentValue() +
1, guardrails().getMaterializedViewsPerTableWarnThreshold())
- );
+ viewName, currentTable(), currentValue() +
1, guardrails().getMaterializedViewsPerTableWarnThreshold()));
}
private void assertCreateViewFails() throws Throwable
@@ -119,7 +118,6 @@ public class GuardrailViewsPerTableTest extends
ThresholdTester
String viewName = createViewName();
assertThresholdFails(format(CREATE_VIEW, viewName),
format("aborting the creation of materialized
view %s on table %s",
- viewName, currentTable())
- );
+ viewName, currentTable()));
}
}
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailWriteConsistencyLevelsTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailWriteConsistencyLevelsTest.java
index 3c38a26..f2ad40f 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailWriteConsistencyLevelsTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailWriteConsistencyLevelsTest.java
@@ -42,6 +42,7 @@ public class GuardrailWriteConsistencyLevelsTest extends
GuardrailConsistencyLev
{
super("write_consistency_levels_warned",
"write_consistency_levels_disallowed",
+ Guardrails.writeConsistencyLevels,
Guardrails::getWriteConsistencyLevelsWarned,
Guardrails::getWriteConsistencyLevelsDisallowed,
Guardrails::getWriteConsistencyLevelsWarnedCSV,
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
index 35f8280..cf2e40f 100644
---
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
+++
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java
@@ -36,7 +36,8 @@ public class GuardrailsConfigProviderTest extends
GuardrailTester
{
String name = getClass().getCanonicalName() + '$' +
CustomProvider.class.getSimpleName();
GuardrailsConfigProvider provider =
GuardrailsConfigProvider.build(name);
- Threshold guard = new Threshold(state ->
provider.getOrCreate(state).getTablesWarnThreshold(),
+ Threshold guard = new Threshold("test_guardrail",
+ state ->
provider.getOrCreate(state).getTablesWarnThreshold(),
state ->
provider.getOrCreate(state).getTablesFailThreshold(),
(isWarn, what, v, t) -> format("%s:
for %s, %s > %s",
isWarn
? "Warning" : "Aborting", what, v, t));
diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
index 6cebccc..c108560 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsTest.java
@@ -43,7 +43,7 @@ public class GuardrailsTest extends GuardrailTester
public void testDisabledThreshold() throws Throwable
{
Threshold.ErrorMessageProvider errorMessageProvider = (isWarn, what,
v, t) -> "Should never trigger";
- testDisabledThreshold(new Threshold(state -> DISABLED, state ->
DISABLED, errorMessageProvider));
+ testDisabledThreshold(new Threshold("x", state -> DISABLED, state ->
DISABLED, errorMessageProvider));
}
private void testDisabledThreshold(Threshold guard) throws Throwable
@@ -60,7 +60,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> 10,
+ Threshold guard = new Threshold("x",
+ state -> 10,
state -> 100,
(isWarn, what, v, t) -> format("%s:
for %s, %s > %s",
isWarn
? "Warning" : "Aborting", what, v, t));
@@ -78,7 +79,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testWarnOnlyThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> 10,
+ Threshold guard = new Threshold("x",
+ state -> 10,
state -> DISABLED,
(isWarn, what, v, t) -> format("%s:
for %s, %s > %s",
isWarn
? "Warning" : "Aborting", what, v, t));
@@ -92,7 +94,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testFailOnlyThreshold() throws Throwable
{
- Threshold guard = new Threshold(state -> DISABLED,
+ Threshold guard = new Threshold("x",
+ state -> DISABLED,
state -> 10,
(isWarn, what, v, t) -> format("%s:
for %s, %s > %s",
isWarn
? "Warning" : "Aborting", what, v, t));
@@ -106,7 +109,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testThresholdUsers() throws Throwable
{
- Threshold guard = new Threshold(state -> 10,
+ Threshold guard = new Threshold("x",
+ state -> 10,
state -> 100,
(isWarn, what, v, t) -> format("%s:
for %s, %s > %s",
isWarn
? "Warning" : "Aborting", what, v, t));
@@ -133,23 +137,23 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testDisableFlag() throws Throwable
{
- assertFails(() -> new DisableFlag(state -> true,
"X").ensureEnabled(userClientState), "X is not allowed");
- assertValid(() -> new DisableFlag(state -> false,
"X").ensureEnabled(userClientState));
+ assertFails(() -> new DisableFlag("x", state -> true,
"X").ensureEnabled(userClientState), "X is not allowed");
+ assertValid(() -> new DisableFlag("x", state -> false,
"X").ensureEnabled(userClientState));
- assertFails(() -> new DisableFlag(state -> true,
"X").ensureEnabled("Y", userClientState), "Y is not allowed");
- assertValid(() -> new DisableFlag(state -> false,
"X").ensureEnabled("Y", userClientState));
+ assertFails(() -> new DisableFlag("x", state -> true,
"X").ensureEnabled("Y", userClientState), "Y is not allowed");
+ assertValid(() -> new DisableFlag("x", state -> false,
"X").ensureEnabled("Y", userClientState));
}
@Test
public void testDisableFlagUsers() throws Throwable
{
- DisableFlag enabled = new DisableFlag(state -> false, "X");
+ DisableFlag enabled = new DisableFlag("x", state -> false, "X");
assertValid(() -> enabled.ensureEnabled(null));
assertValid(() -> enabled.ensureEnabled(userClientState));
assertValid(() -> enabled.ensureEnabled(systemClientState));
assertValid(() -> enabled.ensureEnabled(superClientState));
- DisableFlag disabled = new DisableFlag(state -> true, "X");
+ DisableFlag disabled = new DisableFlag("x", state -> true, "X");
assertFails(() -> disabled.ensureEnabled(null), "X is not allowed");
assertFails(() -> disabled.ensureEnabled(userClientState), "X is not
allowed");
assertValid(() -> disabled.ensureEnabled(systemClientState));
@@ -160,7 +164,8 @@ public class GuardrailsTest extends GuardrailTester
public void testValuesWarned() throws Throwable
{
// Using a sorted set below to ensure the order in the warning message
checked below is not random
- Values<Integer> warned = new Values<>(state -> insertionOrderedSet(4,
6, 20),
+ Values<Integer> warned = new Values<>("x",
+ state -> insertionOrderedSet(4,
6, 20),
state -> Collections.emptySet(),
state -> Collections.emptySet(),
"integer");
@@ -179,7 +184,8 @@ public class GuardrailsTest extends GuardrailTester
public void testValuesIgnored() throws Throwable
{
// Using a sorted set below to ensure the order in the error message
checked below are not random
- Values<Integer> ignored = new Values<>(state -> Collections.emptySet(),
+ Values<Integer> ignored = new Values<>("x",
+ state -> Collections.emptySet(),
state -> insertionOrderedSet(4,
6, 20),
state -> Collections.emptySet(),
"integer");
@@ -212,7 +218,8 @@ public class GuardrailsTest extends GuardrailTester
public void testValuesDisallowed() throws Throwable
{
// Using a sorted set below to ensure the order in the error message
checked below are not random
- Values<Integer> disallowed = new Values<>(state ->
Collections.emptySet(),
+ Values<Integer> disallowed = new Values<>("x",
+ state ->
Collections.emptySet(),
state ->
Collections.emptySet(),
state ->
insertionOrderedSet(4, 6, 20),
"integer");
@@ -236,7 +243,8 @@ public class GuardrailsTest extends GuardrailTester
@Test
public void testValuesUsers() throws Throwable
{
- Values<Integer> disallowed = new Values<>(state ->
Collections.singleton(2),
+ Values<Integer> disallowed = new Values<>("x",
+ state ->
Collections.singleton(2),
state ->
Collections.singleton(3),
state ->
Collections.singleton(4),
"integer");
diff --git a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
index 0d2c9f4..a816ca9 100644
--- a/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
+++ b/test/unit/org/apache/cassandra/db/guardrails/ThresholdTester.java
@@ -30,6 +30,7 @@ import org.assertj.core.api.Assertions;
import static java.lang.String.format;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.fail;
/**
@@ -37,7 +38,6 @@ import static org.junit.Assert.fail;
*/
public abstract class ThresholdTester extends GuardrailTester
{
- private final String name;
private final long warnThreshold;
private final long failThreshold;
private final TriConsumer<Guardrails, Long, Long> setter;
@@ -47,12 +47,12 @@ public abstract class ThresholdTester extends
GuardrailTester
protected ThresholdTester(long warnThreshold,
long failThreshold,
- String name,
+ Threshold threshold,
TriConsumer<Guardrails, Long, Long> setter,
ToLongFunction<Guardrails> warnGetter,
ToLongFunction<Guardrails> failGetter)
{
- this.name = name;
+ super(threshold);
this.warnThreshold = warnThreshold;
this.failThreshold = failThreshold;
this.setter = setter;
@@ -62,12 +62,12 @@ public abstract class ThresholdTester extends
GuardrailTester
protected ThresholdTester(int warnThreshold,
int failThreshold,
- String name,
+ Threshold threshold,
TriConsumer<Guardrails, Integer, Integer> setter,
ToIntFunction<Guardrails> warnGetter,
ToIntFunction<Guardrails> failGetter)
{
- this.name = name;
+ super(threshold);
this.warnThreshold = warnThreshold;
this.failThreshold = failThreshold;
this.setter = (g, w, a) -> setter.accept(g, w.intValue(),
a.intValue());
@@ -91,7 +91,8 @@ public abstract class ThresholdTester extends GuardrailTester
@Test
public void testConfigValidation()
{
- testValidationOfThresholdProperties(name + "_warn_threshold", name +
"_fail_threshold");
+ assertNotNull(guardrail);
+ testValidationOfThresholdProperties(guardrail.name +
"_warn_threshold", guardrail.name + "_fail_threshold");
}
protected void testValidationOfThresholdProperties(String warnName, String
failName)
@@ -104,7 +105,7 @@ public abstract class ThresholdTester extends
GuardrailTester
setter.accept(guardrails(), -1L, -1L);
Assertions.assertThatThrownBy(() -> setter.accept(guardrails(), 2L,
1L))
.hasMessageContaining(format("The warn threshold 2 for %s
should be lower than the fail threshold 1",
- name + "_warn_threshold"));
+ guardrail.name +
"_warn_threshold"));
}
protected void assertThresholdValid(String query) throws Throwable
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
