-
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
discovery: Fix tablets removed from healthcheck when topo server GetTablet call fails #15633
discovery: Fix tablets removed from healthcheck when topo server GetTablet call fails #15633
Conversation
Signed-off-by: Brendan Dougherty <[email protected]>
…rtial result error Signed-off-by: Brendan Dougherty <[email protected]>
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15633 +/- ##
==========================================
- Coverage 68.13% 68.13% -0.01%
==========================================
Files 1556 1556
Lines 194984 195032 +48
==========================================
+ Hits 132849 132881 +32
- Misses 62135 62151 +16 ☔ View full report in Codecov by Sentry. |
I believe this is the real fix for a bug originally reported in #3987 !! |
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.
This is great work. The testing improvements are particularly nice.
We'll get another review from @mattlord but LGTM
We don't need to backport beyond v19 because we only started using GetTablets
in v19.
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.
Excellent work on this @brendar ! Thank you for the great contribution ❤️
…opo server GetTablet call fails (#15633) (#15681) Signed-off-by: Matt Lord <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Matt Lord <[email protected]>
Description
This PR fixes two bugs that can appear when using Zookeeper as the topo server
In order to test these fixes this PR also introduces a more flexible way to return fake errors from memorytopo.
Since the impact of this bug could be pretty disruptive, I think it should be backported to v19.
Related Issue(s)
Fixes #15632
Checklist
Deployment Notes
n/a