CASSANDRA-21139 - Feature/guardrail for misprepare statements

conf/cassandra.yaml

@@ -2531,6 +2531,10 @@ drop_compact_storage_enabled: false
 # This would also apply to system keyspaces.
 # maximum_replication_factor_warn_threshold: -1
 # maximum_replication_factor_fail_threshold: -1
+#
+# When true, allows the usage of misprepared statements, only warns in the logs.
+# When false, misprepared statements will result in an error to the client. Default is true.
+# misprepared_statements_enabled: true
 
 #role_name_policy:
 #  # Implementation class of a validator. When not in form of FQCN, the

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

@@ -937,6 +937,7 @@ public static void setClientMode(boolean clientMode)
     public volatile boolean user_timestamps_enabled = true;
     public volatile boolean alter_table_enabled = true;
     public volatile boolean group_by_enabled = true;
+    public volatile boolean misprepared_statements_enabled = true;
     public volatile boolean bulk_load_enabled = true;
     public volatile boolean drop_truncate_table_enabled = true;
     public volatile boolean drop_keyspace_enabled = true;

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

@@ -5483,6 +5483,15 @@ public static void setUseStatementsEnabled(boolean enabled)
         }
     }
 
+    public static boolean getMispreparedStatementsEnabled()
+    {
+        return conf.misprepared_statements_enabled;
+    }
+
+    public static void setMispreparedStatementsEnabled(boolean enabled)
+    {
+        conf.misprepared_statements_enabled = enabled;
+    }
 
     public static AccordSpec getAccord()
     {

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

@@ -1329,6 +1329,21 @@ public boolean getVectorTypeEnabled()
         return config.vector_type_enabled;
     }
 
+    @Override
+    public boolean getMispreparedStatementsEnabled()
+    {
+        return config.misprepared_statements_enabled;
+    }
+
+
+    public void setMispreparedStatementsEnabled(boolean enabled)
+    {
+        updatePropertyWithLogging("misprepared_statements_enabled",
+                                  enabled,
+                                  () -> config.misprepared_statements_enabled,
+                                  x -> config.misprepared_statements_enabled = x);
+    }
+
     @Override
     public void setVectorTypeEnabled(boolean enabled)
     {

src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java

@@ -409,7 +409,21 @@ public void validate(ClientState state) throws InvalidRequestException
         checkFalse(isVirtual() && hasConditions(), "Conditional updates are not supported by virtual tables");
 
         if (attrs.isTimestampSet())
+        {
             Guardrails.userTimestampsEnabled.ensureEnabled(state);
+        }
+        // Misprepare guardrail shouldn't block system keyspaces
+        if (SchemaConstants.isSystemKeyspace(metadata.keyspace))
+            return;
+
+        boolean hasRestriction = restrictions != null &&
+                                 restrictions.hasPartitionKeyRestrictions() ||
+                                 restrictions.hasClusteringColumnsRestrictions() ||
+                                 restrictions.hasNonPrimaryKeyRestrictions();
+        if (getBindVariables().isEmpty() && hasRestriction)
+        {
+            Guardrails.onMisprepared(state);
+        }
     }
 
     public void validateDiskUsage(QueryOptions options, ClientState state)

src/java/org/apache/cassandra/cql3/statements/SelectStatement.java

@@ -339,8 +339,19 @@ public void authorize(ClientState state) throws InvalidRequestException, Unautho
 
     public void validate(ClientState state) throws InvalidRequestException
     {
-        if (parameters.allowFiltering && !SchemaConstants.isSystemKeyspace(table.keyspace))
+        if (SchemaConstants.isSystemKeyspace(table.keyspace))
+            return;
+        if (parameters.allowFiltering)
             Guardrails.allowFilteringEnabled.ensureEnabled(state);
+        boolean hasRestrictions = restrictions != null &&
+                                  (restrictions.hasPartitionKeyRestrictions() ||
+                                  restrictions.hasClusteringColumnsRestrictions() ||
+                                  restrictions.hasNonPrimaryKeyRestrictions());
+        if (getBindVariables().isEmpty()
+            && hasRestrictions)
+        {
+            Guardrails.onMisprepared(state);
+        }
     }
 
     @Override

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

@@ -54,6 +54,9 @@ public final class Guardrails implements GuardrailsMBean
     public static final String MBEAN_NAME = "org.apache.cassandra.db:type=Guardrails";
 
     public static final GuardrailsConfigProvider CONFIG_PROVIDER = GuardrailsConfigProvider.instance;
+
+    public static final String MISPREPARED_STATEMENT_WARN_MESSAGE = "This query contains only literal values in the WHERE clause. " + "Using one or more '?' placeholder values (bind markers) allow a prepared statement to be reused.";
+
     private static final GuardrailsOptions DEFAULT_CONFIG = DatabaseDescriptor.getGuardrailsConfig();
 
     @VisibleForTesting
@@ -613,6 +616,17 @@ public final class Guardrails implements GuardrailsMBean
                      format("The keyspace %s has a replication factor of %s, above the %s threshold of %s.",
                             what, value, isWarning ? "warning" : "failure", threshold));
 
+    /**
+     * Prevents cache overflow and eviction caused by the anti-pattern of preparing queries with
+     * hardcoded literals instead of bind markers. This prevents filling the statement cache with non-reusable entries.
+     */
+
+    public static final EnableFlag mispreparedStatementsEnabled =
+    new EnableFlag("misprepared_statements_enabled",
+                   "misprepared statements create non-reusable query entries and cause cache overflow",
+                   state -> CONFIG_PROVIDER.getOrCreate(state).getMispreparedStatementsEnabled(),
+                   "misprepared statements");
+
     public static final MaxThreshold maximumAllowableTimestamp =
     new MaxThreshold("maximum_timestamp",
                      "Timestamps too far in the future can lead to data that can't be easily overwritten",
@@ -705,6 +719,18 @@ public void setKeyspacesThreshold(int warn, int fail)
         DEFAULT_CONFIG.setKeyspacesThreshold(warn, fail);
     }
 
