-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
net/http: HTTP/2 configuration API #67813
Comments
This proposal has been added to the active column of the proposals project |
Why are the types uncommon (for Go APIs) sizes like uint32? Can they all be int? |
The proposal reuses the types from the x/net/http2 Server and Transport. HTTP/2 settings are defined as 32-bit unsigned integers; I don't know for sure, but I'd guess that's the reason for the uint32s. I don't see a problem with defining these as ints, although the extra range isn't going to be used. 24 more bytes in the Server and Transport structs isn't going to be noticeable, given that you're not expected to have a lot of either. |
It seems like now that this has outgrown being an HTTP/2 guts package, we should probably use Go types (int or else int64 if it really matters) instead of wire types. Otherwise this seems fine. |
Have all remaining concerns about this proposal been addressed? The proposal is #67813 (comment), except using ints instead of uint32 etc. |
Based on the discussion above, this proposal seems like a likely accept. The proposal is #67813 (comment), except using ints instead of uint32 etc. |
No change in consensus, so accepted. 🎉 The proposal is #67813 (comment), except using ints instead of uint32 etc. |
Change https://go.dev/cl/602175 mentions this issue: |
Change https://go.dev/cl/607255 mentions this issue: |
A minor change to rename two fields:
These fields originate as |
Add a field to Server and Transport containing HTTP/2 configuration parameters. This field will have no effect until golang.org/x/net/http2 is updated to make use of it, and h2_bundle.go is updated with the new http2 package. For #67813 Change-Id: I81d7f8e9ddea78f9666383983aec43e3884c13ed Reviewed-on: https://go-review.googlesource.com/c/go/+/602175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
For golang/go#67813 Change-Id: I6b7f857d6ed250ba8b09649730980a91b3e8d7e9 Reviewed-on: https://go-review.googlesource.com/c/net/+/607255 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
Change https://go.dev/cl/615875 mentions this issue: |
Change https://go.dev/cl/615895 mentions this issue: |
This issue is part of a project to move
x/net/http2
intostd
: #67810Configuring HTTP/2-specific protocol options currently requires users to import the golang.org/x/net/http2 package and call http2.ConfigureServer or http2.ConfigureTransports.
Configuring options in this fashion has the side effect of replacing the bundled HTTP/2 implementation in
net/http
with the one ingolang.org/x/net/http2
.I propose adding HTTP/2 configuration options to
net/http
, permitting HTTP/2 servers and clients to be configured without importing an external package and separating configuration from version selection.Many of the HTTP/2 settings are identical for server and client connections. For example, the
MaxDecoderHeaderTableSize
field of http2.Server and http2.Transport sets the SETTINGS_HEADER_TABLE_SIZE setting sent to the peer. We will unify these settings into a single configuration struct, and add this configuration tohttp.Server
andhttp.Transport
.The
SendPingTImeout
andPingTimeout
fields assume #67812 is accepted, and theWriteByteTimeout
field assumes #67811 is accepted.The http2.Transport.StrictMaxConcurrentStreams field controls whether a new connection should be opened to a server if an existing connection has exceeded its stream limit. For example, if an HTTP/2 server advertises a stream limit of 100 concurrent streams, then the 101st concurrent stream opened by the client will block waiting for an existing stream to complete when
StrictMaxConcurrentStreams
is true, or create a new connection when it is false. There is no equivalent to this setting for HTTP/1 connections, which only support a single concurrent request per connection. We will add this setting tohttp.Transport
, since it could be used to configure HTTP/3 connections (if and when we support HTTP/3):The text was updated successfully, but these errors were encountered: