This is an automated email from the ASF dual-hosted git repository.

jmckenzie 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 105d69fdc5 Add guardrail to disallow creation of uncompressed sstables
105d69fdc5 is described below

commit 105d69fdc531d0882f628cd4bf1e34288280c12b
Author: Josh McKenzie <[email protected]>
AuthorDate: Wed Mar 30 14:19:07 2022 -0400

    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 +
 src/java/org/apache/cassandra/config/Config.java   |  1 +
 .../apache/cassandra/config/GuardrailsOptions.java | 14 ++++
 .../statements/schema/AlterSchemaStatement.java    |  6 +-
 .../statements/schema/AlterTableStatement.java     |  7 +-
 .../statements/schema/CreateTableStatement.java    |  3 +
 .../apache/cassandra/db/guardrails/Guardrails.java | 20 ++++++
 .../cassandra/db/guardrails/GuardrailsConfig.java  |  8 ++-
 .../cassandra/db/guardrails/GuardrailsMBean.java   | 14 ++++
 .../GuardrailAllowUncompressedTables.java          | 77 ++++++++++++++++++++++
 11 files changed, 147 insertions(+), 6 deletions(-)

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<ConsistencyLevel> write_consistency_levels_warned = 
Collections.emptySet();
     public volatile Set<ConsistencyLevel> 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<Column> newColumns;
         private final boolean ifColumnNotExists;
-        private ClientState state;
 
         private AddColumns(String keyspaceName, String tableName, 
Collection<Column> 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");
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to