-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
Proposal: change the default value of initial_max_send_streams
to 100
#731
Comments
This is a great write-up, thank you! I agree it makes sense to adjust the default. I only want to pause to make sure it's the right place to adjust the default. It's worth calling out that for workloads where the server will NOT place a limit, changing this default here could be a negative. This would be places where both peers are trusted. My mind goes to things like linkerd2 proxies. (cc @hawkw @olix0r) The proper solution could actually be that those use cases should explicitly set a high initial value, instead of depending on the default, which perhaps should aim to protect the most common cases. But I also vaguely feel like so far we've put those defaults in hyper or reqwest, and left |
It makes sense that changing the default value could have a negative impact in a rare situation. So probably what we would like to do is:
I also feel that some changes still need to be made to h2, namely:
Do these sound reasonable to you? If they do, I'd be happy to work on these :) |
+1. Having arbitrary defaults everywhere is also a nightmare to handle, so it's good that we're avoiding that.
This sounds like it should be exposed in |
Thanks for your opinion @dswij.
What do you mean by this? If what you mean is that the users of
|
Yes :) I'm completely on board with the first point. Just saying that regardless of changing the default value, this option should be configurable in |
So I separated #732 into a couple of PRs. Now the part of setting a reasonable default value is moved to hyper. Here are the separated PRs: |
Thanks a lot for doing that, your contributions have been wonderful to work with |
Issue
Currently, the number of streams opened by the client before it receives the initial
SETTINGS
frame from the server is set tousize::MAX
by default.h2/src/client.rs
Line 645 in 122091a
This is valid according HTTP/2 spec (in fact the spec doesn't mention any explicit limit here). However, opening unlimited number of streams can lead to majority of them being rejected by the server with
REFUSED_STREAM
, if the server'sSETTINGS_MAX_CONCURRENT_STREAMS
is set to any lower value. This is simply not ideal and waste of resources.Proposal
To better handle this issue, I propose that we start with reasonably low number of streams by default, rather than the maximum possible number. The initial number is only used until the client receives the initial
SETTINGS
frame, so it should be much more preferable to start with a low number in order to reduce the risk ofREFUSED_STREAM
errors even if it might end up with lower utilization for a very short time.What is the reasonable default number? The good number should be as low as what most servers would accept. In fact, the spec recommends that the value should be no smaller than 100. To back up this recommendation, I've done a quick research on the values of
MAX_CONCURRENT_STREAMS
used by several popular servers. The result is as follows.MAX_CONCURRENT_STREAMS
So if the client starts with 100 streams at maximum, all of these servers will accept them and the client will not receive any
REFUSED_STREAM
errors. Therefore we can conclude that 100 is the reasonable default number of streams to start with.Take a quick look at other HTTP/2 implementations
To further support the proposal, let's have a look at how other HTTP/2 client implementations are doing. In short, all of these three implementations adopt starting with the reasonably small number of streams.
nghttp2
nghttp2's default initial maximum number of concurrent streams is 100.
https://github.com/nghttp2/nghttp2/blob/837f0c67c7139aedfb0c31d252634c823bfa75ba/lib/includes/nghttp2/nghttp2.h#L2537-L2554
Go x/net/http2
Go also uses 100 as the the initial number of maximum concurrent strams.
https://cs.opensource.google/go/x/net/+/689bbc7005f6bbf9fac1a8333bf03436fa4b4b2a:http2/transport.go;l=56
One notable difference from nghttp2 is that Go will set 1,000 as the
MAX_CONCURRENT_STREAMS
if the server doesn't provide one, while nghttp2 sets inifinity in this case.Python httpx
httpx starts with just 1.
https://github.com/encode/httpcore/blob/2fcd062df71555cc7de55774c6dc137551eb8692/httpcore/_async/http2.py#L117-L119
The text was updated successfully, but these errors were encountered: