httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject graceful restart revisited
Date Sat, 19 Apr 1997 05:51:09 GMT
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);


Mime
View raw message