Change defaults

[aledbf]

What this PR does / why we need it:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Which issue/s this PR fixes

How Has This Been Tested?

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added tests to cover my changes.
• All new and existing tests passed.

[aledbf]

[aledbf]

/assign @ElvinEfendi

[ElvinEfendi]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, ElvinEfendi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ElvinEfendi,aledbf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mi1os]

I would argue that this is a breaking change and not a bug fix as it is not fixing anything, merely just optimizing things. We have just got heavily burned by this change (in terms of $$) and I am sure we are neither first nor last ones. Changing defaults is always tricky as people learn to rely on them and I personally like the approach of Linux kernel when it is impossible to change user facing defaults/api even if they do not make sense.

Having said that would you consider reverting this change?

[aledbf]

Having said that would you consider reverting this change?

No. The change of the gzip default was also triggering the HPA due to CPU utilization on high RPS. Either way, I don't see a one size fits all.

We have just got heavily burned by this change (in terms of $$) and I am sure we are neither first nor last ones.

Can you expand on that?

Edit: the change also aligns with nginx defaults http://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip

[mi1os]

It is not that important what defaults nginx choose as technically speaking nginx-ingress is different product. I agree that one size does not fit all but you have changed default which has significant monetary impact. Traffic in clouds is quite expensive commodity and if suddenly gzip gets disabled then the egress traffic to world gets multiplied (it was ~3x in our case).

EDIT: perhaps one missing piece is that we are running behind cloudflare who by default tries to use compression with origins servers if at all possible. granted the impact would be much lower if clients were connecting directly to nginx-ingress as they would likely not use compression

[kolorful]

Hi @aledbf, would you provide some detail on why the default value of upstream-keepalive-connections is changed from 32 to 320.

For some legacy reason, we have an HAProxy in front of our nginx ingress, when upstream keepalive connection value is high (we tried, 32, 320, 640 and 960) the time for haproxy to establish a TCP connection with nginx is increased drastically (32 vs 320 connections ==> less than 1ms vs 50ms to 100ms latency). I'm a bit puzzled on why the increase of this field would result in worse latency.

Thanks!