From 105d69fdc531d0882f628cd4bf1e34288280c12b Mon Sep 17 00:00:00 2001 From: Josh McKenzie Date: Wed, 30 Mar 2022 14:19:07 -0400 Subject: [PATCH] Add guardrail to disallow creation of uncompressed sstables Patch by Josh McKenzie; reviewed by David Capwell for CASSANDRA-17504 --- CHANGES.txt | 1 + conf/cassandra.yaml | 2 + .../org/apache/cassandra/config/Config.java | 1 + .../cassandra/config/GuardrailsOptions.java | 14 ++++ .../schema/AlterSchemaStatement.java | 6 +- .../schema/AlterTableStatement.java | 7 +- .../schema/CreateTableStatement.java | 3 + .../cassandra/db/guardrails/Guardrails.java | 20 +++++ .../db/guardrails/GuardrailsConfig.java | 8 +- .../db/guardrails/GuardrailsMBean.java | 14 ++++ .../GuardrailAllowUncompressedTables.java | 77 +++++++++++++++++++ 11 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java diff --git a/CHANGES.txt b/CHANGES.txt index dc3c66e557..9ded13f525 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Add guardrail to disallow creation of uncompressed tables (CASSANDRA-17504) * 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) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index fcae5a5a56..df1eeb259b 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1609,6 +1609,8 @@ drop_compact_storage_enabled: false # The two thresholds default to -1 to disable. # tables_warn_threshold: -1 # tables_fail_threshold: -1 +# Guardrail to enable or disable the ability to create uncompressed tables +# uncompressed_tables_enabled: true # Guardrail to warn or fail when creating/altering a table with more columns per table than threshold. # The two thresholds default to -1 to disable. # columns_per_table_warn_threshold: -1 diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 9213185ece..00db7ea859 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 uncompressed_tables_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; diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index cd83181a91..405cca7ee3 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 getUncompressedTablesEnabled() + { + return config.uncompressed_tables_enabled; + } + + public void setUncompressedTablesEnabled(boolean enabled) + { + updatePropertyWithLogging("uncompressed_tables_enabled", + enabled, + () -> config.uncompressed_tables_enabled, + x -> config.uncompressed_tables_enabled = x); + } + @Override public boolean getCompactTablesEnabled() { diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java index 0c411243b4..fdc49216c8 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java @@ -38,6 +38,7 @@ import org.apache.cassandra.transport.messages.ResultMessage; abstract public class AlterSchemaStatement implements CQLStatement.SingleKeyspaceCqlStatement, SchemaTransformation { protected final String keyspaceName; // name of the keyspace affected by the statement + protected ClientState state; protected AlterSchemaStatement(String keyspaceName) { @@ -46,7 +47,10 @@ abstract public class AlterSchemaStatement implements CQLStatement.SingleKeyspac public void validate(ClientState state) { - // no-op; validation is performed while executing the statement, in apply() + // validation is performed while executing the statement, in apply() + + // Cache our ClientState for use by guardrails + this.state = state; } public ResultMessage execute(QueryState state, QueryOptions options, long queryStartNanoTime) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java index dc737765fa..2736252688 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java @@ -171,7 +171,6 @@ public abstract class AlterTableStatement extends AlterSchemaStatement private final Collection newColumns; private final boolean ifColumnNotExists; - private ClientState state; private AddColumns(String keyspaceName, String tableName, Collection newColumns, boolean ifTableExists, boolean ifColumnNotExists) { @@ -184,9 +183,6 @@ public abstract class AlterTableStatement extends AlterSchemaStatement public void validate(ClientState state) { super.validate(state); - - // save the query state to use it for guardrails validation in #apply - this.state = state; } public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table) @@ -461,6 +457,9 @@ public abstract class AlterTableStatement extends AlterSchemaStatement throw ire("read_repair must be set to 'NONE' for transiently replicated keyspaces"); } + if (!params.compression.isEnabled()) + Guardrails.uncompressedTablesEnabled.ensureEnabled(state); + return keyspace.withSwapped(keyspace.tables.withSwapped(table.withSwapped(params))); } } 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 043a7af0f2..869a062146 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java @@ -117,6 +117,9 @@ public final class CreateTableStatement extends AlterSchemaStatement throw ire("read_repair must be set to 'NONE' for transiently replicated keyspaces"); } + if (!table.params.compression.isEnabled()) + Guardrails.uncompressedTablesEnabled.ensureEnabled(state); + return schema.withAddedOrUpdated(keyspace.withSwapped(keyspace.tables.with(table))); } diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 110bad5303..6e08660b2a 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 user's ability to turn off compression + */ + public static final DisableFlag uncompressedTablesEnabled = + new DisableFlag("uncompressed_tables_enabled", + state -> !CONFIG_PROVIDER.getOrCreate(state).getUncompressedTablesEnabled(), + "Uncompressed table"); + /** * Guardrail disabling the creation of new COMPACT STORAGE tables */ @@ -464,6 +472,18 @@ public final class Guardrails implements GuardrailsMBean DEFAULT_CONFIG.setUserTimestampsEnabled(enabled); } + @Override + public boolean getUncompressedTablesEnabled() + { + return DEFAULT_CONFIG.getUncompressedTablesEnabled(); + } + + @Override + public void setUncompressedTablesEnabled(boolean enabled) + { + DEFAULT_CONFIG.setUncompressedTablesEnabled(enabled); + } + @Override public boolean getCompactTablesEnabled() { diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 8147c4e1bb..3b3473a288 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -98,7 +98,6 @@ public interface GuardrailsConfig */ int getMaterializedViewsPerTableWarnThreshold(); - /** * @return The threshold to warn when partition keys in select more than threshold. */ @@ -136,6 +135,13 @@ public interface GuardrailsConfig */ boolean getUserTimestampsEnabled(); + /** + * Returns whether tables can be uncompressed + * + * @return {@code true} if user's can disable compression, {@code false} otherwise. + */ + boolean getUncompressedTablesEnabled(); + /** * Returns whether users can create new COMPACT STORAGE tables * diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index 2dd78027d1..77f0d2dfc4 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 disable compression on tables + * + * @return {@code true} if users can disable compression on a table, {@code false} otherwise. + */ + boolean getUncompressedTablesEnabled(); + + /** + * Sets whether users can disable compression on tables + * + * @param enabled {@code true} if users can disable compression on a table, {@code false} otherwise. + */ + void setUncompressedTablesEnabled(boolean enabled); + /** * Returns whether users can create new COMPACT STORAGE tables * diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java new file mode 100644 index 0000000000..9754448136 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailAllowUncompressedTables.java @@ -0,0 +1,77 @@ +/* + * 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.Assert; +import org.junit.Test; + +import org.apache.cassandra.schema.Schema; +import org.apache.cassandra.schema.TableMetadata; + +public class GuardrailAllowUncompressedTables extends GuardrailTester +{ + private void setGuardrail(boolean enabled) + { + guardrails().setUncompressedTablesEnabled(enabled); + } + + /** + * If the guardrail has been set, creating tables with compression disabled should work + */ + @Test + public void createSuccess() + { + setGuardrail(true); + String table = createTableName(); + schemaChange(String.format("CREATE TABLE %s.%s (k int primary key, v int) WITH compression={'sstable_compression':''}", KEYSPACE, table)); + TableMetadata tmd = Schema.instance.getTableMetadata(KEYSPACE, table); + Assert.assertFalse(tmd.params.compression.isEnabled()); + } + + /** + * If the guardrail is false, creating tables with compression disabled should fail + */ + @Test + public void createFailure() throws Throwable + { + setGuardrail(false); + String table = createTableName(); + assertFails(String.format("CREATE TABLE %s.%s (k int primary key, v int) WITH compression={'sstable_compression':''}", KEYSPACE, table), "Uncompressed table is not allowed"); + } + + @Test + public void alterSuccess() + { + setGuardrail(true); + String table = createTableName(); + schemaChange(String.format("CREATE TABLE %s.%s (k int primary key, v int)", KEYSPACE, table)); + schemaChange(String.format("ALTER TABLE %s.%s WITH compression = {'sstable_compression': ''}", KEYSPACE, table)); + TableMetadata tmd = Schema.instance.getTableMetadata(KEYSPACE, table); + Assert.assertFalse(tmd.params.compression.isEnabled()); + } + + @Test + public void alterFailure() throws Throwable + { + setGuardrail(false); + String table = createTableName(); + schemaChange(String.format("CREATE TABLE %s.%s (k int primary key, v int)", KEYSPACE, table)); + assertFails(String.format("ALTER TABLE %s.%s WITH compression = {'sstable_compression': ''}", KEYSPACE, table), "Uncompressed table is not allowed"); + } +}