-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Allow customisation of the nginx proxy_buffer_size directive via ConfigMap #1693
Allow customisation of the nginx proxy_buffer_size directive via ConfigMap #1693
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
I confirm that I'm the person who authored the commit. I didn't have my email address configured in my local git. |
clabot needs to be happy. You either need to push with the right user or re-sign, sorry. |
@@ -198,6 +198,9 @@ http://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout | |||
**proxy-send-timeout:** Sets the timeout in seconds for [transmitting a request to the proxied server](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout). The timeout is set only between two successive write operations, not for the transmission of the whole request. | |||
|
|||
|
|||
**proxy-buffer-size:** Sets the size of the buffer used for [reading the first part of the response](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffer_size) received from the proxied server. This part usually contains a small response header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add some words around tradeoffs, and why you picked 4k? this is just HEAD info right? dont' we also want proxy_buffers
to get most bang for the buck? (looking for a counter example where just proxy_buffer_size is useful)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked 4k as the smaller of the two defaults. To be honest I don't have enough operational experience with nginx to give meaningful advice on the tradeoffs involved. This PR was motivated by the need to configure proxy_buffer_size
to fix the upstream sent too big header while reading response header from upstream
errors that we encountered in our deployment.
I'm happy to update this PR to provide the ability to also configure proxy_buffering
and proxy_buffers
in addition to proxy_buffer_size
. Should I go ahead and do that?
proxy_buffering
is currently off
as configured by the nginx ingress controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxy_buffering is currently off as configured by the nginx ingress controller.
The reason for this is that when is on
introduces latency and issues with Server Sent Events/websockets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
Apart from fixing the CLA, what can I do to progress this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from fixing the CLA, what can I do to progress this PR?
Start making clabot happy :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll close this PR in favour of #1743. The latter contains the same changes, pushed with an email address that makes clabot happy :)
…buffer-size-2 Automatic merge from submit-queue Allow customisation of the nginx proxy_buffer_size directive via ConfigMap I'm opening a new PR with the same changes as #1693 because I pushed the latter with an email address that can't be used to sign the CLA. Description from the previous PR: When using nginx as a proxy we can run into the following error: ``` upstream sent too big header while reading response header from upstream ``` In order to fix this, we need to be able to configure the proxy_buffer_size nginx directive to increase its value. This PR updates the nginx-ingress-controller to allow that.
…-controller-proxy-buffer-size-2 Automatic merge from submit-queue Allow customisation of the nginx proxy_buffer_size directive via ConfigMap I'm opening a new PR with the same changes as kubernetes-retired#1693 because I pushed the latter with an email address that can't be used to sign the CLA. Description from the previous PR: When using nginx as a proxy we can run into the following error: ``` upstream sent too big header while reading response header from upstream ``` In order to fix this, we need to be able to configure the proxy_buffer_size nginx directive to increase its value. This PR updates the nginx-ingress-controller to allow that.
When using nginx as a proxy we can run into the following error:
In order to fix this, we need to be able to configure the
proxy_buffer_size
nginx directive to increase its value. This PR updates the nginx-ingress-controller to allow that.This change is![Reviewable](https://camo.githubusercontent.com/bdad2d5a4be7a00dc3b2426ea57eabd73ef84d8ed5ee05653b2f1501b6ea93ab/68747470733a2f2f72657669657761626c652e6b756265726e657465732e696f2f7265766965775f627574746f6e2e737667)