Skip to content

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

Merged
merged 14 commits into from
Aug 14, 2024
Merged

fix: avoid deleting peers on graceful close #14165

merged 14 commits into from
Aug 14, 2024

Conversation

sreya
Copy link
Collaborator

@sreya 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.
@sreya sreya requested a review from spikecurtis August 5, 2024 15:16
Copy link
Contributor

@spikecurtis spikecurtis left a 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.

@sreya sreya requested a review from spikecurtis August 12, 2024 00:03
Copy link
Contributor

@spikecurtis spikecurtis left a 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.

@sreya sreya requested a review from spikecurtis August 13, 2024 15:00
Copy link
Contributor

@spikecurtis spikecurtis left a 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.

@sreya sreya merged commit 4fc0479 into main Aug 14, 2024
27 checks passed
@sreya sreya deleted the jon/pgc branch August 14, 2024 19:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants