From 01c4404fff8c3dde071d4b9d0327acbb299be688 Mon Sep 17 00:00:00 2001 From: Josh McKenzie Date: Fri, 1 Apr 2022 14:34:03 -0400 Subject: [PATCH] Add guardrail to disallow creation of new COMPACT STORAGE tables Patch by Josh McKenzie; reviewed by Caleb Rackliffe for CASSANDRA-17522 --- CHANGES.txt | 1 + .../org/apache/cassandra/config/Config.java | 1 + .../cassandra/config/GuardrailsOptions.java | 14 ++++++ .../schema/CreateTableStatement.java | 4 ++ .../cassandra/db/guardrails/Guardrails.java | 20 +++++++++ .../db/guardrails/GuardrailsConfig.java | 7 +++ .../db/guardrails/GuardrailsMBean.java | 14 ++++++ .../GuardrailNewCompactStorageTest.java | 44 +++++++++++++++++++ 8 files changed, 105 insertions(+) create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailNewCompactStorageTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 5cf8e9b92b..dc3c66e557 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add guardrail to disallow creation of new COMPACT STORAGE tables (CASSANDRA-17522) * repair vtables should expose a completed field due to lack of filtering options in CQL (CASSANDRA-17520) * remove outdated code from cqlsh (CASSANDRA-17490) * remove support for deprecated version specific TLS in Python 3.6 (CASSANDRA-17365) diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 8b740320bf..9213185ece 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -800,6 +800,7 @@ public class Config public volatile Set write_consistency_levels_warned = Collections.emptySet(); public volatile Set write_consistency_levels_disallowed = Collections.emptySet(); public volatile boolean user_timestamps_enabled = true; + public volatile boolean compact_tables_enabled = true; public volatile boolean read_before_write_list_operations_enabled = true; public volatile DataStorageSpec collection_size_warn_threshold = DISABLED_SIZE_GUARDRAIL; public volatile DataStorageSpec collection_size_fail_threshold = DISABLED_SIZE_GUARDRAIL; diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 4a6ff6c845..cd83181a91 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -332,6 +332,20 @@ public class GuardrailsOptions implements GuardrailsConfig x -> config.user_timestamps_enabled = x); } + @Override + public boolean getCompactTablesEnabled() + { + return config.compact_tables_enabled; + } + + public void setCompactTablesEnabled(boolean enabled) + { + updatePropertyWithLogging("compact_tables_enabled", + enabled, + () -> config.compact_tables_enabled, + x -> config.compact_tables_enabled = x); + } + @Override public boolean getReadBeforeWriteListOperationsEnabled() { 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 6d65c5ad36..043a7af0f2 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -146,6 +146,10 @@ public final class CreateTableStatement extends AlterSchemaStatement .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); } } diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 6a07fddf6f..110bad5303 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -129,6 +129,14 @@ public final class Guardrails implements GuardrailsMBean state -> !CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(), "User provided timestamps (USING TIMESTAMP)"); + /** + * Guardrail disabling the creation of new COMPACT STORAGE tables + */ + public static final DisableFlag compactTablesEnabled = + new DisableFlag("compact_tables", + state -> !CONFIG_PROVIDER.getOrCreate(state).getCompactTablesEnabled(), + "Creation of new COMPACT STORAGE tables"); + /** * Guardrail on the number of elements returned within page. */ @@ -456,6 +464,18 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setUserTimestampsEnabled(enabled); } + @Override + public boolean getCompactTablesEnabled() + { + return DEFAULT_CONFIG.getCompactTablesEnabled(); + } + + @Override + public void setCompactTablesEnabled(boolean enabled) + { + DEFAULT_CONFIG.setCompactTablesEnabled(enabled); + } + @Override public int getPageSizeWarnThreshold() { diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 1e1413164c..8147c4e1bb 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -136,6 +136,13 @@ public interface GuardrailsConfig */ boolean getUserTimestampsEnabled(); + /** + * Returns whether users can create new COMPACT STORAGE tables + * + * @return {@code true} if allowed, {@code false} otherwise. + */ + boolean getCompactTablesEnabled(); + /** * @return The threshold to warn when page size exceeds given size. */ diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 67c118b863..2dd78027d1 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -211,6 +211,20 @@ public interface GuardrailsMBean */ void setUserTimestampsEnabled(boolean enabled); + /** + * Returns whether users can create new COMPACT STORAGE tables + * + * @return {@code true} if allowed, {@code false} otherwise. + */ + boolean getCompactTablesEnabled(); + + /** + * Sets whether users can create new COMPACT STORAGE tables + * + * @param enabled {@code true} if allowed, {@code false} otherwise. + */ + void setCompactTablesEnabled(boolean enabled); + /** * @return The threshold to warn when requested page size greater than threshold. * -1 means disabled. diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailNewCompactStorageTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailNewCompactStorageTest.java new file mode 100644 index 0000000000..ce786ed6d3 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailNewCompactStorageTest.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.guardrails; + +import org.junit.Test; + +public class GuardrailNewCompactStorageTest extends GuardrailTester +{ + private void setGuardrail(boolean enabled) + { + Guardrails.instance.setCompactTablesEnabled(enabled); + } + + @Test + public void testFeatureEnabled() throws Throwable + { + setGuardrail(true); + assertValid("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, v int, PRIMARY KEY (pk, ck)) WITH COMPACT STORAGE"); + } + + @Test + public void testFeatureDisabled() throws Throwable + { + setGuardrail(false); + assertFails("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, v int, PRIMARY KEY (pk, ck)) WITH COMPACT STORAGE", + "Creation of new COMPACT STORAGE tables"); + } +}