Merge pull request kubernetes#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
```

Author: Kubernetes Submit Queue

staging/src/k8s.io/apiserver/pkg/server/secure_serving.go

@@ -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)