On Fri, Feb 19, 2016 at 8:21 AM, Yusuke Nojima <nojima@ynojima.com> wrote:
> diff -r 5dfc63c1d9ca -r 88bbee2e18f6 src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c Thu Feb 18 15:49:11 2016 +0300
> +++ b/src/http/ngx_http_upstream.c Fri Feb 19 15:48:40 2016 +0000
> @@ -3872,11 +3872,15 @@
> }
>
> if (status) {
> + int matches_next_condition = u->conf->next_upstream & ft_type;
> + if (u->conf->next_upstream & NGX_HTTP_UPSTREAM_FT_BEFORE_SEND)
> + matches_next_condition |= !u->request_sent;
> +
> u->state->status = status;
> timeout = u->conf->next_upstream_timeout;
>
> if (u->peer.tries == 0
> - || !(u->conf->next_upstream & ft_type)
> + || !matches_next_condition
> || (u->request_sent && r->request_body_no_buffering)
> || (timeout && ngx_current_msec - u->peer.start_time >= timeout))
> {

Hi All,

The patch seems to address an important issue, however one of the
conditions in the above if
statement is:

> || (u->request_sent && r->request_body_no_buffering)

which should guarantee that the request can be sent again *only* if
it's not already sent.

I easily understand "request_sent" flag here, but inclusion of
"request_body_no_buffering" flag seems wrong.

"request_body_no_buffering" seems to be flag to manage early/buffered
reads and I can't determine
why it's relevant at all here.

In other words, instead of this patch, shouldn't we modify that line as below?

> || u->request_sent

Regards,
Tolga Ceylan

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel