Skip to content

fix: avoid deleting peers on graceful close#14165

Merged
sreya merged 14 commits into
mainfrom
jon/pgc
Aug 14, 2024
Merged

fix: avoid deleting peers on graceful close#14165
sreya merged 14 commits into
mainfrom
jon/pgc

Conversation

@sreya

@sreya sreya commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator
  • 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

@spikecurtis spikecurtis left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread enterprise/tailnet/pgcoord.go Outdated
Comment thread enterprise/tailnet/pgcoord_test.go
Comment thread enterprise/tailnet/pgcoord_test.go
Comment thread enterprise/tailnet/pgcoord_test.go
Comment thread enterprise/tailnet/pgcoord_test.go
@sreya sreya requested a review from spikecurtis August 12, 2024 00:03
Comment thread enterprise/tailnet/pgcoord.go Outdated

@spikecurtis spikecurtis left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread enterprise/tailnet/pgcoord.go Outdated
Comment thread enterprise/tailnet/pgcoord.go Outdated
Comment thread enterprise/tailnet/pgcoord_internal_test.go Outdated
Comment thread tailnet/test/peer.go Outdated
Comment thread tailnet/test/peer.go
Comment thread enterprise/tailnet/pgcoord_test.go
@sreya sreya requested a review from spikecurtis August 13, 2024 15:00

@spikecurtis spikecurtis left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread enterprise/tailnet/pgcoord.go Outdated
@sreya sreya merged commit 4fc0479 into main Aug 14, 2024
@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