Hello!
In my scenario, nginx is used with consul-template as a load balancer that implements service discovery. Each time a service instance is registered to or unregistered from consul，consul-template will trigger all nginx instances to reload. In a busy system, this happens very frequently, and each reload will cause many "keepalived" connections to be closed. The client based on golang got a lot of "server closed idle connection" and "EOF" errors.
Of course, the client should implement the "auto retry" logic, but I think we have a better way to completely solve this problem in nginx. My idea is, don't treat the keepalived http connections as idle connections, and send a "Connection: close" header in the next response. If no more requests come, the worker process will wait until "keepalive_timeout", then exit, and won't wait long. So, if we give the client a smaller "IdleConnTimeout", there is no race now. If someone doesn't want to use this patch, just leave "keepalive_graceful_close" unset.
According to your suggestion, I optimized my patch:
# HG changeset patch
# User Youlin Feng <fengyoulin@live.com>
# Date 1640704599 -28800
# Tue Dec 28 23:16:39 2021 +0800
# Node ID b763dec013e5e26e8801614df74b45a85a540e8e
# Parent b002ad258f1d70924dc13d8f4bc0cc44362f0d0a
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 next request, then the
connection can be closed gracefully. If no more requests come, the worker
process will wait until "keepalive_timeout" and then exit.
diff -r b002ad258f1d -r b763dec013e5 src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c Mon Dec 27 19:49:26 2021 +0300
+++ b/src/http/ngx_http_core_module.c Tue Dec 28 23:16:39 2021 +0800
@@ -523,6 +523,13 @@
offsetof(ngx_http_core_loc_conf_t, keepalive_disable),
&ngx_http_core_keepalive_disable },
+ { ngx_string("keepalive_graceful_close"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+ ngx_conf_set_flag_slot,
+ NGX_HTTP_LOC_CONF_OFFSET,
+ offsetof(ngx_http_core_loc_conf_t, keepalive_graceful_close),
+ NULL },
+
{ ngx_string("satisfy"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
ngx_conf_set_enum_slot,
@@ -3517,6 +3524,7 @@
clcf->keepalive_timeout = NGX_CONF_UNSET_MSEC;
clcf->keepalive_header = NGX_CONF_UNSET;
clcf->keepalive_requests = NGX_CONF_UNSET_UINT;
+ clcf->keepalive_graceful_close = NGX_CONF_UNSET;
clcf->lingering_close = NGX_CONF_UNSET_UINT;
clcf->lingering_time = NGX_CONF_UNSET_MSEC;
clcf->lingering_timeout = NGX_CONF_UNSET_MSEC;
@@ -3756,6 +3764,8 @@
prev->keepalive_header, 0);
ngx_conf_merge_uint_value(conf->keepalive_requests,
prev->keepalive_requests, 1000);
+ ngx_conf_merge_value(conf->keepalive_graceful_close,
+ prev->keepalive_graceful_close, 0);
ngx_conf_merge_uint_value(conf->lingering_close,
prev->lingering_close, NGX_HTTP_LINGERING_ON);
ngx_conf_merge_msec_value(conf->lingering_time,
diff -r b002ad258f1d -r b763dec013e5 src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h Mon Dec 27 19:49:26 2021 +0300
+++ b/src/http/ngx_http_core_module.h Tue Dec 28 23:16:39 2021 +0800
@@ -381,6 +381,7 @@
ngx_flag_t client_body_in_single_buffer;
/* client_body_in_singe_buffer */
+ ngx_flag_t keepalive_graceful_close; /* keepalive_graceful_close */
ngx_flag_t internal; /* internal */
ngx_flag_t sendfile; /* sendfile */
ngx_flag_t aio; /* aio */
diff -r b002ad258f1d -r b763dec013e5 src/http/ngx_http_header_filter_module.c
--- a/src/http/ngx_http_header_filter_module.c Mon Dec 27 19:49:26 2021 +0300
+++ b/src/http/ngx_http_header_filter_module.c Tue Dec 28 23:16:39 2021 +0800
@@ -197,6 +197,10 @@
}
}
+ 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;
diff -r b002ad258f1d -r b763dec013e5 src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c Mon Dec 27 19:49:26 2021 +0300
+++ b/src/http/ngx_http_request.c Tue Dec 28 23:16:39 2021 +0800
@@ -2767,7 +2767,7 @@
}
if (!ngx_terminate
- && !ngx_exiting
+ && (!ngx_exiting || clcf->keepalive_graceful_close)
&& r->keepalive
&& clcf->keepalive_timeout > 0)
{
@@ -3252,7 +3252,7 @@
r->http_state = NGX_HTTP_KEEPALIVE_STATE;
#endif
- c->idle = 1;
+ c->idle = !clcf->keepalive_graceful_close;
ngx_reusable_connection(c, 1);
ngx_add_timer(rev, clcf->keepalive_timeout);
From: nginx-devel <nginx-devel-bounces@nginx.org> on behalf of Maxim Dounin <mdounin@mdounin.ru>
Sent: Monday, December 27, 2021 22:08
To: nginx-devel@nginx.org <nginx-devel@nginx.org>
Subject: Re: [PATCH] HTTP: keepalive_graceful_close support
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
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
