haproxy-ingress icon indicating copy to clipboard operation
haproxy-ingress copied to clipboard

Allow setting HTTP2 Initial Window Size and set larger default

Open hensur opened this issue 3 years ago • 2 comments

Uploads using haproxy-ingress to backend servers in k8s were weirdly slow, while uploads to other endpoints to services on the same server could utilize my full upload speed.

While trying to find the root cause for this, I noticed that latency had a huge impact on the transfer speed.

Then I found this haproxy issue: https://github.com/haproxy/haproxy/issues/293

Adding the described option to the global section of haproxy-ingress fixed my upload bandwidth issues.

I think increasing the initial window size by default should be beneficial to most haproxy-ingress use cases, therefore I created this PR.

The default value will at most allow for 25Mbit/s uploads at 25ms RTT (which is what I experienced):

>>> 65535 (bytes) / 0.025 (s)
2621400.0
>>> 2621400.0 (bytes) / 1024
2559.9609375

The new default of 268435456 bytes is also used by envoy: https://github.com/envoyproxy/envoy/blame/main/api/envoy/api/v2/core/protocol.proto#L179 nginx seems to just set the maximum window size once a transfer starts: https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.c#L309

Using 268435456 bytes should fix this issue for most if not all use-cases, theoretically allowing a 10Gbit Link to be saturated at up to ~214ms RTT:

>>> 268435456 (bytes) / 1250000000 (bytes/s)
0.2147483648

hensur avatar Dec 28 '21 13:12 hensur

@jcmoraisjr I fixed the missing gofmt

hensur avatar Dec 29 '21 14:12 hensur

First of all thanks for sharing this solution and sorry about taking so long to provide a feedback.

I usually leave global configurations to be made in the custom snippet area, however some of them have a special value and worth to be provided and documented. Otoh I've some update suggestions that tries to preserve historical behavior and specially preserve default haproxy tunings - I tend to prefer tuning tips documentation instead of changing default behavior on behalf of the user.

Some considerations below:

  • Create the doc itself and the entry point in the config key table in alphabetical order;
  • Declare and populate the config key value in alphabetical order as well;
  • Use h2 instead of http2 in the config key, which is shorter and a well known abbreviation of the protocol;
  • The default value is low for a reason, so I'd suggest to remove the default initialization in order to preserve backward compatibility. Let's see later if it worths to change it. That however doesn't stop us from adding a new topic in the tuning tips;
  • This configuration can be easily made in v0.13 using a custom configuration, so I'd suggest to change the since doc to v0.14, whose tag saga will be started in a week or so.

jcmoraisjr avatar Jan 16 '22 20:01 jcmoraisjr