[nginx] Introduced worker_shutdown_timeout.
洪志道
hongzhidao at gmail.com
Thu Mar 9 06:39:26 UTC 2017
Hi, glad to see you here.
"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."
Well, if the only purpose is to close all the connections those fields
'cancelable' are zero,
it works well now.
"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."
But how to do the timers those are cancelable such as buffer-event in log
module.
When the worker process is shutting down, they lose the chance to do
registered handler.
Even so the log module also works well because of 'ngx_conf_flush_files'.
So there are two questions:
1. Do we need to preserve cancelable timers until *worker_shutdown_timeout*
*?*
2*. *If needed, it seems it's unable to invoke
*ngx_shutdown_timer_handler *there
are still cancelable timers left and no one active connections.
Thanks.
2017-03-09 7:40 GMT+08:00 Datong Sun via nginx-devel <nginx-devel at nginx.org>
:
> Hello!
>
> Looks like to me that the original patch does what it's supposed to do
> (when combined with http://hg.nginx.org/nginx/rev/3069dd358ba2). Here is
> my understanding:
>
> 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 ngx_shutdown_timer_handler 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.
>
> If all connections were closed before ngx_shutdown_timer_handler was
> fired (your case), it is already safe to finish the shutdown and there is
> no reason to invoke or wait for ngx_shutdown_timer_handler to fire anymore
> . The cancelable field makes sure when this happens ngx_shutdown_event does
> not prevent NGINX from being able to complete the shutdown.
>
> Seems to me that delaying the shutdown until worker_shutdown_timeout when
> there is already no active connection left makes little sense.
>
> Thanks,
> Datong
>
> On Tue, Mar 7, 2017 at 11:52 PM, 洪志道 <hongzhidao at gmail.com> wrote:
>
>> Hi!
>>
>> It's a good design.
>> It seems the worker process is not killed until expired, the time is set
>> by 'worker_shutdown_timeout'.
>> But I think ngx_shutdown_timer_handler will never be called, because of
>> the following deal.
>>
>> if (ngx_exiting) {
>> if (ngx_event_no_timers_left() == NGX_OK) {
>> ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "exiting");
>> ngx_worker_process_exit(cycle); // the worker process
>> exits though there are cancelable timers such as ngx_shutdown_event.
>> }
>> }
>>
>> I'm not sure it's right?
>>
>> diff -r d45072375572 src/core/ngx_cycle.c
>> --- a/src/core/ngx_cycle.c Tue Mar 07 18:51:17 2017 +0300
>> +++ b/src/core/ngx_cycle.c Sun Mar 05 17:03:22 2017 +0800
>> @@ -1348,7 +1348,6 @@
>> ngx_shutdown_event.handler = ngx_shutdown_timer_handler;
>> ngx_shutdown_event.data = cycle;
>> ngx_shutdown_event.log = cycle->log;
>> - ngx_shutdown_event.cancelable = 1;
>>
>> ngx_add_timer(&ngx_shutdown_event, ccf->shutdown_timeout);
>> }
>>
>> And I think it's better that the shutting down process keep alive until '
>> worker_shutdown_timeout'.
>>
>> Thanks.
>> B.R.
>>
>> 2017-03-08 1:01 GMT+08:00 Maxim Dounin <mdounin at mdounin.ru>:
>>
>>> details: http://hg.nginx.org/nginx/rev/97c99bb43737
>>> branches:
>>> changeset: 6930:97c99bb43737
>>> user: Maxim Dounin <mdounin at mdounin.ru>
>>> date: Tue Mar 07 18:51:16 2017 +0300
>>> description:
>>> Introduced worker_shutdown_timeout.
>>>
>>> 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.
>>>
>>> diffstat:
>>>
>>> src/core/nginx.c | 9 ++++++
>>> src/core/ngx_cycle.c | 53 ++++++++++++++++++++++++++++++
>>> ++++++++++
>>> src/core/ngx_cycle.h | 2 +
>>> src/os/unix/ngx_process_cycle.c | 1 +
>>> src/os/win32/ngx_process_cycle.c | 1 +
>>> 5 files changed, 66 insertions(+), 0 deletions(-)
>>>
>>> diffs (148 lines):
>>>
>>> diff --git a/src/core/nginx.c b/src/core/nginx.c
>>> --- a/src/core/nginx.c
>>> +++ b/src/core/nginx.c
>>> @@ -124,6 +124,13 @@ static ngx_command_t ngx_core_commands[
>>> offsetof(ngx_core_conf_t, rlimit_core),
>>> NULL },
>>>
>>> + { ngx_string("worker_shutdown_timeout"),
>>> + NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_TAKE1,
>>> + ngx_conf_set_msec_slot,
>>> + 0,
>>> + offsetof(ngx_core_conf_t, shutdown_timeout),
>>> + NULL },
>>> +
>>> { ngx_string("working_directory"),
>>> NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_TAKE1,
>>> ngx_conf_set_str_slot,
>>> @@ -1014,6 +1021,7 @@ ngx_core_module_create_conf(ngx_cycle_t
>>> ccf->daemon = NGX_CONF_UNSET;
>>> ccf->master = NGX_CONF_UNSET;
>>> ccf->timer_resolution = NGX_CONF_UNSET_MSEC;
>>> + ccf->shutdown_timeout = NGX_CONF_UNSET_MSEC;
>>>
>>> ccf->worker_processes = NGX_CONF_UNSET;
>>> ccf->debug_points = NGX_CONF_UNSET;
>>> @@ -1042,6 +1050,7 @@ ngx_core_module_init_conf(ngx_cycle_t *c
>>> ngx_conf_init_value(ccf->daemon, 1);
>>> ngx_conf_init_value(ccf->master, 1);
>>> ngx_conf_init_msec_value(ccf->timer_resolution, 0);
>>> + ngx_conf_init_msec_value(ccf->shutdown_timeout, 0);
>>>
>>> ngx_conf_init_value(ccf->worker_processes, 1);
>>> ngx_conf_init_value(ccf->debug_points, 0);
>>> diff --git a/src/core/ngx_cycle.c b/src/core/ngx_cycle.c
>>> --- a/src/core/ngx_cycle.c
>>> +++ b/src/core/ngx_cycle.c
>>> @@ -15,6 +15,7 @@ static ngx_int_t ngx_init_zone_pool(ngx_
>>> ngx_shm_zone_t *shm_zone);
>>> static ngx_int_t ngx_test_lockfile(u_char *file, ngx_log_t *log);
>>> static void ngx_clean_old_cycles(ngx_event_t *ev);
>>> +static void ngx_shutdown_timer_handler(ngx_event_t *ev);
>>>
>>>
>>> volatile ngx_cycle_t *ngx_cycle;
>>> @@ -22,6 +23,7 @@ ngx_array_t ngx_old_cycles;
>>>
>>> static ngx_pool_t *ngx_temp_pool;
>>> static ngx_event_t ngx_cleaner_event;
>>> +static ngx_event_t ngx_shutdown_event;
>>>
>>> ngx_uint_t ngx_test_config;
>>> ngx_uint_t ngx_dump_config;
>>> @@ -1333,3 +1335,54 @@ ngx_clean_old_cycles(ngx_event_t *ev)
>>> ngx_old_cycles.nelts = 0;
>>> }
>>> }
>>> +
>>> +
>>> +void
>>> +ngx_set_shutdown_timer(ngx_cycle_t *cycle)
>>> +{
>>> + ngx_core_conf_t *ccf;
>>> +
>>> + ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
>>> ngx_core_module);
>>> +
>>> + if (ccf->shutdown_timeout) {
>>> + ngx_shutdown_event.handler = ngx_shutdown_timer_handler;
>>> + ngx_shutdown_event.data = cycle;
>>> + ngx_shutdown_event.log = cycle->log;
>>> + ngx_shutdown_event.cancelable = 1;
>>> +
>>> + ngx_add_timer(&ngx_shutdown_event, ccf->shutdown_timeout);
>>> + }
>>> +}
>>> +
>>> +
>>> +static void
>>> +ngx_shutdown_timer_handler(ngx_event_t *ev)
>>> +{
>>> + ngx_uint_t i;
>>> + ngx_cycle_t *cycle;
>>> + ngx_connection_t *c;
>>> +
>>> + cycle = ev->data;
>>> +
>>> + c = cycle->connections;
>>> +
>>> + for (i = 0; i < cycle->connection_n; i++) {
>>> +
>>> + if (c[i].fd == (ngx_socket_t) -1
>>> + || c[i].read == NULL
>>> + || c[i].read->accept
>>> + || c[i].read->channel
>>> + || c[i].read->resolver)
>>> + {
>>> + continue;
>>> + }
>>> +
>>> + ngx_log_debug1(NGX_LOG_DEBUG_CORE, ev->log, 0,
>>> + "*%uA shutdown timeout", c[i].number);
>>> +
>>> + c[i].close = 1;
>>> + c[i].error = 1;
>>> +
>>> + c[i].read->handler(c[i].read);
>>> + }
>>> +}
>>> diff --git a/src/core/ngx_cycle.h b/src/core/ngx_cycle.h
>>> --- a/src/core/ngx_cycle.h
>>> +++ b/src/core/ngx_cycle.h
>>> @@ -88,6 +88,7 @@ typedef struct {
>>> ngx_flag_t master;
>>>
>>> ngx_msec_t timer_resolution;
>>> + ngx_msec_t shutdown_timeout;
>>>
>>> ngx_int_t worker_processes;
>>> ngx_int_t debug_points;
>>> @@ -129,6 +130,7 @@ ngx_pid_t ngx_exec_new_binary(ngx_cycle_
>>> ngx_cpuset_t *ngx_get_cpu_affinity(ngx_uint_t n);
>>> ngx_shm_zone_t *ngx_shared_memory_add(ngx_conf_t *cf, ngx_str_t *name,
>>> size_t size, void *tag);
>>> +void ngx_set_shutdown_timer(ngx_cycle_t *cycle);
>>>
>>>
>>> extern volatile ngx_cycle_t *ngx_cycle;
>>> diff --git a/src/os/unix/ngx_process_cycle.c
>>> b/src/os/unix/ngx_process_cycle.c
>>> --- a/src/os/unix/ngx_process_cycle.c
>>> +++ b/src/os/unix/ngx_process_cycle.c
>>> @@ -763,6 +763,7 @@ ngx_worker_process_cycle(ngx_cycle_t *cy
>>>
>>> if (!ngx_exiting) {
>>> ngx_exiting = 1;
>>> + ngx_set_shutdown_timer(cycle);
>>> ngx_close_listening_sockets(cycle);
>>> ngx_close_idle_connections(cycle);
>>> }
>>> diff --git a/src/os/win32/ngx_process_cycle.c
>>> b/src/os/win32/ngx_process_cycle.c
>>> --- a/src/os/win32/ngx_process_cycle.c
>>> +++ b/src/os/win32/ngx_process_cycle.c
>>> @@ -800,6 +800,7 @@ ngx_worker_thread(void *data)
>>>
>>> if (!ngx_exiting) {
>>> ngx_exiting = 1;
>>> + ngx_set_shutdown_timer(cycle);
>>> ngx_close_listening_sockets(cycle);
>>> ngx_close_idle_connections(cycle);
>>> }
>>> _______________________________________________
>>> nginx-devel mailing list
>>> nginx-devel at nginx.org
>>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>>>
>>
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>>
>
>
>
> --
>
> *Datong Sun* | Systems Engineer
> datong at cloudflare.com
> <https://www.cloudflare.com/>
>
> 1 888 99 FLARE | www.cloudflare.com
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20170309/c83d6735/attachment-0001.html>
More information about the nginx-devel
mailing list