Skip to content
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

refactor: allow hot swapping the config #192

Merged
merged 64 commits into from
Aug 28, 2024
Merged

Conversation

sbruens
Copy link

@sbruens sbruens commented Jul 11, 2024

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
Copy link

@fortuna fortuna left a comment

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.

service/udp.go Outdated Show resolved Hide resolved
internal/integration_test/integration_test.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/metrics.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/listeners.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/listeners.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
cmd/outline-ss-server/main.go Outdated Show resolved Hide resolved
service/cipher_list.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
@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
Copy link
Author

sbruens commented Aug 7, 2024

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
Copy link

@fortuna fortuna left a comment

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.

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

@sbruens sbruens left a comment

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.

service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
@sbruens sbruens requested a review from fortuna August 9, 2024 20:01
Copy link

@fortuna fortuna left a comment

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?

service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Show resolved Hide resolved
service/listeners.go Outdated Show resolved Hide resolved
service/listeners.go Show resolved Hide resolved
@sbruens sbruens requested a review from fortuna August 14, 2024 22:36
@sbruens sbruens merged commit 72e9238 into master Aug 28, 2024
5 checks passed
@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