-
Notifications
You must be signed in to change notification settings - Fork 63
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
Comments
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. |
@HoneyryderChuck anything we can merge upstream? :) |
@igrigorik I started the fork after lack of success in getting this pull request merged. I tried :) |
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 :) |
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. |
Hmm, bummer. Would love to converge back. If anyone is interested in picking this up... |
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. |
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}
The text was updated successfully, but these errors were encountered: