Arvados

8e43dcf899a3322e66c709b149ce557f5255bf18 at branch 19126-nginx-proxy-settings-change

developer-run-tests: #3112
developer-run-tests-apps-workbench-integration: #3330

The doc update refers to large responses, but the proxy_request_buffering setting is about large request bodies. Should we add "proxy_buffering off" as well?

Also, nginx doc says "When HTTP/1.1 chunked transfer encoding is used to send the original request body, the request body will be buffered regardless of the directive value unless HTTP/1.1 is enabled for proxying." ...and the default is HTTP/1.0. So I think we need to add "proxy_http_version 1.1;" too.

Tom Clegg wrote:

Also, nginx doc says "When HTTP/1.1 chunked transfer encoding is used to send the original request body, the request body will be buffered regardless of the directive value unless HTTP/1.1 is enabled for proxying." ...and the default is HTTP/1.0. So I think we need to add "proxy_http_version 1.1;" too.

As we discussed, the thing that actually works around the problem here is disabling the use of temporary files in response buffering, i.e. the proxy_max_temp_file_size 0; line.

So it seems like we should either limit this change to that line, or go the whole way and disable buffering in every direction:

    proxy_max_temp_file_size 0;
    proxy_request_buffering  off;
    proxy_buffering          off;
    proxy_http_version       1.1;

I think we should go all the way and disable buffering in both directions. I don't think we get any benefit from buffering -- afaict all it is is a source of problems.

Tom Clegg wrote:

I think we should go all the way and disable buffering in both directions. I don't think we get any benefit from buffering -- afaict all it is is a source of problems.

e3363715769a2503fdcbbb1274d7d04c6852c9c3 on branch 19126-nginx-proxy-settings-change

developer-run-tests: #3122

LGTM.

Seems to be based on the #16345 branch, but I've just merged that now, so I don't think it needs to be rebased.

Tom Clegg wrote:

LGTM.

Seems to be based on the #16345 branch, but I've just merged that now, so I don't think it needs to be rebased.

Looks like I missed making the change in the nginx template used for our tests, running the tests one more time:

10d497fe4eb9ab1dc10171ba90c0c1b148108a35 on branch 19126-nginx-proxy-settings-change

developer-run-tests: #3124

And I also missed the change in the installer templates:

f076c4497f2fce89f8fef9ccd601fcf316901993 on branch 19126-nginx-proxy-settings-change

test-provision: #358
test-provision-debian10: #357

LGTM, thanks