Go: x/net/http2: block when Transport hits max concurrent streams

[bradfitz]

Currently if the http2.Transport hits MAX_CONCURRENT_STREAMS for a host, it just makes a new TCP connection and creates the stream on the new connection instead.

We should probably just block the application and chill out for a bit, waiting for the host to be happy again.

We're probably respecting the letter of the spec more than the spirit of the spec.

/cc @bmizerany

[bradfitz]

Relevant code:

func (cc *ClientConn) canTakeNewRequestLocked() bool {
        return cc.goAway == nil &&
                int64(len(cc.streams)+1) < int64(cc.maxConcurrentStreams) &&
                cc.nextStreamID < 2147483647
}

[quentinmit]

@bradfitz @bmizerany Whose role is it to decide what the right behavior is here?

(My 2¢ is that we shouldn't build in any hidden rate limiting to the client; it's up to the user's code or the remote server to rate limit or block new connections.)

[bradfitz]

Usually I end up making decisions about HTTP, but often after various people express their opinions. Mine is that we should be polite by default and respect specs, unless the user configures otherwise.

Related bug: #13957

[nathany]

It sounds like this might help for a scenario I'm working on. My client library receives an initial Settings frame with SettingMaxConcurrentStreams = 1 and once the connection is authenticated, receives SettingMaxConcurrentStreams = 500.

logged via: https://github.com/golang/net/blob/master/http2/transport.go#L1735

My initial thought was to gain access to the settings data via some API (possibly like httptrace?). Then I could manage the number of goroutines sending requests.

Blocking all the requests currently in flight could actually work better though. I'd be curious to try it out.

[bradfitz]

I'm going to punt this to Go 1.9. It's relatively low priority compared to other open stuff, and not really a problem in practice. Also, http1 already has the as-many-conn-as-requested issue, and I'd like to make their two behaviors match, which makes this more work.

[bradfitz]

/cc @tombergan

[gopherbot]

Change https://golang.org/cl/53250 mentions this issue: http2: block RoundTrip when the Transport hits MaxConcurrentStreams

[gopherbot]

Change https://golang.org/cl/54052 mentions this issue: net/http: update bundled http2