-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix race condition that prevents queries from being buffered after vtgate startup #16655
Conversation
…lable when the target is replica and we have no tablets Signed-off-by: Manan Gupta <manan@planetscale.com>
…hcheck updates Signed-off-by: Manan Gupta <manan@planetscale.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…PrimaryIsNotServing Signed-off-by: Manan Gupta <manan@planetscale.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16655 +/- ##
==========================================
- Coverage 68.93% 68.92% -0.02%
==========================================
Files 1562 1564 +2
Lines 200767 201355 +588
==========================================
+ Hits 138407 138792 +385
- Misses 62360 62563 +203 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this, @GuptaManan100 ! I had some questions that hopefully you can help me understand.
…clearer Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @manan! ❤️
Thank you! |
@GuptaManan100 Should this be backported to earlier releases? 🤔 |
…gate startup (vitessio#16655) Signed-off-by: Manan Gupta <manan@planetscale.com>
It's not really a critical bug fix, so doesn't strictly fall into the category of things we backport. But I don't have strong opinions about it. |
Description
This PR fixes the race described in #16656.
The fix for the race is to also wait for the keyspace event watcher to have seen the updates for primary tablets being serving if that is something the user has requested to wait on. This fixes the race because before starting query service we can now be certain that the keyspace event watcher has processed the healthcheck responses for primaries of all the shards in the keyspaces. This ensures that we will start buffering if a primary goes non-serving subsequently.
This PR adds unit tests to ensure the behaviour of the newly added functions is as intended. It also adds comments to parts of code it is not necessarily changing but would have facilitated in helping debug the issue faster.
Related Issue(s)
Checklist
Deployment Notes