[PATCH] ngx_event_pipe: add judgment conditions for downstream errors.
mdounin at mdounin.ru
Mon Jul 30 17:21:10 UTC 2018
On Mon, Jul 30, 2018 at 09:33:11PM +0800, Robinson Nie wrote:
> Here is a patch to solve the following problem: When the proxy is turned
> on, if the downstream error (p->output_filter() returns an error), the
> current code will still try to read upstream & write downstream, which is
> actually unnecessary.
> # HG changeset patch
> # User dapeng.ndp<dapeng.ndp at alibaba-inc.com>
> # Date 1532956361 -28800
> # Mon Jul 30 21:12:41 2018 +0800
> # Node ID cbacf180df090540846aefa4a4a5453e307be219
> # Parent f7e79596baf209151682f2f7d220161c034657ac
> ngx_event_pipe: add judgment conditions for downstream errors.
> Avoid writing downstream & reading upstream, if there is an error due to
> writing downstream.
> diff -r f7e79596baf2 -r cbacf180df09 src/event/ngx_event_pipe.c
> --- a/src/event/ngx_event_pipe.c Tue Jul 24 18:46:54 2018 +0300
> +++ b/src/event/ngx_event_pipe.c Mon Jul 30 21:12:41 2018 +0800
> @@ -247,6 +247,7 @@
> } else if (!p->cacheable
> && p->downstream->data == p->output_ctx
> && p->downstream->write->ready
> + && !p->downstream_error
> && !p->downstream->write->delayed)
If there was a downstream error, nginx won't write to the
downstream anymore, see the ngx_event_pipe_write_to_downstream()
function. Instead, it will call the ngx_event_pipe_drain_chains()
function to discard all pending output buffers.
Reading from the upstream may be desired or not depending on
additional conditions. In particular, if caching is used, nginx
will have to read data from the upstream to save the response to
disk even in case of errors. If further reading of the upstream
response is not needed and p->downstream_error is set, nginx will
stop processing of the request in ngx_http_upstream_process_request().
Assuming p->downstream_error is set, this will happen once reading
from the upstream blocks.
Note well that the specific condition you are modifying is about
flushing the already read buffers if writing to the upstream is
possible. From semantic point of view, it is possible when
p->downstream_error is set (it will simply discard the buffers),
so the change looks wrong. Also, if you are trying to avoid
reading from the upstream if p->downstream_error is set, it looks
like a suboptimal place, as it will not prevent reading from the
upstream unless all buffers are filled.
Given the above, it is not clear what you are trying to improve.
The commit log is clearly misleading, and needs to be fixed. It
is also not clear if any changes are needed here. You may want to
elaborate more on what you are trying to improve.
More information about the nginx-devel