[PATCH] Resolver: make TCP write timer event cancelable.
mdounin at mdounin.ru
Thu May 26 22:31:21 UTC 2022
On Thu, May 26, 2022 at 03:50:22PM +0000, Aleksei Bavshin via nginx-devel wrote:
> # HG changeset patch
> # User Aleksei Bavshin <a.bavshin at f5.com>
> # Date 1653579686 25200
> # Thu May 26 08:41:26 2022 -0700
> # Branch se
Looks like a wrong branch to me.
> # Node ID afb3646a08b6ebcf46fdbd2e7bf97cdcccca774b
> # Parent 5a98e9cb437f7719afa2bde62de68e174fd8e03e
> Resolver: make TCP write timer event cancelable.
> In certain circumstances, this timer could become the only remaining
> non-cancelable timer in a worker, effectively delaying the worker shutdown.
> We're keeping the resolver alive to avoid disrupting any leftover tasks in the
> worker that may require up-to-date upstream peer data, but it is unnecessary
> in absence of other connections. And any useful workload would have its own
> non-cancelable timers, so it is safe to allow canceling this one.
This doesn't sound correct to me: when nginx is doing upstream
dynamic name-to-address resolution during request processing (see
ngx_http_upstream_init_request()), the request has no "own
Still, there should be resolve task timer, and canceling the tcp
timer should be safe, similarly to canceling the resolver resend
timer, see 70e65bf8dfd7. And, BTW, it might be a good idea to
explicitly mention this changeset in the commit log.
> The scenario in question is a periodic resolver task with a period less than
> the TCP write timeout and responses large enough to warrant fallback to a TCP
> connection - something that could be reproduced with a dynamic upstream
> implementation. With each event loop wakeup, we either see a previously
> set write timer instance or schedule a new one.
> diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
> --- a/src/core/ngx_resolver.c
> +++ b/src/core/ngx_resolver.c
> @@ -1624,6 +1624,7 @@ ngx_resolver_send_tcp_query(ngx_resolver
> rec->tcp->data = rec;
> + rec->tcp->write->cancelable = 1;
> rec->tcp->write->handler = ngx_resolver_tcp_write;
> rec->tcp->read->handler = ngx_resolver_tcp_read;
> rec->tcp->read->resolver = 1;
>From style point of view, it would be a good idea to set flags
after the handler.
More information about the nginx-devel