Skip to content

Close downstream connections when upstream connection closes.#220

Merged
markusgust merged 4 commits intomainfrom
close-downstream
Sep 12, 2025
Merged

Close downstream connections when upstream connection closes.#220
markusgust merged 4 commits intomainfrom
close-downstream

Conversation

@markusgust
Copy link
Copy Markdown
Contributor

Currently when upstream connection closes we close the channels on the downstream connections, which is a change in how v1.0.0 used to work. This PR instead closes downstream connections when upstream closes, which makes it easier for clients to reconnect on closed connections.

@markusgust markusgust requested a review from a team as a code owner September 4, 2025 08:41
Copy link
Copy Markdown
Member

@snichme snichme left a comment

Choose a reason for hiding this comment

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

What will happen if you have many connections to the proxy which will generate one connection but many channels to the broker and if one connections sends a close frame, wont that close the whole connection between proxy and broker affecting all connections to the proxy?

@markusgust
Copy link
Copy Markdown
Contributor Author

if a client send a close frame to the proxy, then this code shouldn't be reached I believe, as it'll instead go here:

def read_loop(channel_pool, socket = @socket) # ameba:disable Metrics/CyclomaticComplexity
and close all upstream channels related to the connection.

This is only if the upstream connection terminates, in which case all clients connected over the upstream connections will receive connection_close instead of channel_close.

@markusgust
Copy link
Copy Markdown
Contributor Author

markusgust commented Sep 8, 2025

only time I do think this might be destructive is if a client make a connection to a proxy, and this same connection creates channels on different upstream connections (different brokers). But I don't know if that's a possible scenario?

Copy link
Copy Markdown
Member

@snichme snichme left a comment

Choose a reason for hiding this comment

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

Thank you for the clarification. I initally thought this was for the upstream connections (based on the filename) but after your explanation it cleared things up.
Makes sense to close the client connection if the broker closes the connection and I agree that the scenario where one connection opens different channels to different brokers shouldn't be possible.

I've tried it locally and works as expected.

@markusgust markusgust merged commit d967c27 into main Sep 12, 2025
12 checks passed
@markusgust markusgust deleted the close-downstream branch September 12, 2025 02:54
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