bschoening commented on code in PR #4596:
URL: https://github.com/apache/cassandra/pull/4596#discussion_r2874773870
##########
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.
Review Comment:
Suggest dropping 'leading to heap.. and high GC...' I think it just cause
cache eviction and re-preparation.
##########
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();
Review Comment:
The query type, keyspace and table should be included in the warning
message. This will let users know which prepared statements are the problem,
something the current log message when statements are evicted doesn't do.
I.e., _The (read | insert | update ) query on keyspace.table contains only
literal values in the WHERE clause_
I wouldn't log the query or literals so that any PII values wouldn't be
shown.
##########
conf/cassandra.yaml:
##########
@@ -2536,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.
Review Comment:
I'd suggest renaming this to
**prepared_statements_require_parameters_enabled** and inverting this so false
means it just warns.
```
# When true, warns on the usage of misprepared statements
# When false, misprepared statements will result in an error
prepared_statements_require_parameters_enabled: false
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
