[PATCH] Upstream: flush low-level buffers on write retry

[Maxim Konovalov]

On 05/12/2017 18:16, Maxim Dounin wrote:
> Hello!
> 
> On Fri, Dec 01, 2017 at 04:49:29PM -0800, Piotr Sikora via nginx-devel wrote:
> 
>> # HG changeset patch
>> # User Patryk Lesiewicz <patryk at google.com>
>> # Date 1512172754 28800
>> #      Fri Dec 01 15:59:14 2017 -0800
>> # Node ID 0e2e2da798261fe5105017d9678566267b07e2b9
>> # Parent  fc0d06224edac2c7cfbfd9a4def478f285d9957b
>> Upstream: flush low-level buffers on write retry.
>>
>> If the data to write is bigger than what the socket can send, and the
>> reminder is smaller than NGX_SSL_BUFSIZE, then SSL_write() fails with
>> SSL_ERROR_WANT_WRITE. The reminder of payload however is successfully
>> copied to the low-level buffer and all the output chain buffers are
>> flushed. This means that retry logic doesn't work because
>> ngx_http_upstream_process_non_buffered_request() checks only if there's
>> anything in the output chain buffers and ignores the fact that something
>> may be buffered in low-level parts of the stack.
>>
>> Signed-off-by: Patryk Lesiewicz <patryk at google.com>
>>
>> diff -r fc0d06224eda -r 0e2e2da79826 src/http/ngx_http_upstream.c
>> --- a/src/http/ngx_http_upstream.c
>> +++ b/src/http/ngx_http_upstream.c
>> @@ -3533,7 +3533,7 @@ ngx_http_upstream_process_non_buffered_r
>>  
>>          if (do_write) {
>>  
>> -            if (u->out_bufs || u->busy_bufs) {
>> +            if (u->out_bufs || u->busy_bufs || downstream->buffered) {
>>                  rc = ngx_http_output_filter(r, u->out_bufs);
>>  
>>                  if (rc == NGX_ERROR) {
> 
> Committed, thanks for catching this.
> http://hg.nginx.org/nginx/rev/a762ddf22dbb
> 
Do we need tests for this scenario?

-- 
Maxim Konovalov

"I'm not a software developer, but it doesn't seem as rocket
science"