[nginx] Introduced worker_shutdown_timeout.

[Maxim Dounin]

Hello!

On Wed, Mar 08, 2017 at 03:52:15PM +0800, 洪志道 wrote:

> 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?

It is not expected to be called as long as there are no 
(non-cancelable) timers left and nginx can exit immediately.  It 
is only expected to be called when the timeout expires and nginx 
is still not able to exit.

> 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'.

No, this patch is wrong.  With this patch, worker process shutdown 
will be unconditionally delayed for the specified time.  It is not 
what it meant to do.  The worker_shutdown_timeout directive is not 
expected to delay shutdown if there are no active connections.  It 
was introduced to limit possible time spent in shutdown, that is, 
to ensure fast enough shutdown even if there are active 
connections.

-- 
Maxim Dounin
http://nginx.org/