Skip to content

refactor: allow hot swapping the config#192

Merged
sbruens merged 64 commits into
masterfrom
sbruens/shared-listeners
Aug 28, 2024
Merged

refactor: allow hot swapping the config#192
sbruens merged 64 commits into
masterfrom
sbruens/shared-listeners

Conversation

@sbruens

@sbruens sbruens commented Jul 11, 2024

Copy link
Copy Markdown

Instead of keeping track of a diff of ports to keep open, we introduce reusable listeners that can be hot-swapped by remaining open until the last user of the listener closes it.

This pulls out the listener changes from #182, which is focused on adding a new config format.

sbruens added 3 commits July 11, 2024 16:46
We introduce shared listeners that allow us to keep an old config
running while we set up a new config. This is done by keeping track of
the usage of the listeners and only closing them when the last user is
done with the shared listener.
@sbruens sbruens marked this pull request as ready for review July 11, 2024 20:52
@sbruens sbruens requested a review from a team as a code owner July 11, 2024 20:52
@sbruens sbruens requested a review from fortuna July 11, 2024 20:52
@sbruens sbruens changed the title refactor: add a listener manager to allow re-use of listeners during config changes refactor: allow hot swapping the config Jul 11, 2024

@fortuna fortuna left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to readjust some things. Happy to brainstorm more.

Comment thread service/udp.go Outdated
Comment thread internal/integration_test/integration_test.go Outdated
Comment thread cmd/outline-ss-server/metrics.go Outdated
Comment thread cmd/outline-ss-server/listeners.go Outdated
Comment thread cmd/outline-ss-server/listeners.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread service/cipher_list.go
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
@sbruens sbruens force-pushed the sbruens/shared-listeners branch from 707bc35 to 80e5d49 Compare August 7, 2024 14:49
@sbruens sbruens requested a review from fortuna August 7, 2024 15:54
@sbruens

sbruens commented Aug 7, 2024

Copy link
Copy Markdown
Author

I still need to look at creating a channel for UDP like we talked about.

Added a read and close channel to the virtual packet connection.

@sbruens sbruens force-pushed the sbruens/shared-listeners branch from 18e407a to 4730d74 Compare August 7, 2024 20:54

@fortuna fortuna left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new structure looks nicer. The main things now is the new virtual packet conn, which needs a redesign, and the RefCount can probably go away.

Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread cmd/outline-ss-server/main.go Outdated
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go
Comment thread service/listeners.go Outdated
@sbruens sbruens force-pushed the sbruens/shared-listeners branch from 851ac28 to 8f9f1ea Compare August 9, 2024 19:42

@sbruens sbruens left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ready for another look, @fortuna please take a look specifically at the packet conn handling. I ended up using a read and a response channel. I think this resolves the issue you pointed out, but I'm curious to hear what you think.

Comment thread service/listeners.go Outdated
Comment thread service/listeners.go Outdated
@sbruens sbruens requested a review from fortuna August 9, 2024 20:01

@fortuna fortuna left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just some small clean ups.

Also, is there a benchmark that exercises that code that we can check to make sure it's fine?

Comment thread service/listeners.go Outdated
Comment thread service/listeners.go
Comment thread service/listeners.go Outdated
Comment thread service/listeners.go
@sbruens sbruens requested a review from fortuna August 14, 2024 22:36
@sbruens sbruens merged commit 72e9238 into master Aug 28, 2024
@sbruens sbruens deleted the sbruens/shared-listeners branch August 28, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants