httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <trawi...@bellsouth.net>
Subject Re: New graceful restart mechanism.
Date Mon, 04 Jun 2001 17:02:10 GMT
<rbb@covalent.net> writes:

>                                    This could be improved a bit, by using
> OOB data to wake up the thread, and we would only check the pod if we
> received OOB data.  This is an optimization however, and I am not sure how
> that would work in real-life.

Err, I'm not sure either :)  On which connection would you send OOB
data?  How would we detect that there was OOB data?  If select(),  I
thought we were removing the need for select() (think
"SINGLE_LISTEN_UNSERIALIZED_ACCEPT")...   If SIGURG, I thought we were
avoiding signal-based processing (not sure of SIGURG portability
anyway)...  I guess I'm missing something real basic here...

>                               Assuming nobody complains, I will commit it
> tomorrow or the next day.

no real complaints, but no reason not to address the minor issues
below before committing

> Index: server/mpm_common.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/server/mpm_common.c,v
> retrieving revision 1.47
> diff -u -d -b -w -u -r1.47 mpm_common.c
> --- server/mpm_common.c	2001/05/16 20:51:28	1.47
> +++ server/mpm_common.c	2001/06/04 16:00:37
> @@ -338,4 +338,61 @@
>  }
>  #endif /* def NEED_INITGROUPS */
> 
> +#ifdef AP_MPM_USES_POD
> +static apr_file_t *pod_in = NULL, *pod_out = NULL;
> +
> +AP_DECLARE(apr_status_t) ap_mpm_pod_open(apr_pool_t *p)
> +{
> +    apr_file_t *pod;
> 
> +    pod = apr_file_pipe_create(&pod_in, &pod_out, p);
> +    apr_file_pipe_timeout_set(out, 0);
> +    return APR_SUCCESS;
> +}
> +
> +AP_DECLARE(apr_status_t) ap_mpm_pod_check(void)
> +{
> +    char c;
> +    apr_size_t len = 1;
> +    apr_status_t rv;
> +
> +    rv = apr_file_read(pod_in, &c, &len);
> +
> +    if ((rv == APR_SUCCESS) && (len == 1)) {
> +        return APR_SUCCESS;
> +    }
> +    return 1;
> +}

This feels like a boolean routine.  Why not int/0/non-zero instead of
apr_status_t/APR_SUCCESS/1?

> +AP_DECLARE(void) ap_mpm_pod_close(void)
> +{
> +    apr_file_close(pod_out);
> +    apr_file_close(pod_in);
> +}
> +
> +AP_DECLARE(void) ap_mpm_pod_signal(apr_pool_t *p)
> +{
> +    apr_socket_t *sock = NULL;
> +    apr_sockaddr_t *sa = NULL;

init to NULL not necessary... this may make some folks think it is
needed (as it is/was for some APR functions)

> +
> +    if ((rv = apr_file_write(pipe_of_death_out, &char_of_death, &one))
> +                                 != APR_SUCCESS) {
> +        if (APR_STATUS_IS_EINTR(rv)) continue;
> +            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, ap_server_conf,
> +                         "write pipe_of_death");
> +        }
> +    }
> +
> +    apr_sockaddr_info_get(&sa, "127.0.0.1", APR_UNSPEC, ap_listeners->sd->port,
0, p);
> +    apr_socket_create(&sock, sa->family, SOCK_STREAM, p);
> +    apr_connect(sock, sa);

very helpful to log errors from the last two calls, if only to know
without a shadow of a doubt when debugging a shutdown problem that
they worked 'cause otherwise you'd have seen the message in error_log

crucial to close the socket here so you don't run out of file
descriptors while executing the loop in ap_mpm_pod_killpg()... 

> Index: server/mpm/prefork/prefork.c

I didn't look your prefork changes in detail...

SIGHUP processing in child processes should be SIG_IGN and SA_RESTART,
right?

---------/----------

This feels like a reasonable the way to go but when this has been
brought up before I've had some general worries about it that perhaps
folks can dispel.  (Maybe I'm just mad because when I got the great
idea to use connect() it wouldn't do what I wanted it to do :) )

I remain at least a little concerned about using connect() because 

a) weird firewall-like rules on some possibly-weird systems may keep
   the connect() from working; admin needs to be aware

b) TCP/IP may be dead/dying and socket()+connect() won't work

   I'm not sure how to kill apache on OS/390 when the stack goes away.

   I was told previously by you and possibly others that I couldn't
   teach the child process to tell the parent to get the @#$% out
   because the stack is dead.  If the stack is dead, the user can tell
   apache to go away but nothing will happen and the child processes
   will (probably) continue looping and getting weird errors on
   select() or accept(). 

   This needs to be solved.  For me, simply recognizing the magic
   error feedback and using the magic error status (CHILD_EXITFATAL?)
   is sufficient.

c) We've invented a nice utility for MPMs but it won't work for MPMs
   which want to parent process to decide which child should go away.
   (I realize that isn't a current requirement.)

Thanks!
-- 
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...


Mime
View raw message