[ 
https://issues.apache.org/jira/browse/CASSANDRA-17401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18045743#comment-18045743
 ] 

Alex Petrov commented on CASSANDRA-17401:
-----------------------------------------

I am personally not a big fan of adding version-dependent logic, since it 
complicates maintenance and testing substantially. I think we do need to 
replace invalidate with invalidateAll regardless of this ticket, since this is 
a race, too. Another thing I'd do is I'd check if removal listener is 
sufficient to clean up the prep statements table, and avoid explicitly calling 
{{removePreparedStatement}}. 

Another approach is to evict only if we see that one of the statements is 
cached but not the other (i.e. {{ (a == null && b != null) && (b == null && a 
!= null) }}), which is going to substantially reduce the window of race 
possibility. This, combined with what I wrote earlier, namely

bq. Another addition to the patch I would do is adding an if statement when 
retrieving {{cachedWithKeyspace}} after {{cachedWithoutKeyspace}}, and only 
going for a second get if hashes are actually different.

should be sufficient to trigger only a handful of spurious re-prepares. WDYT?

> Race condition in QueryProcessor causes just prepared statement not to be in 
> the prepared statements cache
> ----------------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-17401
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17401
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Messaging/Client
>            Reporter: Ivan Senic
>            Assignee: Jaydeepkumar Chovatia
>            Priority: Normal
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> The changes in the 
> [QueryProcessor#prepare|https://github.com/apache/cassandra/blame/cassandra-4.0.2/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L575-L638]
>  method that were introduced in versions *4.0.2* and *3.11.12* can cause a 
> race condition between two threads trying to concurrently prepare the same 
> statement. This race condition can cause removing of a prepared statement 
> from the cache, after one of the threads has received the result of the 
> prepare and eventually uses MD5Digest to call 
> [QueryProcessor#getPrepared|https://github.com/apache/cassandra/blame/cassandra-4.0.2/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L212-L215].
> The race condition looks like this:
>  * Thread1 enters _prepare_ method and resolves _safeToReturnCached_ as false
>  * Thread1 executes eviction of hashes
>  * Thread2 enters _prepare_ method and resolves _safeToReturnCached_ as false
>  * Thread1 prepares the statement and caches it
>  * Thread1 returns the result of the prepare
>  * Thread2 executes eviction of hashes
>  * Thread1 tries to execute the prepared statement with the received 
> MD5Digest, but statement is not in the cache as it was evicted by Thread2
> I tried to reproduce this by using a Java driver, but hitting this case from 
> a client side is highly unlikely and I can not simulate the needed race 
> condition. However, we can easily reproduce this in Stargate (details 
> [here|https://github.com/stargate/stargate/pull/1647]), as it's closer to 
> QueryProcessor.
> Reproducing this in a unit test is fairly easy. I am happy to showcase this 
> if needed.
> Note that the issue can occur only when  safeToReturnCached is resolved as 
> false.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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

Reply via email to