Hello!

On Fri, Dec 17, 2021 at 01:59:40PM +0800, Youlin Feng wrote:

> # HG changeset patch
> # User Youlin Feng <fengyoulin@live.com>
> # Date 1639718067 -28800
> # Fri Dec 17 13:14:27 2021 +0800
> # Node ID 54db7272bb0c9040eaf657f92bb02c9147d927e6
> # Parent a7a77549265ef46f1f0fdb3897f4beabf9e09c40
> HTTP: keepalive_graceful_close support.
>
> Previously, keepalived connections will be suddenly closed during worker
> process exiting, without notification to the client. The client may be
> sending a request when the connection is closed, resulting in an error.
>
> With "keepalive_graceful_close on;", the client will receive an
> "Connection: close" response header in the last request, then the
> connection can be closed gracefully.

Thanks for the patch.

First of all, you may want to clarify which problem you are trying
to solve. Note that clients are expected to be prepared for a
connection close by the server, and retry requests as appropriate.
If the client cannot handle this, probably there is a room for
improvement in the client. Further, there is an unavoidable race,
and a client which cannot handle connection close is likely to see
errors regardless of server behaviour. See ticket #1022
(https://trac.nginx.org/nginx/ticket/1022) for detailed
explanation.

What probably nginx should do here is to disable keepalive and
avoid sending "Connection: keep-alive" if it already knows that
keepalive cannot be used when sending response headers. This
should reduce unneeded request failures and retries at no
additional cost.

Patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1640613750 -10800
# Mon Dec 27 17:02:30 2021 +0300
# Node ID 0430ee0554a19281abcc074981702ac201f2922e
# Parent 4fa260f60bac03037017a33672da1ac75ac31408
Avoid sending "Connection: keep-alive" when shutting down.

When a worker process is shutting down, keepalive is not used: this is checked
before the ngx_http_set_keepalive() call in ngx_http_finalize_connection().
Yet the "Connection: keep-alive" header was still sent, even if we know that
the worker process is shutting down, potentially resulting in additional
requests being sent to the connection which is going to be closed anyway.
While clients are expected to be able to handle asynchronous close events
(see ticket #1022), it is certainly possible to send the "Connection: close"
header instead, informing the client that the connection is going to be closed
and potentially saving some unneeded work.

With this change, we additionally check for worker process shutdown just
before sending response headers, and disable keepalive accordingly.

diff --git a/src/http/ngx_http_header_filter_module.c b/src/http/ngx_http_header_filter_module.c
--- a/src/http/ngx_http_header_filter_module.c
+++ b/src/http/ngx_http_header_filter_module.c
@@ -197,6 +197,10 @@ ngx_http_header_filter(ngx_http_request_
}
}

+ if (r->keepalive && (ngx_terminate || ngx_exiting)) {
+ r->keepalive = 0;
+ }
+
len = sizeof("HTTP/1.x ") - 1 + sizeof(CRLF) - 1
/* the end of the header */
+ sizeof(CRLF) - 1;

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel