Fix stale broken cluster connection in CassandraIO (#39788) - #39803
Fix stale broken cluster connection in CassandraIO (#39788)#39803sharan-malyala wants to merge 1 commit into
Conversation
* Fix for CassandraIO read connection issue * Fixed a presubmit failure * Added test case
|
Assigning reviewers: R: @ahmedabu98 for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
R: @Amar3tto |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
|
||
| Cluster cachedCluster = clusterMap.get(clusterHash); | ||
|
|
||
| if (cachedCluster != null && cachedCluster.isClosed()) { |
There was a problem hiding this comment.
Inside this if clause it modified sessionMap and clusterMap. Even though they are concurrent hash map, is there a risk of racing?
There was a problem hiding this comment.
If we dont cleanup the session and just re-create cluster object, the broken session will be returned and used by the work items leading to failure.
There was a problem hiding this comment.
I was thinking about the following scenario:
-
A session corrupted and cluster closed
-
Caller A runs getSession(), found cachedCluster.isClosed
-
Caller B also runs getSession(), found cachedCluster.isClosed
-
Caller A cleaned up session and cluster cache, recreated cluster connection
-
Caller B clean up session again, because it's already inside the if clause. But it should not do so as the cache is current valid, may cause session leak.
Basically it seems the whole getSession now needs to be synchronized.
There was a problem hiding this comment.
I see, this is already merged in #39788 however a race appears possible
There was a problem hiding this comment.
I will check further.
3 participants
This PR fixes an issue in CassandraIO where read operations can fail if the underlying Cassandra Cluster connection is broken due to a transient issue avoiding subsequent work failures.
Previously, ConnectionManager would cache the Cluster and Session and continue to return them even if the cluster was in a broken state, leading to subsequent operation failures.
This change adds a validation step in ConnectionManager.getSession(). If the cached Cluster is found to be closed, the stale references are proactively removed from sessionMap and clusterMap. This ensures that a new, healthy Cluster and Session are transparently recreated via computeIfAbsent.