<div dir="ltr"><div>> Hello!</div><div>> Thank you for the patch.</div><div>> </div><div>> The problem looks valid - indeed, if in non-buffered proxy mode</div><div>> writing to the client blocked, it is possible that we'll continue</div><div>> reading from upstream after the response was fully received, and</div><div>> will keep upstream read timer set. This in turn can result in</div><div>> proxy_read_timeout being triggered if it is smaller than send_timeout.</div><div><br></div><div>Yes, we were plagued with this problem because we needed to make a distinction between upstream timeout and client timeout in our production system.</div><div><br></div><div>> I can't say I like the suggested change though. The resulting</div><div>> conditions look overcomplicated. Also, likely we have similar</div><div>> problem in ngx_http_upstream_process_upgraded(), and it probably</div><div>> needs fixing too.</div><div><br></div><div>> Also, suggested condition is incorrect, as it will keep timer if</div><div>> upstream->read->eof is set, but u->length isn't -1, or if</div><div>> upstream->read->error is set.</div><div><br></div><div>I'm sorry I missed another condition and the correct one may be</div><div><br></div><div>```</div><div>if (upstream->read->active && !upstream->read->ready</div><div> && !(u->length == 0 || (upstream->read->eof && u->length == -1))</div><div> && !upstream->read->error && upstream->read->eof)</div><div>{</div><div>```</div><div><br></div><div>It is assuredly overcomplicated, or it can be in another function to check the correct condition.</div><div><br></div><div>> In case of ngx_http_upstream_process_non_buffered_request() a</div><div>> better approach might be to don't try to do any processing in the</div><div>> proxy module after response is received from upstream, but rather</div><div>> finalize the upstream connection and pass responsibility to</div><div>> ngx_writer() instead, as we do in the normal (aka buffered) case.</div><div>> For ngx_http_upstream_process_upgraded(), additional ->eof /</div><div>> ->error checks are probably needed.</div><div><br></div><div>In case of ngx_http_upstream_process_non_buffered_request(), NGINX receives from upstream and immediately sends to downstream. I'm afraid the problem still will not be solved accord with "after response is received from upstream".</div><div>Furthermore, kqueue notifies about the end of file or a pending error while epoll don't.</div><div><br></div><div>Do you have any other solutions or advices? </div><div><br></div><div>Thanks to inform me.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-01-23 0:34 GMT+08:00 Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello!<br>
<div><div class="h5"><br>
On Fri, Jan 19, 2018 at 08:43:06PM +0800, xiaolong hong wrote:<br>
<br>
> # HG changeset patch<br>
> # User Xiaolong Hong <<a href="mailto:imhongxiaolong@gmail.com">imhongxiaolong@gmail.com</a>><br>
> # Date 1516354115 -28800<br>
> # Fri Jan 19 17:28:35 2018 +0800<br>
> # Node ID f017b8c1a99433cc3321475968556a<wbr>ee50609145<br>
> # Parent 93abb5a855d6534f0356882f45be49<wbr>f8c6a95a8b<br>
> Fixed upstream->read timer when downstream->write not ready.<br>
><br>
> diff -r 93abb5a855d6 -r f017b8c1a994 src/http/ngx_http_upstream.c<br>
> --- a/src/http/ngx_http_upstream.c Thu Jan 11 21:43:49 2018 +0300<br>
> +++ b/src/http/ngx_http_upstream.c Fri Jan 19 17:28:35 2018 +0800<br>
> @@ -3625,7 +3625,9 @@ ngx_http_upstream_process_non_<wbr>buffered_r<br>
> return;<br>
> }<br>
><br>
> - if (upstream->read->active && !upstream->read->ready) {<br>
> + if (upstream->read->active && !upstream->read->ready<br>
> + && !(u->length == 0 || (upstream->read->eof && u->length == -1)))<br>
> + {<br>
> ngx_add_timer(upstream->read, u->conf->read_timeout);<br>
><br>
> } else if (upstream->read->timer_set) {<br>
><br>
> --------<br>
><br>
> When downstream hung and nginx received the last buffer from upstream, both<br>
> downstream->write timer and upstream->read timer would be added in the<br>
> meantime because downstream->write->ready and upstream->read->ready would<br>
> opportunely be 0.<br>
><br>
> Actually if clcf->send_timeout less then u->conf->read_timeout,<br>
> upstream->read timer would be waked before downstream->write timer, it<br>
> caused mistakes to report the "upstream timed out" error deviating from the<br>
> fact that upstream worked normally but downstream hung.<br>
><br>
> This problem could be fixed to check upstream eof when trying to add<br>
> upstream->read timer.<br>
<br>
</div></div>Thank you for the patch.<br>
<br>
The problem looks valid - indeed, if in non-buffered proxy mode<br>
writing to the client blocked, it is possible that we'll continue<br>
reading from upstream after the response was fully received, and<br>
will keep upstream read timer set. This in turn can result in<br>
proxy_read_timeout being triggered if it is smaller than send_timeout.<br>
<br>
I can't say I like the suggested change though. The resulting<br>
conditions look overcomplicated. Also, likely we have similar<br>
problem in ngx_http_upstream_process_<wbr>upgraded(), and it probably<br>
needs fixing too.<br>
<br>
Also, suggested condition is incorrect, as it will keep timer if<br>
upstream->read->eof is set, but u->length isn't -1, or if<br>
upstream->read->error is set.<br>
<br>
In case of ngx_http_upstream_process_non_<wbr>buffered_request() a<br>
better approach might be to don't try to do any processing in the<br>
proxy module after response is received from upstream, but rather<br>
finalize the upstream connection and pass responsibility to<br>
ngx_writer() instead, as we do in the normal (aka buffered) case.<br>
For ngx_http_upstream_process_<wbr>upgraded(), additional ->eof /<br>
->error checks are probably needed.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Maxim Dounin<br>
<a href="http://mdounin.ru/" rel="noreferrer" target="_blank">http://mdounin.ru/</a><br>
______________________________<wbr>_________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/<wbr>mailman/listinfo/nginx-devel</a><br>
</font></span></blockquote></div><br></div>