httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Plüm, Rüdiger, VF-Group" <ruediger.pl...@vodafone.com>
Subject Re: Graceful restart not so graceful?
Date Wed, 07 Jan 2009 12:43:46 GMT
 

> -----Ursprüngliche Nachricht-----
> Von: Jeff Trawick [mailto:trawick@gmail.com] 
> Gesendet: Mittwoch, 7. Januar 2009 13:03
> An: dev@httpd.apache.org
> Betreff: Re: Graceful restart not so graceful?
> 
> On Wed, Jan 7, 2009 at 6:03 AM, Joe Orton <jorton@redhat.com> wrote:
> 
> 
> 	On Tue, Jan 06, 2009 at 12:10:25PM -0600, William Rowe wrote:
> 	> Would folks comment on Nathan's, Joe's and Stefan's work on
> 	>
> 	> https://issues.apache.org/bugzilla/show_bug.cgi?id=42829
> 	>
> 	> and offer any comments on why this patch;
> 	>
> 	> https://issues.apache.org/bugzilla/attachment.cgi?id=22822
> 	>
> 	> should not be applied to trunk/ and branches/2.2.x/ 
> in time for
> 	> the next releases?
> 	
> 	
> 	I never found the time to commit this for a bunch of reasons:
> 	
> 	1) I never convinced myself that adding a bunch of complexity to
> 	prefork, per Stefan's patch, was the right way to address this
> 	
> 	2) I never worked out what impact the lack of check on the poll
> 	descriptor event type (POLLERR etc) had on this, but that needs
> 	resolving too
> 	
> 	3) I hadn't checked whether worker et al had similar issues
> 	
> 	4) I think that a simpler solution to this problem 
> would be to add a
> 	timeout (e.g. 30s) on the child pollset_poll() call so they are
> 	guaranteed to pop at some point, but I haven't had time 
> to try this or
> 	work out whether that will suck in some other way.
> 
> 
> Initial testing of your idea for a timeout was promising.  
> I'll try to test it with graceful stop as well.
> 
> Index: server/mpm/prefork/prefork.c
> ============================== 
> =====================================
> --- server/mpm/prefork/prefork.c    (revision 731724)
> +++ server/mpm/prefork/prefork.c    (working copy)
> @@ -540,10 +540,12 @@
>                  apr_int32_t numdesc;
>                  const apr_pollfd_t *pdesc;
>  
> -                /* timeout == -1 == wait forever */
> -                status = apr_pollset_poll(pollset, -1, 
> &numdesc, &pdesc);
> +                /* timeout == 10 seconds to avoid a hang at 
> graceful restart/stop
> +                 * caused by the closing of sockets by the 
> signal handler
> +                 */
> +                status = apr_pollset_poll(pollset, 
> apr_time_from_sec(10), &numdesc, &pdesc);
>                  if (status != APR_SUCCESS) {
> -                    if (APR_STATUS_IS_EINTR(status)) {
> +                    if (APR_STATUS_IS_TIMEUP(status) || 
> APR_STATUS_IS_EINTR(status)) {
>                          if (one_process && shutdown_pending) {
>                              return;
>                          }

Don't we need to add

SAFE_ACCEPT(accept_mutex_off());

before the continue later on in the 

if (APR_STATUS_IS_TIMEUP(status) || APR_STATUS_IS_EINTR(status)) {

block or is it guaranteed that if we allready have the lock
that another SAFE_ACCEPT(accept_mutex_on()); doesn't do any harm?

Regards

Rüdiger


Mime
View raw message