You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/01/10 16:25:52 UTC
[GitHub] [cassandra] ifesdjeen opened a new pull request #1383: Fix Prepared Statements behaviours after 15252
ifesdjeen opened a new pull request #1383:
URL: https://github.com/apache/cassandra/pull/1383
Patch by Alex Petrov; reviewed by Marcus Eriksson for CASSANDR-17248.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] ifesdjeen closed pull request #1383: Fix Prepared Statements behaviours after 15252 - 3.0
Posted by GitBox <gi...@apache.org>.
ifesdjeen closed pull request #1383:
URL: https://github.com/apache/cassandra/pull/1383
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] krummas commented on a change in pull request #1383: Fix Prepared Statements behaviours after 15252 - 3.0
Posted by GitBox <gi...@apache.org>.
krummas commented on a change in pull request #1383:
URL: https://github.com/apache/cassandra/pull/1383#discussion_r788572469
##########
File path: src/java/org/apache/cassandra/cql3/QueryProcessor.java
##########
@@ -399,40 +400,111 @@ public static UntypedResultSet resultify(String query, PartitionIterator partiti
return prepare(queryString, cState, cState instanceof ThriftClientState);
}
- public static ResultMessage.Prepared prepare(String queryString, ClientState clientState, boolean forThrift)
+ private volatile boolean newPreparedStatementBehaviour = false;
+ public boolean useNewPreparedStatementBehaviour()
+ {
+ if (newPreparedStatementBehaviour || DatabaseDescriptor.getForceNewPreparedStatementBehaviour())
+ return true;
+
+ synchronized (this)
+ {
+ CassandraVersion minVersion = Gossiper.instance.getMinVersion(DatabaseDescriptor.getWriteRpcTimeout(), TimeUnit.MILLISECONDS);
+ if (minVersion != null && minVersion.compareTo(NEW_PREPARED_STATEMENT_BEHAVIOUR_SINCE) >= 0)
+ {
+ logger.info("Fully upgraded to at least {}", NEW_PREPARED_STATEMENT_BEHAVIOUR_SINCE);
+ newPreparedStatementBehaviour = true;
+ }
+
+ return newPreparedStatementBehaviour;
+ }
+ }
+
+ /**
+ * This method got slightly out of hand, but this is with best intentions: to allow users to be upgraded from any
+ * prior version, and help implementers avoid previous mistakes by clearly separating fully qualified and non-fully
+ * qualified statement behaviour.
+ *
+ * Basically we need to handle 4 different hashes here;
+ * 1. fully qualified query with keyspace
+ * 2. fully qualified query without keyspace
+ * 3. unqualified query with keyspace
+ * 4. unqualified query without keyspace
+ *
+ * The correct combination to return is 2/3 - the problem is during upgrades (assuming upgrading from < 3.0.26)
+ * - Existing clients have hash 1 or 3
+ * - Query prepared on a 3.0.25 instance needs to return hash 1/3 to be able to execute it on a 3.0.25 instance
+ * - This is handled by the useNewPreparedStatementBehaviour flag - while there still are 3.0.25 instances in
+ * the cluster we always return hash 1/3
+ * - Once fully upgraded we start returning hash 2/3, this will cause a prepared statement id mismatch for existing
+ * clients, but they will be able to continue using the old prepared statement id after that exception since we
+ * store the query both with and without keyspace.
+ */
+ public ResultMessage.Prepared prepare(String queryString, ClientState clientState, boolean forThrift)
{
- ResultMessage.Prepared existing = getStoredPreparedStatement(queryString, clientState.getRawKeyspace(), forThrift);
- if (existing != null)
- return existing;
+ boolean newPreparedStatementBehaviour = useNewPreparedStatementBehaviour();
+ MD5Digest hashWithoutKeyspace = computeId(queryString, null);
+ MD5Digest hashWithKeyspace = computeId(queryString, clientState.getRawKeyspace());
+ ParsedStatement.Prepared cachedWithoutKeyspace = preparedStatements.get(hashWithoutKeyspace);
+ ParsedStatement.Prepared cachedWithKeyspace = preparedStatements.get(hashWithKeyspace);
+ // We assume it is only safe to return cached prepare if we have both instances
+ boolean safeToReturnCached = cachedWithoutKeyspace != null && cachedWithKeyspace != null;
+
+ // If we're on 3.0.26 or later, try to use caches
Review comment:
this comment is confusing - can probably remove, we try to use the caches even in the legacy case
##########
File path: src/java/org/apache/cassandra/cql3/QueryProcessor.java
##########
@@ -399,40 +400,111 @@ public static UntypedResultSet resultify(String query, PartitionIterator partiti
return prepare(queryString, cState, cState instanceof ThriftClientState);
}
- public static ResultMessage.Prepared prepare(String queryString, ClientState clientState, boolean forThrift)
+ private volatile boolean newPreparedStatementBehaviour = false;
+ public boolean useNewPreparedStatementBehaviour()
+ {
+ if (newPreparedStatementBehaviour || DatabaseDescriptor.getForceNewPreparedStatementBehaviour())
+ return true;
+
+ synchronized (this)
+ {
+ CassandraVersion minVersion = Gossiper.instance.getMinVersion(DatabaseDescriptor.getWriteRpcTimeout(), TimeUnit.MILLISECONDS);
+ if (minVersion != null && minVersion.compareTo(NEW_PREPARED_STATEMENT_BEHAVIOUR_SINCE) >= 0)
+ {
+ logger.info("Fully upgraded to at least {}", NEW_PREPARED_STATEMENT_BEHAVIOUR_SINCE);
+ newPreparedStatementBehaviour = true;
+ }
+
+ return newPreparedStatementBehaviour;
+ }
+ }
+
+ /**
+ * This method got slightly out of hand, but this is with best intentions: to allow users to be upgraded from any
+ * prior version, and help implementers avoid previous mistakes by clearly separating fully qualified and non-fully
+ * qualified statement behaviour.
+ *
+ * Basically we need to handle 4 different hashes here;
+ * 1. fully qualified query with keyspace
+ * 2. fully qualified query without keyspace
+ * 3. unqualified query with keyspace
+ * 4. unqualified query without keyspace
+ *
+ * The correct combination to return is 2/3 - the problem is during upgrades (assuming upgrading from < 3.0.26)
+ * - Existing clients have hash 1 or 3
+ * - Query prepared on a 3.0.25 instance needs to return hash 1/3 to be able to execute it on a 3.0.25 instance
Review comment:
think this should be "query prepared on a 3.0.26 instance ... "
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org