Implement a guardrail for not having zero default_time_to_live on tab…

…les with TWCS

patch by Stefan Miklosovic; reviewed by Andrés de la Peña, Josh McKenzie, Brandon Williams and Brad Schoening for CASSANDRA-18042

Co-authored-by: Andrés de la Peña <a.penya.garcia@gmail.com>

CHANGES.txt

@@ -1,4 +1,5 @@
 4.2
+ * Implement a guardrail for not having zero default_time_to_live on tables with TWCS (CASSANDRA-18042)
  * Add CQL scalar functions for collection aggregation (CASSANDRA-18060)
  * Make cassandra.replayList property for CommitLogReplayer possible to react on keyspaces only (CASSANDRA-18044)
  * Add Mathematical functions (CASSANDRA-17221)

conf/cassandra.yaml

@@ -1862,6 +1862,19 @@ drop_compact_storage_enabled: false
 # maximum_replication_factor_warn_threshold: -1
 # maximum_replication_factor_fail_threshold: -1
 
+# Guardrail to enable a CREATE or ALTER TABLE statement when default_time_to_live is set to 0
+# and the table is using TimeWindowCompactionStrategy compaction or a subclass of it.
+# It is suspicious to use default_time_to_live set to 0 with such compaction strategy.
+# Please keep in mind that data will not start to automatically expire after they are older than
+# a respective compaction window unit of a certain size. Please set TTL for your INSERT or UPDATE
+# statements if you expect data to be expired as table settings will not do it.
+# Defaults to true. If set to false, such statements fail and zero_ttl_on_twcs_warned flag is irrelevant.
+#zero_ttl_on_twcs_enabled: true
+# Guardrail to warn a user upon executing CREATE or ALTER TABLE statement when default_time_to_live is set to 0
+# and the table is using TimeWindowCompactionStrategy compaction or a subclass of it. Defaults to true.
+# if zero_ttl_on_twcs_enabled is set to false, this property is irrelevant as such statements will fail.
+#zero_ttl_on_twcs_warned: true
+
 # Startup Checks are executed as part of Cassandra startup process, not all of them
 # are configurable (so you can disable them) but these which are enumerated bellow.
 # Uncomment the startup checks and configure them appropriately to cover your needs.

src/java/org/apache/cassandra/config/Config.java

@@ -869,6 +869,8 @@ public static void setClientMode(boolean clientMode)
     public volatile int minimum_replication_factor_fail_threshold = -1;
     public volatile int maximum_replication_factor_warn_threshold = -1;
     public volatile int maximum_replication_factor_fail_threshold = -1;
+    public volatile boolean zero_ttl_on_twcs_warned = true;
+    public volatile boolean zero_ttl_on_twcs_enabled = true;
 
     public volatile DurationSpec.LongNanosecondsBound streaming_state_expires = new DurationSpec.LongNanosecondsBound("3d");
     public volatile DataStorageSpec.LongBytesBound streaming_state_size = new DataStorageSpec.LongBytesBound("40MiB");

src/java/org/apache/cassandra/config/GuardrailsOptions.java

@@ -730,6 +730,36 @@ public void setMaximumReplicationFactorThreshold(int warn, int fail)
                                   x -> config.maximum_replication_factor_fail_threshold = x);
     }
 
+    @Override
+    public boolean getZeroTTLOnTWCSWarned()
+    {
+        return config.zero_ttl_on_twcs_warned;
+    }
+
+    @Override
+    public void setZeroTTLOnTWCSWarned(boolean value)
+    {
+        updatePropertyWithLogging("zero_ttl_on_twcs_warned",
+                                  value,
+                                  () -> config.zero_ttl_on_twcs_warned,
+                                  x -> config.zero_ttl_on_twcs_warned = x);
+    }
+
+    @Override
+    public boolean getZeroTTLOnTWCSEnabled()
+    {
+        return config.zero_ttl_on_twcs_enabled;
+    }
+
+    @Override
+    public void setZeroTTLOnTWCSEnabled(boolean value)
+    {
+        updatePropertyWithLogging("zero_ttl_on_twcs_enabled",
+                                  value,
+                                  () -> config.zero_ttl_on_twcs_enabled,
+                                  x -> config.zero_ttl_on_twcs_enabled = x);
+    }
+
     private static <T> void updatePropertyWithLogging(String propertyName, T newValue, Supplier<T> getter, Consumer<T> setter)
     {
         T oldValue = getter.get();

src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java

@@ -26,6 +26,8 @@
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.cql3.CQLStatement;
 import org.apache.cassandra.cql3.QueryOptions;
+import org.apache.cassandra.db.compaction.TimeWindowCompactionStrategy;
+import org.apache.cassandra.db.guardrails.Guardrails;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.schema.*;
 import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff;
@@ -140,6 +142,14 @@ private void validateKeyspaceName()
         }
     }
 
+    protected void validateDefaultTimeToLive(TableParams params)
+    {
+        if (params.defaultTimeToLive == 0
+            && !SchemaConstants.isSystemKeyspace(keyspaceName)
+            && TimeWindowCompactionStrategy.class.isAssignableFrom(params.compaction.klass()))
+            Guardrails.zeroTTLOnTWCSEnabled.ensureEnabled(state);
+    }
+
     private void grantPermissionsOnResource(IResource resource, AuthenticatedUser user)
     {
         try

src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java

@@ -443,6 +443,8 @@ public void validate(ClientState state)
             super.validate(state);
 
             Guardrails.tableProperties.guard(attrs.updatedProperties(), attrs::removeProperty, state);
+
+            validateDefaultTimeToLive(attrs.asNewTableParams());
         }
 
         public KeyspaceMetadata apply(KeyspaceMetadata keyspace, TableMetadata table)

src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java

@@ -149,6 +149,8 @@ public void validate(ClientState state)
         // Guardrail to check whether creation of new COMPACT STORAGE tables is allowed
         if (useCompactStorage)
             Guardrails.compactTablesEnabled.ensureEnabled(state);
+
+        validateDefaultTimeToLive(attrs.asNewTableParams());
     }
 
     SchemaChange schemaChangeEvent(KeyspacesDiff diff)

src/java/org/apache/cassandra/db/guardrails/EnableFlag.java

@@ -32,6 +32,7 @@
  */
 public class EnableFlag extends Guardrail
 {
+    private final Predicate<ClientState> warned;
     private final Predicate<ClientState> enabled;
     private final String featureName;
 
@@ -46,8 +47,32 @@ public class EnableFlag extends Guardrail
      *                    EnableFlag#ensureEnabled(String, ClientState)} can specify a different {@code featureName}.
      */
     public EnableFlag(String name, @Nullable String reason, Predicate<ClientState> enabled, String featureName)
+    {
+        this(name, reason, (state) -> false, enabled, featureName);
+    }
+
+    /**
+     * Creates a new {@link EnableFlag} guardrail.
+     *
+     * @param name        the identifying name of the guardrail
+     * @param reason      the optional description of the reason for guarding the operation
+     * @param warned      a {@link ClientState}-based supplier of boolean indicating whether warning should be
+     *                    emitted even guardrail as such has passed. If guardrail fails, the warning will not be
+     *                    emitted. This might be used for cases when we want to warn a user regardless of successful
+     *                    guardrail execution.
+     * @param enabled     a {@link ClientState}-based supplier of boolean indicating whether the feature guarded by this
+     *                    guardrail is enabled.
+     * @param featureName The feature that is guarded by this guardrail (for reporting in error messages), {@link
+     *                    EnableFlag#ensureEnabled(String, ClientState)} can specify a different {@code featureName}.
+     */
+    public EnableFlag(String name,
+                      @Nullable String reason,
+                      Predicate<ClientState> warned,
+                      Predicate<ClientState> enabled,
+                      String featureName)
     {
         super(name, reason);
+        this.warned = warned;
         this.enabled = enabled;
         this.featureName = featureName;
     }
@@ -93,7 +118,16 @@ public void ensureEnabled(@Nullable ClientState state)
      */
     public void ensureEnabled(String featureName, @Nullable ClientState state)
     {
-        if (!isEnabled(state))
+        if (!enabled(state))
+            return;
+
+        if (!enabled.test(state))
+        {
             fail(featureName + " is not allowed", state);
+            return;
+        }
+
+        if (warned.test(state))
+            warn(featureName + " is not recommended");
     }
 }

src/java/org/apache/cassandra/db/guardrails/Guardrails.java

@@ -33,6 +33,7 @@
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.config.GuardrailsOptions;
 import org.apache.cassandra.db.ConsistencyLevel;
+import org.apache.cassandra.db.compaction.TimeWindowCompactionStrategy;
 import org.apache.cassandra.locator.InetAddressAndPort;
 import org.apache.cassandra.service.disk.usage.DiskUsageBroadcaster;
 import org.apache.cassandra.utils.MBeanWrapper;
@@ -199,6 +200,20 @@ public final class Guardrails implements GuardrailsMBean
                    state -> CONFIG_PROVIDER.getOrCreate(state).getCompactTablesEnabled(),
                    "Creation of new COMPACT STORAGE tables");
 
+    /**
+     * Guardrail to warn or fail a CREATE or ALTER TABLE statement when default_time_to_live is set to 0 and
+     * the table is using TimeWindowCompactionStrategy compaction or a subclass of it.
+     */
+    public static final EnableFlag zeroTTLOnTWCSEnabled =
+    new EnableFlag("zero_ttl_on_twcs",
+                   "It is suspicious to use default_time_to_live set to 0 with such compaction strategy. " +
+                   "Please keep in mind that data will not start to automatically expire after they are older " +
+                   "than a respective compaction window unit of a certain size. Please set TTL for your INSERT or UPDATE " +
+                   "statements if you expect data to be expired as table settings will not do it. ",
+                   state -> CONFIG_PROVIDER.getOrCreate(state).getZeroTTLOnTWCSWarned(),
+                   state -> CONFIG_PROVIDER.getOrCreate(state).getZeroTTLOnTWCSEnabled(),
+                   "0 default_time_to_live on a table with " + TimeWindowCompactionStrategy.class.getSimpleName() + " compaction strategy");
+
     /**
      * Guardrail on the number of elements returned within page.
      */
@@ -1013,6 +1028,30 @@ public void setMinimumReplicationFactorThreshold(int warn, int fail)
         DEFAULT_CONFIG.setMinimumReplicationFactorThreshold(warn, fail);
     }
 
+    @Override
+    public boolean getZeroTTLOnTWCSEnabled()
+    {
+        return DEFAULT_CONFIG.getZeroTTLOnTWCSEnabled();
+    }
+
+    @Override
+    public void setZeroTTLOnTWCSEnabled(boolean value)
+    {
+        DEFAULT_CONFIG.setZeroTTLOnTWCSEnabled(value);
+    }
+
+    @Override
+    public boolean getZeroTTLOnTWCSWarned()
+    {
+        return DEFAULT_CONFIG.getZeroTTLOnTWCSWarned();
+    }
+
+    @Override
+    public void setZeroTTLOnTWCSWarned(boolean value)
+    {
+        DEFAULT_CONFIG.setZeroTTLOnTWCSWarned(value);
+    }
+
     private static String toCSV(Set<String> values)
     {
         return values == null || values.isEmpty() ? "" : String.join(",", values);

src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java

@@ -319,4 +319,35 @@ public interface GuardrailsConfig
      * @return The threshold to fail when replication factor is greater than threshold.
      */
     int getMaximumReplicationFactorFailThreshold();
+
+    /**
+     * Returns whether warnings will be emitted when usage of 0 default TTL on a
+     * table with TimeWindowCompactionStrategy is detected.
+     *
+     * @return {@code true} if warnings will be emitted, {@code false} otherwise.
+     */
+    boolean getZeroTTLOnTWCSWarned();
+
+    /**
+     * Sets whether warnings will be emitted when usage of 0 default TTL on a
+     * table with TimeWindowCompactionStrategy is detected.
+     *
+     * @param value {@code true} if warning will be emitted, {@code false} otherwise.
+     */
+    void setZeroTTLOnTWCSWarned(boolean value);
+
+    /**
+     * Returns whether it is allowed to create or alter table to use 0 default TTL with TimeWindowCompactionStrategy.
+     * If it is not, such query will fail.
+     *
+     * @return {@code true} if 0 default TTL is allowed on TWCS table, {@code false} otherwise.
+     */
+    boolean getZeroTTLOnTWCSEnabled();
+
+    /**
+     * Sets whether users can use 0 default TTL on a table with TimeWindowCompactionStrategy.
+     *
+     * @param value {@code true} if 0 default TTL on TWCS tables is allowed, {@code false} otherwise.
+     */
+    void setZeroTTLOnTWCSEnabled(boolean value);
 }

src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java

@@ -620,4 +620,35 @@ public interface GuardrailsMBean
      *             -1 means disabled.
      */
     void setMaximumReplicationFactorThreshold (int warn, int fail);
+
+    /**
+     * Returns whether warnings will be emitted when usage of 0 default TTL on a
+     * table with TimeWindowCompactionStrategy is detected.
+     *
+     * @return {@code true} if warnings will be emitted, {@code false} otherwise.
+     */
+    boolean getZeroTTLOnTWCSWarned();
+
+    /**
+     * Sets whether warnings will be emitted when usage of 0 default TTL on a
+     * table with TimeWindowCompactionStrategy is detected.
+     *
+     * @param value {@code true} if warning will be emitted, {@code false} otherwise.
+     */
+    void setZeroTTLOnTWCSWarned(boolean value);
+
+    /**
+     * Returns whether it is allowed to create or alter table to use 0 default TTL with TimeWindowCompactionStrategy.
+     * If it is not, such query will fail.
+     *
+     * @return {@code true} if 0 default TTL is allowed on TWCS table, {@code false} otherwise.
+     */
+    boolean getZeroTTLOnTWCSEnabled();
+
+    /**
+     * Sets whether users can use 0 default TTL on a table with TimeWindowCompactionStrategy.
+     *
+     * @param value {@code true} if 0 default TTL on TWCS tables is allowed, {@code false} otherwise.
+     */
+    void setZeroTTLOnTWCSEnabled(boolean value);
 }