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

x/ref/runtime/internal/flow: delete flow control counters when a flow is closed #287

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

cosnicolaou
Copy link
Contributor

@cosnicolaou cosnicolaou commented Aug 24, 2022

This PR addresses #285 by deleting the per-flow flow control counters maintained by the underlying connection when a flow is closed.

@cosnicolaou cosnicolaou merged commit f2f0f5e into main Aug 24, 2022
cosnicolaou added a commit that referenced this pull request Aug 26, 2022
PR #287 deleted the toRelease/borrowed entries for a flow when it is closed. This greatly reduced the size of the Release messages for the common case. However, those counters are required when borrowed is true past the flow being closed so that the dialer can recover the borrowed tokens to its shared pool of tokens. In cases where a great number of short lived connections are created the resulting Release message can be larger than the default buffer size allowed and hence will require fragmentation. When routed through a proxy these large messages can lead to deadlock since the proxy's encapsulate flow will not forward a message that has to be fragmented if it does not have enough flow control counters for the entire message. This PR explicitly fragments the Release message to get around this limitation.

A better fix is to collect all of the borrowed tokens into a single 'special flow' in the Release message and for the dialer to add these tokens to the shared pool directly rather than iterating over all of the per-flow borrowed counters.
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.

1 participant