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

Increasing initial-window beyond 128K results in no window-updates #162

Closed
steve-misky opened this issue Dec 1, 2022 · 7 comments
Closed

Comments

@steve-misky
Copy link

steve-misky commented Dec 1, 2022

I noticed the unit-tests reduced the window-size, but in our application we are trying to increase the size
and observed the downloads stop, because no window-update frames get sent.

If we use 128000 as the window-size, the WU frames DO get sent for 65k.
But, if we increase to 256000, no WU frames get sent and download halts.

It looks like, there is a check that doesn't account for larger buffers where 50% is larger than 65k.

Performance is improved with larger buffer sizes, and 65K is much too small..

I've confirmed the same behavior with Versions: 0.10.1 and 0.11.0

With size set to 128K, the WU messages are all 65k,
once 65K drops below 50% of the window size, the logic fails to send WU frames.

STREAM 0 SEND MGMT {:type=>:settings, :stream=>0, :payload=>[[:settings_max_concurrent_streams, 100], [:settings_initial_window_size, 128000]]}

STREAM 1 SEND WindowUpdate {:type=>:window_update, :increment=>65535, :stream=>1}
STREAM 0 SEND WindowUpdate {:type=>:window_update, :stream=>0, :increment=>65535}

STREAM 1 SEND WindowUpdate {:type=>:window_update, :increment=>65535, :stream=>1}
STREAM 0 SEND WindowUpdate {:type=>:window_update, :stream=>0, :increment=>65535}

STREAM 1 SEND WindowUpdate {:type=>:window_update, :increment=>65535, :stream=>1}
STREAM 0 SEND WindowUpdate {:type=>:window_update, :stream=>0, :increment=>65535}

@steve-misky steve-misky changed the title Increasing initial-window causes no window-update frames to be sent Increasing initial-window beyond 128K results in no window-updates Dec 1, 2022
@HoneyryderChuck
Copy link
Collaborator

Hi @steve-misky ,

Feel free to try reproducing it against http-2-next, which is a fork of this gem with a few enhancements, mostly compliance.

@igrigorik
Copy link
Owner

@HoneyryderChuck anything we can merge upstream? :)

@HoneyryderChuck
Copy link
Collaborator

@igrigorik I started the fork after lack of success in getting this pull request merged. I tried :)

@igrigorik
Copy link
Owner

Apologies, did not follow through. Would you be willing to cherry-pick and land what you outlined in that thread? You have the commit bit :)

@HoneyryderChuck
Copy link
Collaborator

Not sure if plain cherry-picking will be possible at this point, as repos diverged enough for this not to be straightforward. If you see the full list of changes since the fork, I've also changed rubocop directives, added RBS signatures, among other things that you may be wary importing here.

I've kept the license intact, and am not opposed to anyone "importing" the improvements here, it's just that it's not trivial, and I lack the time to do it myselff.

@igrigorik
Copy link
Owner

Hmm, bummer. Would love to converge back. If anyone is interested in picking this up...

@HoneyryderChuck
Copy link
Collaborator

I believe this can be closed. @steve-misky feel free to report back if this still happens with 1.0.0, I'll open it if that's the case.

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

No branches or pull requests

3 participants