Cp 351d144fa1fc0bd934e2408202be0c29f25e35a0 #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…EAMS And add the http2.Transport.StrictMaxConcurrentStreams bool knob to behavior being reverted. In CL 53250 for golang/go#13774 (for Go 1.10) we changed the HTTP/2 Transport's policy such that a server's advertisement of a MAX_CONCURRENT_STREAMS value meant that it was a maximum for the entire process, instead of just a single connection. We thought that was a reasonable interpretation of the spec and provided nice safety against slamming a server from a bunch of goroutines doing concurrent requests, but it's been largely unpopular (see golang/go#27044). It's also different behavior from HTTP/1 and because you're usually not sure which protocol version you're going to get, you need to limit your outbound HTTP requests anyway in case you're hitting an HTTP/1 server. And nowadays we have the Go 1.11 Transport.MaxConnsPerHost knob too (CL 71272 for golang/go#13957). It doesn't yet work for HTTP/2, but it will in either Go 1.12 or Go 1.13 (golang/go#27753) After this is bundled into net/http's, the default HTTP client will have this knob set false, restoring the old Go 1.9 behavior where new TCP connections are created as necessary. Users wanting the strict behavior and import golang.org/x/net/http2 themselves and make a Transport with StrictMaxConcurrentStreams set to true. Or they can set Transport.MaxConnsPerHost, once that works for HTTP/2. Updates golang/go#27044 (fixes after bundle into std) Change-Id: I4efdad7698feaf674ee8e01032d2dfa5c2f8a3a8 Reviewed-on: https://go-review.googlesource.com/c/151857 Reviewed-by: Andrew Bonventre <andybons@golang.org>
zachmccormick
approved these changes
Jan 7, 2019
jonhyman
approved these changes
Jan 7, 2019
|
yeah the code isn't written entirely intuitively but I think this should be it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry pick golang@351d144 before golang 1.12 is released. This returns the http2 transport to golang 1.9's behavior and is slated for general release to the best of my understanding. See the original commit comment for more context.