+    @Override
+    public boolean getMispreparedStatementsEnabled()
+    {
+        return DEFAULT_CONFIG.getMispreparedStatementsEnabled();
+    }
+
+    @Override
+    public void setMispreparedStatementsEnabled(boolean enabled)
+    {
+        DEFAULT_CONFIG.setMispreparedStatementsEnabled(enabled);
+    }
+
     @Override
     public int getTablesWarnThreshold()
     {
@@ -1886,4 +1912,10 @@ private static long minimumTimestampAsRelativeMicros(@Nullable DurationSpec.Long
                ? Long.MIN_VALUE
                : (ClientState.getLastTimestampMicros() - duration.toMicroseconds());
     }
+
+    public static void onMisprepared(ClientState state)
+    {
+        mispreparedStatementsEnabled.ensureEnabled(state);
+        mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE);
+    }
 }

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

@@ -406,6 +406,39 @@ public interface GuardrailsConfig
      */
     boolean getVectorTypeEnabled();
 
+    /**
+     * <p>
+     * A statement is considered "mis-prepared" if it contains only hardcoded liter values and without any bind markers
+     * instead of bind markers. This is a performance anti-pattern because it prevents
+     * query plan reuse and floods the server-side Prepared Statement Cache with
+     * unique entries, leading to heap exhaustion and high GC pressure.
+     * <p>
+     * <b>LWT and Batch Considerations:</b>
+     * This check applies to both the {@code WHERE} clause and the {@code IF} conditions
+     * in Lightweight Transactions (LWT). For example, the following is considered
+     * mis-prepared because of the hardcoded literals:
+     * <pre>{@code
+     * UPDATE users SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'
+     * }</pre>
+     * To be compliant, it should use bind markers ({@code ?}):
+     * <pre>{@code
+     * UPDATE users SET description = ? WHERE id = ? AND name = ? IF description = ?
+     * }</pre>
+     * For {@code BATCH} statements, if any individual statement within the batch is
+     * identified as mis-prepared, the entire batch triggers this guardrail.
+
+     *
+     * @return true if the usage of mis-prepared statements is enabled, false otherwise. Returns true by default.
+     * {@code false} if mis-prepared statements should be strictly rejected, causing the query to fail.
+     */
+
+    boolean getMispreparedStatementsEnabled();
+
+    /**
+     * sets whether misprepared statements is enabled
+     */
+    void setMispreparedStatementsEnabled(boolean enabled);
+
     /**
      * Sets whether new columns can be created with vector type
      *

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

@@ -54,6 +54,13 @@ public interface GuardrailsMBean
      */
     void setKeyspacesThreshold(int warn, int fail);
 
+    /**
+     * @return the use of misprepared statement is enabled
+     */
+    boolean getMispreparedStatementsEnabled();
+
+    void setMispreparedStatementsEnabled(boolean enabled);
+
     /**
      * @return The threshold to warn when creating more tables than threshold.
      * -1 means disabled.

src/java/org/apache/cassandra/service/StorageService.java

@@ -4836,6 +4836,18 @@ public void setColumnIndexCacheSize(int cacheSizeInKB)
         logger.info("Updated column_index_cache_size to {}", cacheSizeInKB);
     }
 
+    @Override
+    public boolean getMispreparedStatementsEnabled()
+    {
+        return DatabaseDescriptor.getMispreparedStatementsEnabled();
+    }
+
+    @Override
+    public void setMispreparedStatementsEnabled(boolean enabled)
+    {
+        DatabaseDescriptor.setMispreparedStatementsEnabled(enabled);
+    }
+
     /*
      * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
      * For consistency purposes, the respective getter "getColumnIndexCacheSize" was also deprecated and replaced by

src/java/org/apache/cassandra/service/StorageServiceMBean.java

@@ -1097,6 +1097,16 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion,
     @Deprecated(since = "5.0")
     public void setColumnIndexCacheSize(int cacheSizeInKB);
 
+    /**
+     * Returns whether use of misprepared statements is enabled
+     */
+    public boolean getMispreparedStatementsEnabled();
+
+    /**
+     * Sets whether use of misprepared statements is enabled
+     */
+    public void setMispreparedStatementsEnabled(boolean enabled);
+
     /** Returns the threshold for skipping the column index when caching partition info **/
     public int getColumnIndexCacheSizeInKiB();
 

test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java

@@ -781,6 +781,27 @@ public void testRowIndexSizeWarnEqAbort()
         DatabaseDescriptor.applyReadThresholdsValidations(conf);
     }
 
+    @Test
+    public void testMispreparedStatementsEnabled() {
+        boolean originalValue = DatabaseDescriptor.getMispreparedStatementsEnabled();
+        try
+        {
+            DatabaseDescriptor.setMispreparedStatementsEnabled(true);
+            Assert.assertTrue("Value should be true after setting to true",
+                              DatabaseDescriptor.getMispreparedStatementsEnabled());
+
+            DatabaseDescriptor.setMispreparedStatementsEnabled(false);
+            Assert.assertFalse("Value should be false after setting to false",
+                               DatabaseDescriptor.getMispreparedStatementsEnabled());
+
+            Assert.assertTrue("Default value of misprepared_statement_enabled must be true", originalValue);
+        }
+        finally
+        {
+            DatabaseDescriptor.setMispreparedStatementsEnabled(originalValue);
+        }
+    }
+
     @Test
     public void testRowIndexSizeWarnEnabledAbortDisabled()
     {