You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ed Korthof <ed...@apache.org> on 1999/10/29 16:02:41 UTC
[PATCH] reclaim_child_processes in 1.3.x
Hi --
Here's a short patch to fix what may be a bug in 1.3.x; at least, it looks
like a bug to me. The current situation is that Apache will wait a long
time before issuing SIGKILL to wayward children; this means that if you
must restart the server using SIGTERM (for example, because third party
code leaks on restarts), you may wind up waiting a long time.
Up above (and in the code shown) waittime is set to 16384 (microseconds);
if the initial wait should be 16ms as the comments indicate, then the
first change is required. All the other changes are just to correct the
comments to be exact (because I'm anal).
This shouldn't generally be an issue; but I have seen it get triggered
with the right set of third party modules. Without changes here, Apache
frequently takes 20 seconds to shut down; during that time, the server
being restarted doesn't serve traffic. That kinda sucks.
I know current development is in the 2.0 branch, and if this doesn't get
in, I won't mind; it's just that it does look like a bug. [It wouldn't
hurt to also put in a short fixed sleep before trying to reap the child
processes, to give them a chance to die after the SIGKILL; but that'd be a
larger change, which might as well stay out of the older code.]
Ed, hoping to spend some time working with the 2.0 code in the near future
*****
--- http_main.c 1999/10/29 07:47:57 1.480
+++ http_main.c 1999/10/29 13:07:35
@@ -2385,8 +2385,8 @@
*/
tv.tv_sec = waittime / 1000000;
tv.tv_usec = waittime % 1000000;
- waittime = waittime * 4;
ap_select(0, NULL, NULL, NULL, &tv);
+ waittime = waittime * 4;
/* now see who is done */
not_dead_yet = 0;
@@ -2403,10 +2403,10 @@
}
++not_dead_yet;
switch (tries) {
- case 1: /* 16ms */
- case 2: /* 82ms */
+ case 1: /* 16.384ms */
+ case 2: /* 65.536ms */
break;
- case 3: /* 344ms */
+ case 3: /* 262.144ms */
/* perhaps it missed the SIGHUP, lets try again */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
server_conf,
@@ -2415,11 +2415,11 @@
kill(pid, SIGHUP);
waittime = 1024 * 16;
break;
- case 4: /* 16ms */
- case 5: /* 82ms */
- case 6: /* 344ms */
+ case 4: /* 16.384ms */
+ case 5: /* 65.536ms */
+ case 6: /* 262.144ms */
break;
- case 7: /* 1.4sec */
+ case 7: /* 1.048576 sec */
/* ok, now it's being annoying */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
server_conf,
@@ -2427,14 +2427,14 @@
pid);
kill(pid, SIGTERM);
break;
- case 8: /* 6 sec */
+ case 8: /* 4.194304 sec */
/* die child scum */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
"child process %d still did not exit, sending a SIGKILL",
pid);
kill(pid, SIGKILL);
break;
- case 9: /* 14 sec */
+ case 9: /* 16.777216 sec */
/* gave it our best shot, but alas... If this really
* is a child we are trying to kill and it really hasn't
* exited, we will likely fail to bind to the port
******
Re: [PATCH] reclaim_child_processes in 1.3.x
Posted by Ed Korthof <ed...@apache.org>.
Opps. Please ignore the original patch; it's clear that it was born out
of insufficient sleep. The problem isn't the placement of the change to
waittime (as should have been obvious) -- the problem is the fact that it
doesn't do a small sleep between SIGKILL and trying to reap the processes.
That's problematic, but it's something which we can patch locally; if
you're interested, here's a patch which does what seem more or less
minimal changes to accomplish the goal:
*****
--- http_main.c 1999/10/29 07:47:57 1.480
+++ http_main.c 1999/10/29 16:33:00
@@ -2377,9 +2377,9 @@
#endif
ap_sync_scoreboard_image();
- for (tries = terminate ? 4 : 1; tries <= 9; ++tries) {
+ for (tries = terminate ? 4 : 1; tries <= 14; ++tries) {
/* don't want to hold up progress any more than
* necessary, but we need to allow children a few moments to exit.
* Set delay with an exponential backoff.
*/
@@ -2402,40 +2402,47 @@
continue;
}
++not_dead_yet;
switch (tries) {
- case 1: /* 16ms */
- case 2: /* 82ms */
+ case 1: /* 16.384ms */
+ case 2: /* 65.536ms */
break;
- case 3: /* 344ms */
+ case 3: /* 262.144ms */
/* perhaps it missed the SIGHUP, lets try again */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
server_conf,
"child process %d did not exit, sending another SIGHUP",
pid);
kill(pid, SIGHUP);
waittime = 1024 * 16;
break;
- case 4: /* 16ms */
- case 5: /* 82ms */
- case 6: /* 344ms */
+ case 4: /* 16.384ms */
+ case 5: /* 65.536ms */
+ case 6: /* 262.144ms */
break;
- case 7: /* 1.4sec */
+ case 7: /* 1.048576 sec */
/* ok, now it's being annoying */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
server_conf,
"child process %d still did not exit, sending a SIGTERM",
pid);
kill(pid, SIGTERM);
break;
- case 8: /* 6 sec */
+ case 8: /* 4.194304 sec */
/* die child scum */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
"child process %d still did not exit, sending a SIGKILL",
pid);
kill(pid, SIGKILL);
+ waittime= 1024 *16;
break;
- case 9: /* 14 sec */
+ case 9: /* 16.384ms */
+ case 10: /* 65.536ms */
+ case 11: /* 262.144ms */
+ case 12: /* 1.048576 sec */
+ case 13: /* 4.194304 sec */
+ break;
+ case 14: /* 16.777216 sec */
/* gave it our best shot, but alas... If this really
* is a child we are trying to kill and it really hasn't
* exited, we will likely fail to bind to the port
* after the restart.
*****
This is a slightly larger change; it means Apache may linger on for a few
extra seconds in a really bad state -- but it was already waiting 16
seconds in that case. (IMO, there's no point in waiting that long anyway,
but ... <shrug>)
Regardless, this does solve the problem which I've been seeing. Sending a
SIGKILL, checking right away, and then waiting 16 seconds -- that seems
silly, since (unless the machine is in a bad state) SIGKILL is pretty
reliable.
thanks --
Ed
On Fri, 29 Oct 1999, Ed Korthof wrote:
> Hi --
>
> Here's a short patch to fix what may be a bug in 1.3.x; at least, it looks
> like a bug to me. The current situation is that Apache will wait a long
> time before issuing SIGKILL to wayward children; this means that if you
> must restart the server using SIGTERM (for example, because third party
> code leaks on restarts), you may wind up waiting a long time.
>
> Up above (and in the code shown) waittime is set to 16384 (microseconds);
> if the initial wait should be 16ms as the comments indicate, then the
> first change is required. All the other changes are just to correct the
> comments to be exact (because I'm anal).
>
> This shouldn't generally be an issue; but I have seen it get triggered
> with the right set of third party modules. Without changes here, Apache
> frequently takes 20 seconds to shut down; during that time, the server
> being restarted doesn't serve traffic. That kinda sucks.
>
> I know current development is in the 2.0 branch, and if this doesn't get
> in, I won't mind; it's just that it does look like a bug. [It wouldn't
> hurt to also put in a short fixed sleep before trying to reap the child
> processes, to give them a chance to die after the SIGKILL; but that'd be a
> larger change, which might as well stay out of the older code.]
>
> Ed, hoping to spend some time working with the 2.0 code in the near future
>
> *****
> --- http_main.c 1999/10/29 07:47:57 1.480
> +++ http_main.c 1999/10/29 13:07:35
> @@ -2385,8 +2385,8 @@
> */
> tv.tv_sec = waittime / 1000000;
> tv.tv_usec = waittime % 1000000;
> - waittime = waittime * 4;
> ap_select(0, NULL, NULL, NULL, &tv);
> + waittime = waittime * 4;
>
> /* now see who is done */
> not_dead_yet = 0;
> @@ -2403,10 +2403,10 @@
> }
> ++not_dead_yet;
> switch (tries) {
> - case 1: /* 16ms */
> - case 2: /* 82ms */
> + case 1: /* 16.384ms */
> + case 2: /* 65.536ms */
> break;
> - case 3: /* 344ms */
> + case 3: /* 262.144ms */
> /* perhaps it missed the SIGHUP, lets try again */
> ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
> server_conf,
> @@ -2415,11 +2415,11 @@
> kill(pid, SIGHUP);
> waittime = 1024 * 16;
> break;
> - case 4: /* 16ms */
> - case 5: /* 82ms */
> - case 6: /* 344ms */
> + case 4: /* 16.384ms */
> + case 5: /* 65.536ms */
> + case 6: /* 262.144ms */
> break;
> - case 7: /* 1.4sec */
> + case 7: /* 1.048576 sec */
> /* ok, now it's being annoying */
> ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
> server_conf,
> @@ -2427,14 +2427,14 @@
> pid);
> kill(pid, SIGTERM);
> break;
> - case 8: /* 6 sec */
> + case 8: /* 4.194304 sec */
> /* die child scum */
> ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
> "child process %d still did not exit, sending a SIGKILL",
> pid);
> kill(pid, SIGKILL);
> break;
> - case 9: /* 14 sec */
> + case 9: /* 16.777216 sec */
> /* gave it our best shot, but alas... If this really
> * is a child we are trying to kill and it really hasn't
> * exited, we will likely fail to bind to the port
> ******
>
>
>
>
>