-
Notifications
You must be signed in to change notification settings - Fork 876
fix: avoid deleting peers on graceful close #14165
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
Conversation
sreya
commented
Aug 5, 2024
- Fixes an issue where a coordinator deletes all its peers on shutdown. This can cause disconnects whenever a coderd is redeployed.
- Fixes an issue where a coordinator deletes all its peers on shutdown. This can cause disconnects whenever a coderd is redeployed.
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.
I added a bunch of stuff in #12155 to ensure that the binder and tunneler complete before the querier closes, since that would delete the Coordinator row from the DB. Now that we aren't deleting that row, the ordering isn't necessary, and there are comments I added that are no longer true.
I think we should keep the tracking of when things shut down, though, so that Close()
doesn't return while there are still PGCoordinator goroutines running.
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.
I like the use of the single query for cleanup, and the new tests.
The only blocking issue, I think, is the race with the binding workers.
A few other minor comments inline as well.
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.
Minor issue inline, but I don't need to review again.