-
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 panic in vttablet when closing topo server twice #17094
Fix panic in vttablet when closing topo server twice #17094
Conversation
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17094 +/- ##
==========================================
+ Coverage 67.15% 67.44% +0.28%
==========================================
Files 1571 1571
Lines 252250 252249 -1
==========================================
+ Hits 169409 170120 +711
+ Misses 82841 82129 -712 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
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.
❤️
Description
As described in #17093 we were closing the topo server in vttablet twice: in a defer function, and right before returning an error. This PR fixes the issue.
Using the same reproduction as the one detailed in the issue, we get:
As mentioned in the issue, this bug is present on
release-21.0
and causes a crash (though it only happens if vttablet was not going to start anyway) so I am adding theBackport to: release-21.0
label to this PR.Related Issue(s)
Close()
of the*topo.Server
leads to panics when starting vttablet #17093Checklist