net/http: provide some Transport knob (bool?) to keep max 1 TLS dial in flight at once when discovering http2 support #25761
Labels
Milestone
Comments
|
If you immediately start 3 HTTP requests in parallel with no cached connections open in a fresh program, net/http doesn't know that your peer supports HTTP/2, so it does a new TCP connection & TLS handshake until it discovers the "h2" ALPN response. If that's all this is about, it sounds like working as intended. You might be interested in #6785 / #13957 (kinda dups), though, and https://go-review.googlesource.com/c/go/+/71272 which adds a limit on the number of connections per host, which includes those in dialing state. But neither of those is specifically about limiting per-host initial TLS dials for the purpose of discovering whether a host supports HTTP/2. The problem with doing that by default is that you sacrifice latency in the case where it turns out the peer did NOT support HTTP/2. Now some of the requests have to then do the TCP connection after all with the associated TCP+TLS setup round trips. |
bradfitz
changed the title
bradfitz
added
NeedsFix
|
Hi Brad,
Thanks for the quick response.
Immediately executing three concurrent requests appears to work correctly
with the http2.golang.org server as I noted in my issue. It does mux the
requests on a single connection. The client only fails to do this when
pointed at an NginX HTTP2 server.
Earlier, I had thought your point might be the problem so I tried sending a
synchronous request first to 'prime' the connection cache; and then
followed this with the three concurrent requests. It didn't change the
result for the NginX case. The cached connection was used for one of the
requests and the other two created new connections.
The correct HTTP2 semantics is to have concurrent requests for a host wait
until its support for HTTP2 is determined. Then either mux them for HTTP2
or create HTTP1.1 connections.
Since this is exactly what the client appears to be doing for
http2.golang.org, it seems the logic is already in the client to handle
this.
It appears there is some HTTP2 protocol issue occurring between the go http
client and the NginX HTTP2 reverse proxy.
Also, as I noted in my issue, the http2.golang.org case seems to be doing
multiple TLS handshakes when it should only do one. It would likely be a
TLS protocol error to actually do this so it seems this may be an httptrace
issue rather than a client issue.
On the latency vs HTTP2 correct operation tradeoffs, if the latency for the
non-HTTP2 case is an issue then providing both options is needed. The
correct HTTP2 operation should be the default.
Having HTTP2 clients fail to properly mux concurrent requests is not
something that is acceptable.
Bottom-line, there is one or more HTTP2 protocol issues here that need to
be sorted. The documentation for the go http client needs to be very clear
about when it does and does not mux concurrent requests to a host. If there
are differences in how the popular HTTP2 servers work with the go HTTP2
client, this needs to be surfaced.
…-- Mark
On Wed, Jun 6, 2018 at 4:19 PM, Brad Fitzpatrick ***@***.***> wrote:
If you immediately start 3 HTTP requests in parallel with no cached
connections open in a fresh program, net/http doesn't know that your peer
supports HTTP/2, so it does a new TCP connection & TLS handshake until it
discovers the "h2" ALPN response.
If that's all this is about, it sounds like working as intended.
You might be interested in #6785
<#6785> / #13957
<#13957> (kinda dups), though, and
https://go-review.googlesource.com/c/go/+/71272 which adds a limit on the
number of connections per host, which includes those in dialing state. But
neither of those is specifically about limiting per-host initial TLS dials
for the purpose of discovering whether a host supports HTTP/2.
The problem with doing that by default is that you sacrifice latency in
the case where it turns out the peer did NOT support HTTP/2. Now some of
the requests have to then do the TCP connection after all with the
associated TCP+TLS setup round trips.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25761 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAySh_gdOIOhdWm79IHIAIXte27iNeBJks5t6GOMgaJpZM4UdMKA>
.
|
I think you might just be getting lucky with timing.
Which part of the spec are you referring to? I don't recall anything about this.
Maybe. But I think a number of other people might disagree on their preference. Go style in general is to avoid knobs whenever possible, but this might be a case where we need one. I'm not sure what the default would be.
Curious: why? We don't expose details of HTTP/1 connection management to net/http users. Why is that optimization detail suddenly so much more import to net/http users when the underlying protocol changes? The net/http API remains unchanged. |
|
Applications that are using HTTP2 for sophisticated REST services are
relying on the correct muxing of requests. Muxing is not an optional
'optimization'.
I'm in the process of designing such a protocol. If concurrent requests get
splayed out over many connections it will severely affect performance.
This is equivalent to having go routines be lightweight contexts in one
environment and having them be heavyweight in another. It would kill
software that was designed for the lightweight case.
Am I making my case :>)
…On Wed, Jun 6, 2018 at 10:38 PM, Brad Fitzpatrick ***@***.***> wrote:
Immediately executing three concurrent requests appears to work correctly
with the http2.golang.org server as I noted in my issue.
I think you might just be getting lucky with timing.
Bottom-line, there is one or more HTTP2 protocol issues here that need to
be sorted.
Which part of the spec are you referring to? I don't recall anything about
this.
On the latency vs HTTP2 correct operation tradeoffs, if the latency for the
non-HTTP2 case is an issue then providing both options is needed. The
correct HTTP2 operation should be the default.
Maybe. But I think a number of other people might disagree on their
preference. Go style in general is to avoid knobs whenever possible, but
this might be a case where we need one. I'm not sure what the default would
be.
Having HTTP2 clients fail to properly mux concurrent requests is not
something that is acceptable.
Curious: why? We don't expose details of HTTP/1 connection management to
net/http users. Why is that optimization detail suddenly so much more
import to net/http users when the underlying protocol changes? The net/http
API remains unchanged.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25761 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAySh8yWbobHBmUebyJck0e1Z7q-8mwqks5t6LxSgaJpZM4UdMKA>
.
|
|
It actually is an optimization as far as the specs & net/http are concerned. You failed to cite where in the spec it says it's mandatory. (I don't think it is.) If your particular application depends on such a property, you'll need to arrange to do that yourself. You can use the golang.org/x/net/http2 package directly if needed. |
|
Hi Brad,
I’m well aware that the first step in responding to issues like this is to
boomerang them.
In this case, I’m not going to accept that.
Seriously, I feel it is important to the go community and requires
consideration. If you are not going to pursue it further then let me know
the person/group that I need to escalate it to.
*Background*
The proposition that, since the HTTP2 spec does not require client stream
muxing, http client can ignore it or provide a marginal implementation of
it is technically correct - but wrong.
No spec requires http client to provide connection caching; however, most
would consider a client that didn’t provide it to be broken.
The facts are this:
- Client stream mux’ing is one of the singular features of HTTP2
- It is widely touted as a significant performance improvement for
internet layer communication
- A large part of Chapter 5 is devoted to specifying its semantics
- Implementing it does add some complexity to HTTP2 client packages -
clearly the internet community feels its performance improvement is worth
its cost
Just like the go community expects http client to provide an efficient
connection cache, it expects it to efficiently implement client stream
mux’ing.
Currently, http client doc says nothing about how HTTP2 client streams to a
host are mux’ed - this needs to be fixed.
HTTP2 efficiency is a core requirement of all platforms - I expect go to
excel in this area and I know that the go team does also.
Telling users that expect client stream mux’ing to write their own client
with golang.org/x/net/http2 is like telling them to write their own
connection-caching client. It shoots below the expectations of the go
community.
The go team needs to take this issue seriously.
Since client stream mux’ing does appear - in some cases - to work,
resolving this issue might be as simple as properly documenting how to use
its existing functionality.
— Mark
…On Thu, Jun 7, 2018 at 8:04 AM, Brad Fitzpatrick ***@***.***> wrote:
It actually is an optimization as far as the specs & net/http are
concerned. You failed to cite where in the spec it says it's mandatory. (I
don't think it is.)
If your particular application depends on such a property, you'll need to
arrange to do that yourself. You can use the golang.org/x/net/http2
package directly if needed.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25761 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAySh4hzb3XexK5_3_-1ptc5AUBxFbkRks5t6UDjgaJpZM4UdMKA>
.
|
|
Hi Brad,
Do you have any response to my prior email?
…-- Mark
On Thu, Jun 7, 2018 at 5:45 PM, Mark Hapner ***@***.***> wrote:
Hi Brad,
I’m well aware that the first step in responding to issues like this is to
boomerang them.
In this case, I’m not going to accept that.
Seriously, I feel it is important to the go community and requires
consideration. If you are not going to pursue it further then let me know
the person/group that I need to escalate it to.
*Background*
The proposition that, since the HTTP2 spec does not require client stream
muxing, http client can ignore it or provide a marginal implementation of
it is technically correct - but wrong.
No spec requires http client to provide connection caching; however, most
would consider a client that didn’t provide it to be broken.
The facts are this:
- Client stream mux’ing is one of the singular features of HTTP2
- It is widely touted as a significant performance improvement for
internet layer communication
- A large part of Chapter 5 is devoted to specifying its semantics
- Implementing it does add some complexity to HTTP2 client packages -
clearly the internet community feels its performance improvement is worth
its cost
Just like the go community expects http client to provide an efficient
connection cache, it expects it to efficiently implement client stream
mux’ing.
Currently, http client doc says nothing about how HTTP2 client streams to
a host are mux’ed - this needs to be fixed.
HTTP2 efficiency is a core requirement of all platforms - I expect go to
excel in this area and I know that the go team does also.
Telling users that expect client stream mux’ing to write their own client
with golang.org/x/net/http2 is like telling them to write their own
connection-caching client. It shoots below the expectations of the go
community.
The go team needs to take this issue seriously.
Since client stream mux’ing does appear - in some cases - to work,
resolving this issue might be as simple as properly documenting how to use
its existing functionality.
— Mark
On Thu, Jun 7, 2018 at 8:04 AM, Brad Fitzpatrick ***@***.***
> wrote:
> It actually is an optimization as far as the specs & net/http are
> concerned. You failed to cite where in the spec it says it's mandatory. (I
> don't think it is.)
>
> If your particular application depends on such a property, you'll need to
> arrange to do that yourself. You can use the golang.org/x/net/http2
> package directly if needed.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#25761 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAySh4hzb3XexK5_3_-1ptc5AUBxFbkRks5t6UDjgaJpZM4UdMKA>
> .
>
|
|
We'll add some way to control this one way or another, but not right now. Go 1.11 is in freeze at the moment (https://github.com/golang/go/wiki/Go-Release-Cycle) so the current priority is fixing Go 1.11 bugs. |
|
OK, that's great. As long as it gets in the pipeline I'm happy.
The Go team has done an excellent job top-to-bottom.
…On Tue, Jun 12, 2018 at 9:04 AM Brad Fitzpatrick ***@***.***> wrote:
We'll add some way to control this one way or another, but not right now.
Go 1.11 is in freeze at the moment (
https://github.com/golang/go/wiki/Go-Release-Cycle) so the current
priority is fixing Go 1.11 bugs.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#25761 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAySh45YcoQeVl0hJsKZr_GDJ5CDcS7eks5t7-aXgaJpZM4UdMKA>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version)?go version go1.10.2 darwin/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env)?Container Linux by CoreOS stable (1745.5.0)
What did you do?
Attached NginX client replicates this issue. It requires some minimal reconfiguration for a test HTTP2 server. The client<->Nginx hop is HTTP2 and the NginX<->Service hop is HTTP1.1. This is a typical config. I've verified NginX is exposing an HTTP2 virtual server endpoint with the h2c utility.
Any HTTP2 client for an NginX HTTP2 reverse proxy should replicated this behavior.
NginX sets http2_max_concurrent_streams to 128 by default so it should accept the 3 concurrent streams in this test.
I've also included an equivalent HTTP2 client for http2.golang.org. httptrace reports this client does one TLS handshake per request; however, all three requests use the same connection.
So, for some reason not visible via httptrace, the client is multiplexing requests to a Go HTTP2 server; but, is not multiplexing requests to an NginX HTTP2 reverse proxy.
What did you expect to see?
I expected to see the go http client multiplex these concurrent HTTP2 requests on a single HTTP2 connection to the NginX HTTP2 reverse proxy.
I expected that only one TLS handshake would be done.
What did you see instead?
Instead of multiplexing these requests on one NginX reverse proxy connection, the client sends each request on a separate connection.
The duplicate TLS handshakes reported by httptrace for http2.golang.org don't make sense if all requests are using the same connection.
nginx.conf.txt
simSwitch.go.txt
http2client.go.txt
simSwitch-httptrace.txt
http2client-httptrace.txt
The text was updated successfully, but these errors were encountered: