Skip to content

http proxy: allow multiple listeners#949

Merged
mmatczuk merged 2 commits intomainfrom
mmt/run_with_listener
Nov 8, 2024
Merged

http proxy: allow multiple listeners#949
mmatczuk merged 2 commits intomainfrom
mmt/run_with_listener

Conversation

@mmatczuk
Copy link
Contributor

@mmatczuk mmatczuk commented Nov 7, 2024

Add HTTPProxyConfig.ExtraListeners and refactor code to work with multiple listeners.

@mmatczuk mmatczuk requested a review from Choraden as a code owner November 7, 2024 13:25
@mmatczuk mmatczuk force-pushed the mmt/run_with_listener branch from 0c731d2 to 7a795db Compare November 7, 2024 13:26
Comment on lines +533 to +544
g.Go(func() error {
return srv.Serve(l)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

^C2024/11/07 14:41:01.894475 [proxy] [INFO] closing down proxy
2024/11/07 14:41:01.894515 [proxy] [INFO] waiting for connections to close
2024/11/07 14:41:01.894522 [proxy] [INFO] all connections closed
2024/11/07 14:41:01.894584 [ERROR] fatal error exiting: http: Server closed
exit status 1

Closing a proxy in a handler mode results in a fatal error.

@Choraden
Copy link
Contributor

Choraden commented Nov 7, 2024

LGTM, personally I would go with a wrapper over a list of listeners that returns any accepted connection from the underlying slice, but this is even better!

Left a proposition on adding "named" listeners.

@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

MultiListener is complicated its main disadvantage is that you accept a connection and then send it to worker. It's not guaranteed there is a worker available. It could be done but the standard interfaces are not well suited for that.

@mmatczuk mmatczuk force-pushed the mmt/run_with_listener branch from 7a795db to d3d3289 Compare November 8, 2024 11:52
@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

@Choraden PTAL

@mmatczuk mmatczuk force-pushed the mmt/run_with_listener branch from d3d3289 to c967c5a Compare November 8, 2024 12:03
Add HTTPProxyConfig.ExtraListeners and refactor code to work with multiple listeners.
@mmatczuk mmatczuk force-pushed the mmt/run_with_listener branch from c967c5a to e0da9aa Compare November 8, 2024 12:04
@mmatczuk
Copy link
Contributor Author

mmatczuk commented Nov 8, 2024

Squashed some commits and rebased.

@mmatczuk mmatczuk merged commit b8ed0f2 into main Nov 8, 2024
@mmatczuk mmatczuk deleted the mmt/run_with_listener branch November 8, 2024 12:29
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