You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/04/19 07:51:09 UTC
graceful restart revisited
I've always wanted graceful restart to work... so I looked into it. And I
read the archives to figure out what the complaints about it were. I
worked together a patch which:
- blows away exising idle children immediately rather than wait for them
to cleanup on next check of generation. (Sends USR1 to the process
group, idle children use this to just_die)
- avoids problems where sending USR1 to a child can cause havoc (something
about causing a fork bomb didn't look at why...)
- avoids problems with multiple SIGUSR1s in quick succession
- eliminates a race condition in both graceful and SIGHUP restarts
This has only been tested on a linux box. I'll probably give it a whirl
on irix and solaris this weekend. It also hasn't been tested with
Listeners.
Doing this exposed two race conditions in our current SIGHUP support.
I describe them both in comments in the patch, one in set_signals and the
other in standalone_main.
This is not intended for 1.2.
Dean
Future:
I don't think we should actually blow away the scoreboard and
start a new one. I think that graceful_restart should maintain the
scoreboard, bump the generation, ditch the current idle children, and
start a bunch of new ones. Otherwise it's easy to get your system up
over the MaxClients point if you've got children doing long transfers
surviving across USR1. And I know of many webservers where having one
child more than MaxClients causes it to start swapping.
If the children were to flock( LOCK_SH ) all their log files then
it'd be possible to write a log rotate script which could accurately
know when all the children had finished logging. It would just have
to rename, kill -USR1, and then try to flock( LOCK_EX ).
Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_main.c,v
retrieving revision 1.137
diff -c -3 -r1.137 http_main.c
*** http_main.c 1997/04/12 04:24:57 1.137
--- http_main.c 1997/04/19 05:17:08
***************
*** 1173,1195 ****
#if 1
static int wait_or_timeout(int *status)
! {
#ifndef NEED_WAITPID
int ret;
ret=waitpid(-1,status,WNOHANG);
! if(ret <= 0)
! {
sleep(1);
return -1;
! }
return ret;
#else
if(!reap_children())
sleep(1);
return -1;
#endif
! }
#else
--- 1173,1194 ----
#if 1
static int wait_or_timeout(int *status)
! {
#ifndef NEED_WAITPID
int ret;
ret=waitpid(-1,status,WNOHANG);
! if(ret <= 0) {
sleep(1);
return -1;
! }
return ret;
#else
if(!reap_children())
sleep(1);
return -1;
#endif
! }
#else
***************
*** 1420,1426 ****
}
void graceful_restart()
! {
scoreboard_image->global.exit_generation=generation;
is_graceful=1;
update_scoreboard_global();
--- 1419,1427 ----
}
void graceful_restart()
! {
! signal (SIGALRM, SIG_IGN);
! alarm (0);
scoreboard_image->global.exit_generation=generation;
is_graceful=1;
update_scoreboard_global();
***************
*** 1429,1435 ****
#else
siglongjmp(restart_buffer,1);
#endif
! }
void set_signals()
{
--- 1430,1436 ----
#else
siglongjmp(restart_buffer,1);
#endif
! }
void set_signals()
{
***************
*** 1450,1455 ****
--- 1451,1464 ----
sa.sa_handler=(void (*)())sig_term;
if(sigaction(SIGTERM,&sa,NULL) < 0)
log_unixerr("sigaction(SIGTERM)", NULL, NULL, server_conf);
+
+ /* wait_or_timeout uses sleep() which could deliver a SIGALRM just as
+ * we're trying to process the restart requests. That's not good.
+ * restart and graceful_restart both clean out the SIGALRM handler,
+ * but this totally avoids the race condition between when the restart
+ * request is made and when the handler is invoked.
+ */
+ sigaddset (&sa.sa_mask, SIGALRM);
sa.sa_handler=(void (*)())restart;
if(sigaction(SIGHUP,&sa,NULL) < 0)
log_unixerr("sigaction(SIGHUP)", NULL, NULL, server_conf);
***************
*** 1715,1720 ****
--- 1724,1735 ----
BUFF *conn_io;
request_rec *r;
+ /* Prepare to receive a SIGUSR1 due to graceful restart so that
+ * we can exit cleanly. Since we're between connections right
+ * now it's the right time to exit, but we might be blocked in a
+ * system call when the graceful restart request is made. */
+ signal (SIGUSR1, (void (*)())just_die);
+
/*
* (Re)initialize this child to a pre-connection state.
*/
***************
*** 1798,1803 ****
--- 1813,1825 ----
}
}
+ /* We've got a connection in progress, ignore USR1 graceful restarts.
+ * Yep there's a tiny race condition between when we accept()d the
+ * connection above, and when we do this. But this is something we
+ * probably have to live with.
+ */
+ signal (SIGUSR1, SIG_IGN);
+
accept_mutex_off(); /* unlock after "accept" */
note_cleanups_for_fd(ptrans,csd);
***************
*** 2071,2076 ****
--- 2093,2099 ----
++generation;
signal (SIGHUP, SIG_IGN); /* Until we're done (re)reading config */
+ signal (SIGUSR1, SIG_IGN);
if(!one_process && !is_graceful)
{
***************
*** 2082,2094 ****
log_unixerr ("killpg SIGHUP", NULL, NULL, server_conf);
}
! if(is_graceful)
! {
/* USE WITH EXTREME CAUTION. Graceful restarts are known to break */
/* problems will be dealt with in a future release */
log_error("SIGUSR1 received. Doing graceful restart",server_conf);
kill_cleanups_for_fd(pconf,sd);
}
else if (sd != -1 || listenmaxfd != -1) {
reclaim_child_processes(); /* Not when just starting up */
log_error ("SIGHUP received. Attempting to restart", server_conf);
--- 2105,2124 ----
log_unixerr ("killpg SIGHUP", NULL, NULL, server_conf);
}
! if(is_graceful) {
/* USE WITH EXTREME CAUTION. Graceful restarts are known to break */
/* problems will be dealt with in a future release */
log_error("SIGUSR1 received. Doing graceful restart",server_conf);
kill_cleanups_for_fd(pconf,sd);
+ if (!one_process) {
+ #ifndef NO_KILLPG
+ if (killpg(pgrp, SIGUSR1) < 0) /* kill off the idle ones */
+ #else
+ if (kill(-pgrp, SIGUSR1) < 0)
+ #endif
+ log_unixerr ("killpg SIGUSR1", NULL, NULL, server_conf);
}
+ }
else if (sd != -1 || listenmaxfd != -1) {
reclaim_child_processes(); /* Not when just starting up */
log_error ("SIGHUP received. Attempting to restart", server_conf);
***************
*** 2158,2164 ****
while (1) {
int status, child_slot;
int pid = wait_or_timeout(&status);
!
if (pid >= 0) {
/* Child died... note that it's gone in the scoreboard. */
sync_scoreboard_image();
--- 2188,2211 ----
while (1) {
int status, child_slot;
int pid = wait_or_timeout(&status);
!
! /* graceful_restart: If we've done a graceful restart we killed off
! * all the old idle children and they will need reaping. But we
! * don't know *when* they'll need reaping... and it's very likely
! * that some of them will be reaped the moment we get back into
! * this loop. But at that point all of our new servers may not have
! * started, and the count_idle_server() check below will report that
! * we don't have enough idle servers. We would then erroneously start
! * another server. So instead we check if the pid we got back was
! * in our current generation of children, and if not we refuse to
! * use this event to start up another child.
! *
! * XXX: Something similar to the above happens at server startup and
! * at SIGHUP time -- if it takes more than a second for all the
! * StartServers children to start up we will spawn an extra child
! * here.
! */
! child_slot = -1; /* not necessary, but -Wall safe */
if (pid >= 0) {
/* Child died... note that it's gone in the scoreboard. */
sync_scoreboard_image();
***************
*** 2170,2176 ****
}
sync_scoreboard_image();
! if ((count_idle_servers() < daemons_min_free)
&& (child_slot = find_free_child_num()) >= 0
&& child_slot < daemons_limit) {
Explain1("Starting new child in slot %d",child_slot);
--- 2217,2224 ----
}
sync_scoreboard_image();
! if ( !(pid >= 0 && child_slot == -1)
! && (count_idle_servers() < daemons_min_free)
&& (child_slot = find_free_child_num()) >= 0
&& child_slot < daemons_limit) {
Explain1("Starting new child in slot %d",child_slot);