<div dir="ltr"><div>Hi, glad to see you here.</div><div><br></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">"The directive configures a timeout to be used when gracefully shutting down
worker processes. When the timer expires, nginx will try to close all
the connections currently open to facilitate shutdown."</span><br></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre"><br></span></div><div>Well, if the only <span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">purpose is to close all the connections those fields 'cancelable' are zero,</span></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">it works well now.</span></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre"><br></span></div><div><br></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">"Cancelable timers are now preserved if there are other timers.
There is no need to cancel timers early if there are other timers blocking
shutdown anyway. Preserving such timers allows nginx to continue some
periodic work till the shutdown is actually possible.
With the new approach, timers with ev->cancelable are simply ignored when
checking if there are any timers left during shutdown."</span><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre"><br></span></div><div><br></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">But how to do the timers those are </span><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">cancelable such as buffer-event in log module.</span></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre">When the worker process is shutting down, they lose the chance to do registered handler.</span></div><div><font color="#000000" face="monospace" size="3"><span style="white-space:pre">Even so the log module also works well because of 'ngx_conf_flush_files'.</span></font></div><div><font color="#000000" face="monospace" size="3"><span style="white-space:pre"><br></span></font></div><div><font color="#000000" face="monospace" size="3"><span style="white-space:pre">So there are two questions:</span></font></div><div><font color="#000000" face="monospace" size="3"><span style="white-space:pre">1. Do we need to preserve cancelable timers until </span></font><span style="font-family:monospace,monospace"><i><font size="1">worker_shutdown_timeout</font></i></span><span style="font-family:monospace,monospace"><font size="1"><i>?</i></font></span></div><div><span style="font-family:monospace,monospace">2<i style="font-size:x-small">. </i>If needed, it seems it's unable to invoke </span><span style="font-family:monospace,monospace"><i><font size="1">ngx_shutdown_timer_handler </font></i>there are still cancelable timers left and no one active connections.</span></div><div><span style="font-family:monospace,monospace"><br></span></div><div><span style="font-family:monospace,monospace"><br></span></div><div><span style="font-family:monospace,monospace">Thanks.</span></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre"><br></span></div><div><span style="color:rgb(0,0,0);font-family:monospace;font-size:medium;white-space:pre"><br></span></div><div class="gmail_extra"><br><div class="gmail_quote">2017-03-09 7:40 GMT+08:00 Datong Sun via nginx-devel <span dir="ltr"><<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hello!<div><br></div><div>Looks like to me that the original patch does what it's supposed to do (when combined with <a href="http://hg.nginx.org/nginx/rev/3069dd358ba2" rel="noreferrer" style="font-size:12.8px" target="_blank">http://hg.nginx.org/<wbr>nginx/rev/3069dd358ba2</a>). Here is my understanding:</div><div><br></div><div>Before this patch, an active connection could potentially delay shutdown indefinitely due to the presence of connection related timer inside the timer tree. The only reason why <span style="font-size:12.8px"><font face="monospace, monospace">ngx_shutdown_timer_handler</font> has to be invoked would be to force those lingering connections to be closed and therefore, free-up the timer tree and allow the shutdown to finish.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">If all connections were closed before </span><span style="font-family:monospace,monospace;font-size:12.8px">ngx_shutdown_timer_<wbr>handler</span><span style="font-size:12.8px"> was fired (your case), it is already safe to finish the shutdown and there is no reason to invoke or wait for </span><span style="font-family:monospace,monospace;font-size:12.8px">ngx_shutdown_timer_handler</span><wbr> to fire anymore<span style="font-family:monospace,monospace;font-size:12.8px">.</span> The <font face="monospace, monospace">cancelable</font> field makes sure when this happens <font face="monospace, monospace">ngx_shutdown_event</font> <wbr>does not prevent NGINX from being able to complete the shutdown.</div><div><br></div><div>Seems to me that delaying the shutdown until <font face="monospace, monospace">worker_shutdown_timeout</font> when there is already no active connection left makes little sense.</div><div><br></div><div>Thanks,</div><div>Datong</div></div><div class="gmail_extra"><div><div class="gmail-h5"><br><div class="gmail_quote">On Tue, Mar 7, 2017 at 11:52 PM, 洪志道 <span dir="ltr"><<a href="mailto:hongzhidao@gmail.com" target="_blank">hongzhidao@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi!<div><br></div><div>It's a good design.</div><div>It seems the worker process is not killed until expired, the time is set by '<span style="font-size:14px">worker_shutdown_timeout'.</span></div><div><span style="font-size:14px">But I think </span><span style="font-size:14px">ngx_shutdown_timer_handl<wbr>er will never be called, because of the following deal.</span></div><div><span style="font-size:14px"><br></span></div><div><div><span style="font-size:14px"> if (ngx_exiting) {</span></div><div><span style="font-size:14px"> if (ngx_event_no_timers_left() == NGX_OK) {</span></div><div><span style="font-size:14px"> ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "exiting");</span></div><div><span style="font-size:14px"> ngx_worker_process_exit(cycle)<wbr>; // the worker process exits though there are cancelable timers such as </span><span style="font-size:14px">ngx_shutdown_event.</span></div><div><span style="font-size:14px"> }</span></div><div><span style="font-size:14px"> }</span></div></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px">I'm not sure it's right?</span></div><div><span style="font-size:14px"><br></span></div><div><div><span style="font-size:14px">diff -r d45072375572 src/core/ngx_cycle.c</span></div><div><span style="font-size:14px">--- a/src/core/ngx_cycle.c<span class="gmail-m_3387288055963784541m_-8642361306868347428gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>Tue Mar 07 18:51:17 2017 +0300</span></div><div><span style="font-size:14px">+++ b/src/core/ngx_cycle.c<span class="gmail-m_3387288055963784541m_-8642361306868347428gmail-Apple-tab-span" style="white-space:pre-wrap"> </span>Sun Mar 05 17:03:22 2017 +0800</span></div><div><span style="font-size:14px">@@ -1348,7 +1348,6 @@</span></div><div><span style="font-size:14px"> ngx_shutdown_event.handler = ngx_shutdown_timer_handler;</span></div><div><span style="font-size:14px"> ngx_shutdown_event.data = cycle;</span></div><div><span style="font-size:14px"> ngx_shutdown_event.log = cycle->log;</span></div><div><span style="font-size:14px">- ngx_shutdown_event.cancelable = 1;</span></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px"> ngx_add_timer(&ngx_shutdown_e<wbr>vent, ccf->shutdown_timeout);</span></div><div><span style="font-size:14px"> }</span></div></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px">And I think it's better that the shutting down process keep alive until '</span><span style="font-size:14px">worker_shutdown_</span><span style="font-size:14px">timeout'.</span></div><div><span style="font-size:14px"><br></span></div><div><span style="font-size:14px">Thanks.<br>B.R.</span></div></div><div class="gmail-m_3387288055963784541HOEnZb"><div class="gmail-m_3387288055963784541h5"><div class="gmail_extra"><br><div class="gmail_quote">2017-03-08 1:01 GMT+08:00 Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">details: <a href="http://hg.nginx.org/nginx/rev/97c99bb43737" rel="noreferrer" target="_blank">http://hg.nginx.org/nginx/rev<wbr>/97c99bb43737</a><br>
branches:<br>
changeset: 6930:97c99bb43737<br>
user: Maxim Dounin <<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>><br>
date: Tue Mar 07 18:51:16 2017 +0300<br>
description:<br>
Introduced worker_shutdown_timeout.<br>
<br>
The directive configures a timeout to be used when gracefully shutting down<br>
worker processes. When the timer expires, nginx will try to close all<br>
the connections currently open to facilitate shutdown.<br>
<br>
diffstat:<br>
<br>
src/core/nginx.c | 9 ++++++<br>
src/core/ngx_cycle.c | 53 ++++++++++++++++++++++++++++++<wbr>++++++++++<br>
src/core/ngx_cycle.h | 2 +<br>
src/os/unix/ngx_process_cycle<wbr>.c | 1 +<br>
src/os/win32/ngx_process_cycl<wbr>e.c | 1 +<br>
5 files changed, 66 insertions(+), 0 deletions(-)<br>
<br>
diffs (148 lines):<br>
<br>
diff --git a/src/core/nginx.c b/src/core/nginx.c<br>
--- a/src/core/nginx.c<br>
+++ b/src/core/nginx.c<br>
@@ -124,6 +124,13 @@ static ngx_command_t ngx_core_commands[<br>
offsetof(ngx_core_conf_t, rlimit_core),<br>
NULL },<br>
<br>
+ { ngx_string("worker_shutdown_ti<wbr>meout"),<br>
+ NGX_MAIN_CONF|NGX_DIRECT_CONF|<wbr>NGX_CONF_TAKE1,<br>
+ ngx_conf_set_msec_slot,<br>
+ 0,<br>
+ offsetof(ngx_core_conf_t, shutdown_timeout),<br>
+ NULL },<br>
+<br>
{ ngx_string("working_directory"<wbr>),<br>
NGX_MAIN_CONF|NGX_DIRECT_CONF<wbr>|NGX_CONF_TAKE1,<br>
ngx_conf_set_str_slot,<br>
@@ -1014,6 +1021,7 @@ ngx_core_module_create_conf(ng<wbr>x_cycle_t<br>
ccf->daemon = NGX_CONF_UNSET;<br>
ccf->master = NGX_CONF_UNSET;<br>
ccf->timer_resolution = NGX_CONF_UNSET_MSEC;<br>
+ ccf->shutdown_timeout = NGX_CONF_UNSET_MSEC;<br>
<br>
ccf->worker_processes = NGX_CONF_UNSET;<br>
ccf->debug_points = NGX_CONF_UNSET;<br>
@@ -1042,6 +1050,7 @@ ngx_core_module_init_conf(ngx_<wbr>cycle_t *c<br>
ngx_conf_init_value(ccf->daem<wbr>on, 1);<br>
ngx_conf_init_value(ccf->mast<wbr>er, 1);<br>
ngx_conf_init_msec_value(ccf-<wbr>>timer_resolution, 0);<br>
+ ngx_conf_init_msec_value(ccf-><wbr>shutdown_timeout, 0);<br>
<br>
ngx_conf_init_value(ccf->work<wbr>er_processes, 1);<br>
ngx_conf_init_value(ccf->debu<wbr>g_points, 0);<br>
diff --git a/src/core/ngx_cycle.c b/src/core/ngx_cycle.c<br>
--- a/src/core/ngx_cycle.c<br>
+++ b/src/core/ngx_cycle.c<br>
@@ -15,6 +15,7 @@ static ngx_int_t ngx_init_zone_pool(ngx_<br>
ngx_shm_zone_t *shm_zone);<br>
static ngx_int_t ngx_test_lockfile(u_char *file, ngx_log_t *log);<br>
static void ngx_clean_old_cycles(ngx_event<wbr>_t *ev);<br>
+static void ngx_shutdown_timer_handler(ngx<wbr>_event_t *ev);<br>
<br>
<br>
volatile ngx_cycle_t *ngx_cycle;<br>
@@ -22,6 +23,7 @@ ngx_array_t ngx_old_cycles;<br>
<br>
static ngx_pool_t *ngx_temp_pool;<br>
static ngx_event_t ngx_cleaner_event;<br>
+static ngx_event_t ngx_shutdown_event;<br>
<br>
ngx_uint_t ngx_test_config;<br>
ngx_uint_t ngx_dump_config;<br>
@@ -1333,3 +1335,54 @@ ngx_clean_old_cycles(ngx_event<wbr>_t *ev)<br>
ngx_old_cycles.nelts = 0;<br>
}<br>
}<br>
+<br>
+<br>
+void<br>
+ngx_set_shutdown_timer(ngx_cy<wbr>cle_t *cycle)<br>
+{<br>
+ ngx_core_conf_t *ccf;<br>
+<br>
+ ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx, ngx_core_module);<br>
+<br>
+ if (ccf->shutdown_timeout) {<br>
+ ngx_shutdown_event.handler = ngx_shutdown_timer_handler;<br>
+ ngx_shutdown_event.data = cycle;<br>
+ ngx_shutdown_event.log = cycle->log;<br>
+ ngx_shutdown_event.cancelable = 1;<br>
+<br>
+ ngx_add_timer(&ngx_shutdown_ev<wbr>ent, ccf->shutdown_timeout);<br>
+ }<br>
+}<br>
+<br>
+<br>
+static void<br>
+ngx_shutdown_timer_handler(ng<wbr>x_event_t *ev)<br>
+{<br>
+ ngx_uint_t i;<br>
+ ngx_cycle_t *cycle;<br>
+ ngx_connection_t *c;<br>
+<br>
+ cycle = ev->data;<br>
+<br>
+ c = cycle->connections;<br>
+<br>
+ for (i = 0; i < cycle->connection_n; i++) {<br>
+<br>
+ if (c[i].fd == (ngx_socket_t) -1<br>
+ || c[i].read == NULL<br>
+ || c[i].read->accept<br>
+ || c[i].read->channel<br>
+ || c[i].read->resolver)<br>
+ {<br>
+ continue;<br>
+ }<br>
+<br>
+ ngx_log_debug1(NGX_LOG_DEBUG_C<wbr>ORE, ev->log, 0,<br>
+ "*%uA shutdown timeout", c[i].number);<br>
+<br>
+ c[i].close = 1;<br>
+ c[i].error = 1;<br>
+<br>
+ c[i].read->handler(c[i].read);<br>
+ }<br>
+}<br>
diff --git a/src/core/ngx_cycle.h b/src/core/ngx_cycle.h<br>
--- a/src/core/ngx_cycle.h<br>
+++ b/src/core/ngx_cycle.h<br>
@@ -88,6 +88,7 @@ typedef struct {<br>
ngx_flag_t master;<br>
<br>
ngx_msec_t timer_resolution;<br>
+ ngx_msec_t shutdown_timeout;<br>
<br>
ngx_int_t worker_processes;<br>
ngx_int_t debug_points;<br>
@@ -129,6 +130,7 @@ ngx_pid_t ngx_exec_new_binary(ngx_cycle_<br>
ngx_cpuset_t *ngx_get_cpu_affinity(ngx_uint<wbr>_t n);<br>
ngx_shm_zone_t *ngx_shared_memory_add(ngx_con<wbr>f_t *cf, ngx_str_t *name,<br>
size_t size, void *tag);<br>
+void ngx_set_shutdown_timer(ngx_cyc<wbr>le_t *cycle);<br>
<br>
<br>
extern volatile ngx_cycle_t *ngx_cycle;<br>
diff --git a/src/os/unix/ngx_process_cycl<wbr>e.c b/src/os/unix/ngx_process_cycl<wbr>e.c<br>
--- a/src/os/unix/ngx_process_cycl<wbr>e.c<br>
+++ b/src/os/unix/ngx_process_cycl<wbr>e.c<br>
@@ -763,6 +763,7 @@ ngx_worker_process_cycle(ngx_c<wbr>ycle_t *cy<br>
<br>
if (!ngx_exiting) {<br>
ngx_exiting = 1;<br>
+ ngx_set_shutdown_timer(cycle);<br>
ngx_close_listening_sockets(c<wbr>ycle);<br>
ngx_close_idle_connections(cy<wbr>cle);<br>
}<br>
diff --git a/src/os/win32/ngx_process_cyc<wbr>le.c b/src/os/win32/ngx_process_cyc<wbr>le.c<br>
--- a/src/os/win32/ngx_process_cyc<wbr>le.c<br>
+++ b/src/os/win32/ngx_process_cyc<wbr>le.c<br>
@@ -800,6 +800,7 @@ ngx_worker_thread(void *data)<br>
<br>
if (!ngx_exiting) {<br>
ngx_exiting = 1;<br>
+ ngx_set_shutdown_timer(cycle);<br>
ngx_close_listening_sockets(c<wbr>ycle);<br>
ngx_close_idle_connections(cy<wbr>cle);<br>
}<br>
______________________________<wbr>_________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/mailm<wbr>an/listinfo/nginx-devel</a><br>
</blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/mailm<wbr>an/listinfo/nginx-devel</a><br></blockquote></div><br><br clear="all"><div><br></div></div></div><span class="gmail-HOEnZb"><font color="#888888">-- <br><div class="gmail-m_3387288055963784541gmail_signature"><div dir="ltr"><p style="font-family:helvetica;font-size:12px;color:rgb(64,64,64)"><b>Datong Sun</b> | Systems Engineer<br><a href="mailto:datong@cloudflare.com" style="color:rgb(47,123,191)" target="_blank">datong@cloudflare.com</a></p><a href="https://www.cloudflare.com/" style="font-family:times;font-size:medium" target="_blank"><div style="background-image:url("https://www.cloudflare.com/img/signature-cloud.png");width:200px;height:30px;margin-right:20px;margin-top:20px"></div></a><p style="font-family:helvetica;font-size:12px;color:rgb(64,64,64)">1 888 99 FLARE | <a href="https://www.cloudflare.com/" style="color:rgb(47,123,191)" target="_blank">www.cloudflare.com</a></p></div></div>
</font></span></div>
<br>______________________________<wbr>_________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" rel="noreferrer" target="_blank">http://mailman.nginx.org/<wbr>mailman/listinfo/nginx-devel</a><br></blockquote></div><br></div></div>