Merge pull request #67902 from liggitt/http2-buffers

Automatic merge from submit-queue (batch tested with PRs 67694, 64973, 67902). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Size http2 buffers to allow concurrent streams

http/2 requests from a given client multiplex over a single connection via streams, chopped up into frames.

The amount of data the client is allowed to send for a given stream and for the overall connection before acknowledgement is determined by the server's MaxUploadBufferPerStream and MaxUploadBufferPerConnection settings respectively, both defaulting to 1MB.

The number of concurrent streams a client is allowed to send over a single connection is determined by the server's MaxConcurrentStreams setting, defaulting to 250.

We observed a starvation issue with the kube aggregator's proxy client if handling of a POST through the aggregator to a backend server exceeded the 1MB buffer size AND the backend server required a second POST request through the aggregator to be handled before it could drain the first request's body.

Logically, if concurrent streams are allowed in a single connection, the connection buffer should be MaxUploadBufferPerStream*MaxConcurrentStreams to allow individual streams to make progress even when one stream is blocked.

This PR shrinks the `MaxUploadBufferPerStream` size to 256kb (which is still large enough to allow all the resources we saw in our test clusters to be sent in a single frame), and grows the MaxUploadBufferPerConnection to accomodate concurrent streams.

I'm also opening a golang issue, [reproducer](https://gist.github.com/liggitt/00239c99b4c148ac1b23e57f86b3af93), and fix for the defaults for this

```release-note
adjusted http/2 buffer sizes for apiservers to prevent starvation issues between concurrent streams
```
This commit is contained in:
Kubernetes Submit Queue 2018-08-28 07:21:21 -07:00 committed by GitHub
commit 9edf196c01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -87,10 +87,30 @@ func (s *SecureServingInfo) Serve(handler http.Handler, shutdownTimeout time.Dur
secureServer.TLSConfig.ClientCAs = s.ClientCA
}
// At least 99% of serialized resources in surveyed clusters were smaller than 256kb.
// This should be big enough to accommodate most API POST requests in a single frame,
// and small enough to allow a per connection buffer of this size multiplied by `MaxConcurrentStreams`.
const resourceBody99Percentile = 256 * 1024
http2Options := &http2.Server{}
// shrink the per-stream buffer and max framesize from the 1MB default while still accommodating most API POST requests in a single frame
http2Options.MaxUploadBufferPerStream = resourceBody99Percentile
http2Options.MaxReadFrameSize = resourceBody99Percentile
// use the overridden concurrent streams setting or make the default of 250 explicit so we can size MaxUploadBufferPerConnection appropriately
if s.HTTP2MaxStreamsPerConnection > 0 {
http2.ConfigureServer(secureServer, &http2.Server{
MaxConcurrentStreams: uint32(s.HTTP2MaxStreamsPerConnection),
})
http2Options.MaxConcurrentStreams = uint32(s.HTTP2MaxStreamsPerConnection)
} else {
http2Options.MaxConcurrentStreams = 250
}
// increase the connection buffer size from the 1MB default to handle the specified number of concurrent streams
http2Options.MaxUploadBufferPerConnection = http2Options.MaxUploadBufferPerStream * int32(http2Options.MaxConcurrentStreams)
// apply settings to the server
if err := http2.ConfigureServer(secureServer, http2Options); err != nil {
return fmt.Errorf("error configuring http2: %v", err)
}
glog.Infof("Serving securely on %s", secureServer.Addr)