[DRAFT] Fix pool renewal race#835
Draft
andrzej-jackowski-scylladb wants to merge 2 commits intoscylladb:masterfrom
Draft
[DRAFT] Fix pool renewal race#835andrzej-jackowski-scylladb wants to merge 2 commits intoscylladb:masterfrom
andrzej-jackowski-scylladb wants to merge 2 commits intoscylladb:masterfrom
Conversation
When pool creation races for the same host, a slower attempt can overwrite a pool that another thread already published and close connections with in-flight requests. Capture the previous pool before connection setup, then compare that state under the session lock before publishing the new pool. If another thread changed the pool, discard the stale pool instead of replacing the current one. Keep pool removals behind the same lock so the check observes all writers. Fixes: scylladb#317
Add a deterministic unit test for the case where another thread publishes a pool while a slower add attempt is still constructing its pool. This guards against closing in-flight connections by replacing the pool that should remain current. Refs: scylladb#317
Author
|
@dkropachev, please reassign the review if there are more appropriate developers on the driver team. |
|
I'll look into it tomorrow. I did think about how to solve the issue, and couldn't figure out anything sensible, so I'm looking forward to reading your solution. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@dkropachev, @sylwiaszunejko, #317 continuously causes ScyllaDB CI failures. This is a draft PR that attempts to solve the problem, but I don't have a deep understanding of all the python-driver corner cases. Do you think this approach is worth pursuing?
=====
When pool creation races for the same host, a slower attempt can
overwrite a pool that another thread already published and close
connections with in-flight requests.
Capture the previous pool before connection setup, then compare
that state under the session lock before publishing the new pool.
If another thread changed the pool, discard the stale pool instead
of replacing the current one.
Keep pool removals behind the same lock so the check observes all
writers.
Fixes: #317
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.