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/09/10 02:49:56 UTC

[PATCH] (take 1) reliable piped logs

Ok, this patch contains two new APIs.  I'll start off by describing them.

register_other_child -- this lets you register a child for the parent to
maintain, and you include a callback that the parent uses to inform you of
various events.  (i.e. child died, child was lost (bogosity occured),
a restart has been requested, or an unregister event was performed).

Hacked into the register_other_child api is a write-fd monitor -- a
select is performed occasionally to see if the write-fd is still writable.
If it's ever not writable, an "unwritable" event is passed to the callback.

Then there's the piped_log api itself, which uses the above api to implement
a piped log which can respawn its log program.  It will respawn when a
restart happens (either kind), or if the logger child dies.  mod_log_config
has been tweaked to use this new API.

The theory here is that someone will write a nice log processing script
that does log rotation, vhost splitting, and maybe even parallel DNS
resolution.  Whatever.  My test case was "|cat >>/tmp/access_log".  The
point is that this script alone should deal with the vagaries of
site-specific logging, which keeps Apache's code simple (and lets you
do things that Apache can't, like asynchronous DNS).

What has been tested?

- kill the log child
- kill -HUP the server
- kill -USR1 the server

That's it.  Pretty lame of me, eh?  The following still needs to be
tested/investigated:

- let the log child "back up", i.e. stop reading, and make sure the parent
    notices and restarts it

- perhaps we need another process group for the log child because otherwise
    it will receive SIGHUPs and SIGUSR1s like the rest of the children
    during a restart ... we could dictate the logger needs to ignore those
    signals, or whatever.  This part of the API is undefined as of yet.
    Note that not only does the logger get these signals, but the
    maintenance callback gets the restart event, and you'll notice the
    piped_log_maintenance function issues a SIGTERM in this case.

- make sure I didn't royally screw up reclaim_child_processes

- maybe restore the reap_children() code, but it needs to grok
    other_children ... or just ditch the one architecture that needs it.

- while 1; kill -HUP and kill -USR1 storms

- ap_slack the pipe()d descriptors ... maybe close the read descriptor in
    httpd children -- I took a "why bother" attitude since I expect only
    one TransferLog per apache invocation, I expect the splitting to be
    done within the logger.

- port the piped_log code to OS2 and WIN32.  I honestly could not use
    spawn_child to do this, I had to do it by hand.  The reason is that
    spawn_child always creates new pipe()s and the piped_log code needs
    to re-use the same pipe.

- it would probably be helpful if $SERVER_ROOT and such were available to
    the logger.

Help?  I've been sick this week on top of just moving to a new place,
and having spent a week away from work.  So I won't be able to polish
this off like I like to.

Dean

Index: main/http_log.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.c,v
retrieving revision 1.31
diff -u -r1.31 http_log.c
--- http_log.c	1997/08/31 22:14:07	1.31
+++ http_log.c	1997/09/10 00:46:44
@@ -63,6 +63,7 @@
 #include "http_config.h"
 #include "http_core.h"
 #include "http_log.h"
+#include "http_main.h"
 
 #include <stdarg.h>
 
@@ -317,4 +318,148 @@
 #else
     exit(1);
 #endif
+}
+
+/* reliable piped log support */
+
+/* forward declaration */
+static void piped_log_maintenance (int reason, void *data, int status);
+
+static int piped_log_spawn (piped_log *pl)
+{
+    int pid;
+
+    block_alarms();
+    pid = fork();
+    if (pid == 0) {
+	/* XXX: this needs porting to OS2 and WIN32 */
+	/* XXX: need to check what open fds the logger is actually passed,
+	 * XXX: and CGIs for that matter ... cleanup_for_exec *should*
+	 * XXX: close all the relevant stuff, but hey, it could be broken. */
+	/* we're now in the child */
+	close (STDIN_FILENO);
+	dup2 (pl->fds[0], STDIN_FILENO);
+
+	cleanup_for_exec ();
+	signal (SIGCHLD, SIG_DFL);	/* for HPUX */
+	signal (SIGHUP, SIG_IGN);
+	execl (SHELL_PATH, SHELL_PATH, "-c", pl->program, NULL);
+	fprintf (stderr,
+	    "piped_log_spawn: unable to exec %s -c '%s': %s\n",
+	    SHELL_PATH, pl->program, strerror (errno));
+	exit (1);
+    }
+    if (pid == -1) {
+	fprintf (stderr,
+	    "piped_log_spawn: unable to fork(): %s\n", strerror (errno));
+	unblock_alarms ();
+	return -1;
+    }
+    unblock_alarms();
+    pl->pid = pid;
+    register_other_child (pid, piped_log_maintenance, pl, pl->fds[1]);
+    return 0;
+}
+
+
+static void piped_log_maintenance (int reason, void *data, int status)
+{
+    piped_log *pl = data;
+    int pid;
+
+    switch (reason) {
+    case OC_REASON_DEATH:
+    case OC_REASON_LOST:
+	pl->pid = -1;
+	unregister_other_child (pl);
+	if (pl->program == NULL) {
+	    /* during a restart */
+	    break;
+	}
+	if (piped_log_spawn (pl) == -1) {
+	    /* what can we do?  This could be the error log we're having
+	     * problems opening up... */
+	    fprintf (stderr,
+		"piped_log_maintenance: unable to respawn '%s': %s\n",
+		pl->program, strerror (errno));
+	}
+	break;
+    
+    case OC_REASON_UNWRITABLE:
+	if (pl->pid != -1) {
+	    kill (pl->pid, SIGTERM);
+	}
+	break;
+    
+    case OC_REASON_RESTART:
+	pl->program = NULL;
+	if (pl->pid != -1) {
+	    kill (pl->pid, SIGTERM);
+	}
+	break;
+
+    case OC_REASON_UNREGISTER:
+	break;
+    }
+}
+
+
+static void piped_log_cleanup (void *data)
+{
+    piped_log *pl = data;
+
+    if (pl->pid != -1) {
+	kill (pl->pid, SIGTERM);
+    }
+    unregister_other_child (pl);
+    close (pl->fds[0]);
+    close (pl->fds[1]);
+}
+
+
+static void piped_log_cleanup_for_exec (void *data)
+{
+    piped_log *pl = data;
+
+    close (pl->fds[0]);
+    close (pl->fds[1]);
+}
+
+
+API_EXPORT(piped_log *) open_piped_log (pool *p, const char *program)
+{
+    piped_log *pl;
+    int pid;
+
+    pl = palloc (p, sizeof (*pl));
+    pl->p = p;
+    pl->program = pstrdup (p, program);
+    pl->pid = -1;
+    block_alarms ();
+    if (pipe (pl->fds) == -1) {
+	int save_errno = errno;
+	unblock_alarms();
+	errno = save_errno;
+	return NULL;
+    }
+    register_cleanup (p, pl, piped_log_cleanup, piped_log_cleanup_for_exec);
+    if (piped_log_spawn (pl) == -1) {
+	int save_errno = errno;
+	kill_cleanup (p, pl, piped_log_cleanup);
+	close (pl->fds[0]);
+	close (pl->fds[1]);
+	unblock_alarms ();
+	errno = save_errno;
+	return NULL;
+    }
+    unblock_alarms ();
+    return pl;
+}
+
+API_EXPORT(void) close_piped_log (piped_log *pl)
+{
+    block_alarms ();
+    piped_log_cleanup (pl);
+    kill_cleanup (pl->p, pl, piped_log_cleanup);
+    unblock_alarms ();
 }
Index: main/http_log.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_log.h,v
retrieving revision 1.12
diff -u -r1.12 http_log.h
--- http_log.h	1997/08/27 05:45:37	1.12
+++ http_log.h	1997/09/10 00:46:44
@@ -79,3 +79,14 @@
 API_EXPORT(void) log_reason(const char *reason, const char *fname,
 			    request_rec *r);
 
+typedef struct piped_log {
+    pool *p;
+    char *program;
+    int pid;
+    int fds[2];
+} piped_log;
+
+API_EXPORT(piped_log *) open_piped_log (pool *p, const char *program);
+API_EXPORT(void) close_piped_log (piped_log *);
+#define piped_log_read_fd(pl)	((pl)->fds[0])
+#define piped_log_write_fd(pl)	((pl)->fds[1])
Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.215
diff -u -r1.215 http_main.c
--- http_main.c	1997/09/03 21:50:54	1.215
+++ http_main.c	1997/09/10 00:46:48
@@ -234,6 +234,17 @@
 
 int one_process = 0;
 
+/* used to maintain list of children which aren't part of the scoreboard */
+typedef struct other_child_rec other_child_rec;
+struct other_child_rec {
+    other_child_rec *next;
+    int pid;
+    void (*maintenance)(int, void *, int);
+    void *data;
+    int write_fd;
+};
+static other_child_rec *other_children;
+
 pool *pconf;			/* Pool for config stuff */
 pool *ptrans;			/* Pool for per-transaction stuff */
 
@@ -1043,6 +1054,105 @@
 #endif /* ndef NO_LINGCLOSE */
 
 /*****************************************************************
+ * dealing with other children
+ */
+
+void register_other_child (int pid,
+    void (*maintenance)(int reason, void *, int status),
+    void *data, int write_fd)
+{
+    other_child_rec *ocr;
+
+    ocr = palloc (pconf, sizeof (*ocr));
+    ocr->pid = pid;
+    ocr->maintenance = maintenance;
+    ocr->data = data;
+    ocr->write_fd = write_fd;
+    ocr->next = other_children;
+    other_children = ocr;
+}
+
+/* note that since this can be called by a maintenance function while we're
+ * scanning the other_children list, all scanners should protect themself
+ * by loading ocr->next before calling any maintenance function.
+ */
+void unregister_other_child (void *data)
+{
+    other_child_rec **pocr, *nocr;
+
+    for (pocr = &other_children; *pocr; pocr = &(*pocr)->next) {
+	if ((*pocr)->data == data) {
+	    nocr = (*pocr)->next;
+	    (*(*pocr)->maintenance)(OC_REASON_UNREGISTER, (*pocr)->data, -1);
+	    *pocr = nocr;
+	    /* XXX: um, well we've just wasted some space in pconf ? */
+	    return;
+	}
+    }
+}
+
+/* test to ensure that the write_fds are all still writable, otherwise
+ * invoke the maintenance functions as appropriate */
+static void probe_writable_fds (void)
+{
+    fd_set writable_fds;
+    int fd_max;
+    other_child_rec *ocr, *nocr;
+    struct timeval tv;
+    int rc;
+
+    if (other_children == NULL) return;
+
+    fd_max = 0;
+    FD_ZERO (&writable_fds);
+    for (ocr = other_children; ocr; ocr = ocr->next) {
+	if (ocr->write_fd == -1) continue;
+	FD_SET (ocr->write_fd, &writable_fds);
+	if (ocr->write_fd > fd_max) {
+	    fd_max = ocr->write_fd;
+	}
+    }
+    if (fd_max == 0) return;
+
+    do {
+	tv.tv_sec = 0;
+	tv.tv_usec = 0;
+	rc = ap_select (fd_max + 1, NULL, &writable_fds, NULL, &tv);
+    } while (rc == -1 && errno == EINTR);
+
+    if (rc == -1) {
+	/* XXX: uhh this could be really bad, we could have a bad file
+	 * descriptor due to a bug in one of the maintenance routines */
+	log_unixerr ("probe_writable_fds", "select", "could not probe
+	    writable fds", server_conf);
+	return;
+    }
+    if (rc == 0) return;
+
+    for (ocr = other_children; ocr; ocr = nocr) {
+	nocr = ocr->next;
+	if (ocr->write_fd == -1) continue;
+	if (FD_ISSET (ocr->write_fd, &writable_fds)) continue;
+	(*ocr->maintenance)(OC_REASON_UNWRITABLE, ocr->data, -1);
+    }
+}
+
+/* possibly reap an other_child, return 0 if yes, -1 if not */
+static int reap_other_child (int pid, int status)
+{
+    other_child_rec *ocr, *nocr;
+
+    for (ocr = other_children; ocr; ocr = nocr) {
+	nocr = ocr->next;
+	if (ocr->pid != pid) continue;
+	ocr->pid = -1;
+	(*ocr->maintenance)(OC_REASON_DEATH, ocr->data, status);
+	return 0;
+    }
+    return -1;
+}
+
+/*****************************************************************
  *
  * Dealing with the scoreboard... a lot of these variables are global
  * only to avoid getting clobbered by the longjmp() that happens when
@@ -1599,105 +1709,110 @@
 {
 #ifndef MULTITHREAD
     int i, status;
+    long int waittime = 4096; /* in usecs */
+    struct timeval tv;
+    int waitret, tries;
+    int not_dead_yet;
+    other_child_rec *ocr, *nocr;
 
     sync_scoreboard_image();
-    for (i = 0; i < max_daemons_limit; ++i) {
-	int pid = scoreboard_image->servers[i].x.pid;
 
-	if (pid != my_pid && pid != 0) { 
-	    int waitret = 0,
-		tries = start_tries;
-
-	    while (waitret == 0 && tries <= 4) {
-		long int waittime = 4096; /* in usecs */
-		struct timeval tv;
-	    
-		/* don't want to hold up progress any more than 
-		 * necessary, so keep checking to see if the child
-		 * has exited with an exponential backoff.
-		 * Currently set for a maximum wait of a bit over
-		 * four seconds.
-		 */
-		while (((waitret = waitpid(pid, &status, WNOHANG)) == 0) &&
-			 waittime < 3000000) {
-		       tv.tv_sec = waittime / 1000000;
-		       tv.tv_usec = waittime % 1000000;
-		       waittime = waittime * 2;
-		       ap_select(0, NULL, NULL, NULL, &tv);
-		}
-		if (waitret == 0) {
-		    switch (tries) {
-		    case 1:
-			/* perhaps it missed the SIGHUP, lets try again */
-			aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
-				    "child process %d did not exit, sending another SIGHUP",
-				    pid);
-			kill(pid, SIGHUP);
-			break;
-		    case 2:
-			/* ok, now it's being annoying */
-			aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
-				    "child process %d still did not exit, sending a SIGTERM",
-				    pid);
-			kill(pid, SIGTERM);
-			break;
-		    case 3:
-			/* die child scum */
-			aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
-				    "child process %d still did not exit, sending a SIGKILL",
-				    pid);
-			kill(pid, SIGKILL);
-			break;
-		    case 4:
-			/* 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.
-			 */
-			aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
-				    "could not make child process %d exit, attempting to continue anyway",
-				    pid);
-			break;
-		    }
-		}
-		tries++;
+    tries = 0;
+    for(tries = start_tries; tries < 4; ++tries) {
+	/* don't want to hold up progress any more than 
+	* necessary, but we need to allow children a few moments to exit.
+	* delay with an exponential backoff.
+	* Currently set for a maximum wait of a bit over
+	* four seconds.
+	*/
+	tv.tv_sec = waittime / 1000000;
+	tv.tv_usec = waittime % 1000000;
+	waittime = waittime * 2;
+	ap_select(0, NULL, NULL, NULL, &tv);
+
+	/* now see who is done */
+	not_dead_yet = 0;
+	for (i = 0; i < max_daemons_limit; ++i) {
+	    int pid = scoreboard_image->servers[i].x.pid;
+
+	    if (pid == my_pid || pid == 0) continue;
+
+	    waitret = waitpid (pid, &status, WNOHANG);
+	    if (waitret == pid || waitret == -1) {
+		scoreboard_image->servers[i].x.pid = 0;
+		continue;
+	    }
+	    ++not_dead_yet;
+	    switch (tries) {
+	    case 1:
+		/* perhaps it missed the SIGHUP, lets try again */
+		aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
+		    "child process %d did not exit, sending another SIGHUP",
+		    pid);
+		kill(pid, SIGHUP);
+		break;
+	    case 2:
+		/* ok, now it's being annoying */
+		aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
+		    "child process %d still did not exit, sending a SIGTERM",
+		    pid);
+		kill(pid, SIGTERM);
+		break;
+	    case 3:
+		/* die child scum */
+		aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
+		    "child process %d still did not exit, sending a SIGKILL",
+		    pid);
+		kill(pid, SIGKILL);
+		break;
+	    case 4:
+		/* 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.
+		    */
+		aplog_error(APLOG_MARK, APLOG_ERR, server_conf,
+		    "could not make child process %d exit, "
+		    "attempting to continue anyway", pid);
+		break;
 	    }
 	}
-    }
-#endif /* ndef MULTITHREAD */
-}
-
-#if defined(BROKEN_WAIT) || defined(NEED_WAITPID)
-/*
-Some systems appear to fail to deliver dead children to wait() at times.
-This sorts them out. In fact, this may have been caused by a race condition
-in wait_or_timeout(). But this routine is still useful for systems with no
-waitpid().
-*/
-int reap_children (void)
-{
-    int status, n;
-    int ret = 0;
-
-    for (n = 0; n < max_daemons_limit; ++n) {
-	if (scoreboard_image->servers[n].status != SERVER_DEAD
-		&& waitpid (scoreboard_image->servers[n].x.pid, &status, WNOHANG)
-		    == -1
-		&& errno == ECHILD) {
-	    sync_scoreboard_image ();
-	    update_child_status (n, SERVER_DEAD, NULL);
-	    ret = 1;
+	for (ocr = other_children; ocr; ocr = nocr) {
+	    nocr = ocr->next;
+	    if (ocr->pid == -1) continue;
+
+	    waitret = waitpid (ocr->pid, &status, WNOHANG);
+	    if (waitret == ocr->pid) {
+		ocr->pid = -1;
+		(*ocr->maintenance)(OC_REASON_DEATH, ocr->data, status);
+	    } else if (waitret == 0) {
+		(*ocr->maintenance)(OC_REASON_RESTART, ocr->data, -1);
+		++not_dead_yet;
+	    } else if (waitret == -1) {
+		/* uh what the heck? they didn't call unregister? */
+		ocr->pid = -1;
+		(*ocr->maintenance)(OC_REASON_LOST, ocr->data, -1);
+	    }
+	}
+	if (!not_dead_yet) {
+	    /* nothing left to wait for */
+	    break;
 	}
     }
-    return ret;
+#endif /* ndef MULTITHREAD */
 }
-#endif
 
 /* Finally, this routine is used by the caretaker process to wait for
  * a while...
  */
 
-static int wait_or_timeout (void)
+/* number of calls to wait_or_timeout between writable probes */
+#ifndef INTERVAL_OF_WRITABLE_PROBES
+#define INTERVAL_OF_WRITABLE_PROBES 10
+#endif
+static int wait_or_timeout_counter;
+
+static int wait_or_timeout (int *status)
 {
 #ifdef WIN32
 #define MAXWAITOBJ MAXIMUM_WAIT_OBJECTS
@@ -1734,21 +1849,20 @@
 
 #else /* WIN32 */
     struct timeval tv;
-#ifndef NEED_WAITPID
     int ret;
 
-    ret = waitpid (-1, NULL, WNOHANG);
+    ++wait_or_timeout_counter;
+    if (wait_or_timeout_counter == INTERVAL_OF_WRITABLE_PROBES) {
+	wait_or_timeout_counter = 0;
+	probe_writable_fds();
+    }
+    ret = waitpid (-1, status, WNOHANG);
     if (ret == -1 && errno == EINTR) {
 	return -1;
     }
     if (ret > 0) {
 	return ret;
     }
-#else
-    if (reap_children ()) {
-	return -1;
-    }
-#endif
     tv.tv_sec = SCOREBOARD_MAINTENANCE_INTERVAL / 1000000;
     tv.tv_usec = SCOREBOARD_MAINTENANCE_INTERVAL % 1000000;
     ap_select(0, NULL, NULL, NULL, &tv);
@@ -2003,6 +2117,10 @@
     return (suexec_enabled);
 }
 
+/*****************************************************************
+ * Connection structures and accounting...
+ */
+
 /* This hashing function is designed to get good distribution in the cases
  * where the server is handling entire "networks" of servers.  i.e. a
  * whack of /24s.  This is probably the most common configuration for
@@ -3171,7 +3289,8 @@
 
 	while (!restart_pending && !shutdown_pending) {
 	    int child_slot;
-	    int pid = wait_or_timeout();
+	    int status;
+	    int pid = wait_or_timeout (&status);
 
 	    /* XXX: if it takes longer than 1 second for all our children
 	     * to start up and get into IDLE state then we may spawn an
@@ -3195,6 +3314,8 @@
 			/* don't perform idle maintenance yet */
 			continue;
 		    }
+		} else if (reap_other_child (pid, status) == 0) {
+		    /* handled */
 		} else if (is_graceful) {
 		    /* Great, we've probably just lost a slot in the
 		     * scoreboard.  Somehow we don't know about this
Index: main/http_main.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.h,v
retrieving revision 1.16
diff -u -r1.16 http_main.h
--- http_main.h	1997/07/21 05:53:46	1.16
+++ http_main.h	1997/09/10 00:46:49
@@ -96,3 +96,40 @@
 void time_process_request (int child_num, int status);
 unsigned int set_callback_and_alarm(void (*fn)(int), int x);
 int check_alarm(void);
+
+/*
+ * register an other_child -- a child which the main loop keeps track of
+ * and knows it is different than the rest of the scoreboard.
+ *
+ * pid is the pid of the child.
+ *
+ * maintenance is a function that is invoked with a reason, the data
+ * pointer passed here, and when appropriate a status result from waitpid().
+ *
+ * write_fd is an fd that is probed for writing by select() if it is ever
+ * unwritable, then maintenance is invoked with reason OC_REASON_UNWRITABLE.
+ * This is useful for log pipe children, to know when they've blocked.  To
+ * disable this feature, use -1 for write_fd.
+ */
+API_EXPORT(void) register_other_child (int pid,
+    void (*maintenance)(int reason, void *data, int status), void *data,
+    int write_fd);
+#define OC_REASON_DEATH		0	/* child has died, caller must call
+                                         * unregister still */
+#define OC_REASON_UNWRITABLE	1	/* write_fd is unwritable */
+#define OC_REASON_RESTART	2	/* a restart is occuring, perform
+					 * any necessary cleanup (including
+					 * sending a special signal to child)
+					 */
+#define OC_REASON_UNREGISTER	3	/* unregister has been called, do
+					 * whatever is necessary (including
+					 * kill the child) */
+#define OC_REASON_LOST		4	/* somehow the child exited without
+					 * us knowing ... buggy os? */
+
+/*
+ * unregister an other_child.  Note that the data pointer is used here, and
+ * is assumed to be unique per other_child.  This is because the pid and
+ * write_fd are possibly killed off separately.
+ */
+API_EXPORT(void) unregister_other_child (void *data);
Index: modules/standard/mod_log_config.c
===================================================================
RCS file: /export/home/cvs/apachen/src/modules/standard/mod_log_config.c,v
retrieving revision 1.36
diff -u -r1.36 mod_log_config.c
--- mod_log_config.c	1997/08/23 02:59:45	1.36
+++ mod_log_config.c	1997/09/10 00:46:49
@@ -164,6 +164,7 @@
 #include "httpd.h"
 #include "http_config.h"
 #include "http_core.h" /* For REMOTE_NAME */
+#include "http_log.h"
 
 module MODULE_VAR_EXPORT config_log_module;
 
@@ -680,32 +681,6 @@
 { NULL }
 };
 
-static int config_log_child (void *cmd)
-{
-    /* Child process code for 'TransferLog "|..."';
-     * may want a common framework for this, since I expect it will
-     * be common for other foo-loggers to want this sort of thing...
-     */
-    int child_pid = 1;
-
-    cleanup_for_exec();
-#ifdef SIGHUP
-    signal (SIGHUP, SIG_IGN);
-#endif
-#if defined(WIN32)
-    child_pid = spawnl (_P_NOWAIT, SHELL_PATH, SHELL_PATH, "/c", (char *)cmd, NULL);
-    return(child_pid);
-#elif defined(__EMX__)
-    /* For OS/2 we need to use a '/' */
-    execl (SHELL_PATH, SHELL_PATH, "/c", (char *)cmd, NULL);
-#else
-    execl (SHELL_PATH, SHELL_PATH, "-c", (char *)cmd, NULL);
-#endif
-    perror ("exec");
-    fprintf (stderr, "Exec of shell for logging failed!!!\n");
-    return(child_pid);
-}
-
 static config_log_state *open_config_log (server_rec *s, pool *p,
 				   config_log_state *cls,
 				   array_header *default_format) {
@@ -716,16 +691,13 @@
     }
 
     if (*cls->fname == '|') {
-        FILE *dummy;
-        
-        if (!spawn_child (p, config_log_child, (void *)(cls->fname+1),
-                    kill_after_timeout, &dummy, NULL)) {
-	    perror ("spawn_child");
-            fprintf (stderr, "Couldn't fork child for TransferLog process\n");
-            exit (1);
-        }
+	piped_log *pl;
 
-        cls->log_fd = fileno (dummy);
+	pl = open_piped_log (p, cls->fname + 1);
+	if (pl == NULL) {
+	    exit (1);
+	}
+	cls->log_fd = piped_log_write_fd (pl);
     }
     else {
         char *fname = server_root_relative (p, cls->fname);


Re: [PATCH] (take 1) reliable piped logs

Posted by Dean Gaudet <dg...@arctic.org>.
Well, since we're not forcing people to use this logger ... we can make it
do whatever we want.  If writing out-of-order logs works better (it does),
then so be it.  The log analysers will need to catch up with reality. 
It's possible right now for a log to be written out of order.  It's just
by luck that it doesn't happen frequently.  Consider:  a task can call
time(0), get a result, then finish its time slice, the next second comes
along, another child calls time(0) and manages to write() before losing
its slice, then the first task gets awaked and does a write() ... out of
order log. 

Dean

On Wed, 10 Sep 1997, Elizabeth Mattijsen wrote:

> At 18:04 9-09-97 -0700, Dean wrote:
> >    - DNS lookups occuring asynchronously with the hit
> A note of caution.  Whoever is going to write a generic logger with
> asynchronous DNS lookups, may have to worry about writing logfiles in the
> order in which the requests were handled, not in which the IP-numbers were
> resolved.  Many statistics programs do not handle not chronologically
> ordered logfiles correctly (logfiles in which a request "later" in the
> logfile has an "earlier" timestamp).
> 
> 
> Elizabeth Mattijsen
> xxLINK Internet Services
> 


Re: [PATCH] (take 1) reliable piped logs

Posted by Elizabeth Mattijsen <li...@xxLINK.nl>.
At 18:04 9-09-97 -0700, Dean wrote:
>    - DNS lookups occuring asynchronously with the hit
A note of caution.  Whoever is going to write a generic logger with
asynchronous DNS lookups, may have to worry about writing logfiles in the
order in which the requests were handled, not in which the IP-numbers were
resolved.  Many statistics programs do not handle not chronologically
ordered logfiles correctly (logfiles in which a request "later" in the
logfile has an "earlier" timestamp).


Elizabeth Mattijsen
xxLINK Internet Services

Re: [PATCH] (take 1) reliable piped logs

Posted by Dean Gaudet <dg...@arctic.org>.
Uh, I doubt we'll ever have a fixed API, this is why we have #defines for 
you to conditionally compile against.  I know we miss updating them sometimes
during development but we try never to miss them on releases...

But I think in this case you and me are miscommunicating.  I am under
the impression that your module behaves like this:  While running in the
parent (presumably the module's init function) you fork.  You fork again
to disassociate yourself from the parent's process group.  In this new
process you set up shop, which includes setting up a new pool.

In this case I'd say that you're completely within the API calling
make_sub_pool(NULL) because you're on your own for cleaning up that new
process.  The only worry of calling make_sub_pool(NULL) is that it needs
to be cleaned up some time... so normal modules operating within Apache's
memory space (i.e. within the parent or within the httpd children) shouldn't
be using it.  For example, this was a bug in mod_rewrite which I fixed at
revision 1.28 (replacing a make_sub_pool(NULL) call with a make_sub_pool(p)
to make it a subpool of the pconf pool).

Essentially I'm saying that make_sub_pool(NULL) currently *is* the API
initialization for processes using apache's resource management outside
of apache itself ...  of course I'll probably regret saying this.  But
we'll revisit this because we want to make a libap.a which includes all
of Apache's utility routines, and resource management definately falls
into that category.  In that case we need to abstract out some of the
initialization and such.

Dean

On Mon, 22 Sep 1997, Stanley Gambarin wrote:

> 
> 
> On Mon, 22 Sep 1997, Dean Gaudet wrote:
> > 
> > > > > inline pool *mem_init() 
> > > > > {
> > > > > 	return (make_sub_pool(NULL));
> > > > > }
> > > > > inline void memory_destroy(pool *p) 
> > > > > {
> > > > > 	destroy_pool(p);
> > > > > }
> > > > > 
> > > > > which would allow external modules to use  Apache memory pool stuff,
> > > > > without any sideeffects.  If pepople are interested, say "Arg" and
> > 
> > I don't see why it's a problem for you to call make_sub_pool(NULL) ...
> > since you're in a separate process.  This is what the API allows you to
> > do.  Or am I still missing something?
> > 
> > Dean
> > 
> > 
> 	It maybe my misunderstanding, but I thought that the whole
> idea behind API was to isolate module developer from the internal
> implementation.  I do realize that I can call the function myself, 
> however, should the pool handling change, it will force
> incompatibility across modules, which really sucks.  I am currently
> running in the same problem, where the changes are made to the 
> server forcing source changes, resulting in maintaining of multiple
> copies of the same module for each version of Apache server.  I do
> realize that some of the changes are necessary, however, it does 
> not make my life any easier (this is why a clear and fixed API is
> a necessity).  Also, if all the system() functions are to be 
> replaced by the ap_ or os_ counterparts, this would also make my life
> a bit easier (as it will avoid stuff like just happened with ap_md5())
> which forced yet another copy of the module to maintain.  Providing
> some sort of abstraction layer for ALL system functions used in 
> Apache would be a nice addition (especially for module developers)
> Just some of my whining...
> 					Stanley.
> 
> 


Re: [PATCH] (take 1) reliable piped logs

Posted by Stanley Gambarin <st...@cs.bu.edu>.

On Mon, 22 Sep 1997, Dean Gaudet wrote:
> 
> > > > inline pool *mem_init() 
> > > > {
> > > > 	return (make_sub_pool(NULL));
> > > > }
> > > > inline void memory_destroy(pool *p) 
> > > > {
> > > > 	destroy_pool(p);
> > > > }
> > > > 
> > > > which would allow external modules to use  Apache memory pool stuff,
> > > > without any sideeffects.  If pepople are interested, say "Arg" and
> 
> I don't see why it's a problem for you to call make_sub_pool(NULL) ...
> since you're in a separate process.  This is what the API allows you to
> do.  Or am I still missing something?
> 
> Dean
> 
> 
	It maybe my misunderstanding, but I thought that the whole
idea behind API was to isolate module developer from the internal
implementation.  I do realize that I can call the function myself, 
however, should the pool handling change, it will force
incompatibility across modules, which really sucks.  I am currently
running in the same problem, where the changes are made to the 
server forcing source changes, resulting in maintaining of multiple
copies of the same module for each version of Apache server.  I do
realize that some of the changes are necessary, however, it does 
not make my life any easier (this is why a clear and fixed API is
a necessity).  Also, if all the system() functions are to be 
replaced by the ap_ or os_ counterparts, this would also make my life
a bit easier (as it will avoid stuff like just happened with ap_md5())
which forced yet another copy of the module to maintain.  Providing
some sort of abstraction layer for ALL system functions used in 
Apache would be a nice addition (especially for module developers)
Just some of my whining...
					Stanley.


Re: [PATCH] (take 1) reliable piped logs

Posted by Dean Gaudet <dg...@arctic.org>.

On Wed, 10 Sep 1997, Stanley Gambarin wrote:

> > > inline pool *mem_init() 
> > > {
> > > 	return (make_sub_pool(NULL));
> > > }
> > > inline void memory_destroy(pool *p) 
> > > {
> > > 	destroy_pool(p);
> > > }
> > > 
> > > which would allow external modules to use  Apache memory pool stuff,
> > > without any sideeffects.  If pepople are interested, say "Arg" and
> > > I will whip out some patches...
> > 
> > Can you explain why you need this?
> > 
> 
> 	When we create a new process group, i got a new process which 
> is completely unrelated to Apache (process manager in my case).  I would
> still like to use all Apache functions, however most of them need a pool
> as one of the arguments, which i currently do not have.  So, what I am
> forced to do in the module right now is to manually do make_sub_pool(NULL)
> since I need to call ap_md5 function which needs a pool.  Do you see
> my problem ?  Abstracting above would be ideal...

I don't see why it's a problem for you to call make_sub_pool(NULL) ...
since you're in a separate process.  This is what the API allows you to
do.  Or am I still missing something?

Dean


Re: [PATCH] (take 1) reliable piped logs

Posted by Stanley Gambarin <st...@cs.bu.edu>.

On Wed, 10 Sep 1997, Dean Gaudet wrote:

> > 	Sidenote enhancements: (i can probably workaround those, but)
> > (a) call_exec() function relies on the request_rec being passed to it...
> > what if i would like to exec a process not based on any request... i 
> > would have to duplicate the code, which i don't like to do.. so generic
> > call_exec would be nice  (right now request_rec structure is only used
> > for logging and setrlimit, so it would be easy to separate those into
> > a separate data structure)
> 
> Yeah this is needed by the piped log stuff too, you'll see I just
> duplicated the unix code 'cause I needed something at a lower level than
> spawn_child provides.
> 
> fastcgi spawning happens in the parent too, right?  Or could it?  'cause
> if you look at what I did in this patch I think you could use
> register_other_child to handle fastCGI tasks as well.
> 
	The actual sequence is  apache spawns procA, which fork/exec()s
procB, which is then used to fork/exec fastcgi apps.  Before it was :
apache spawns ProcB directly.. this was a problem, since upon termination
procB would recieve SIGKILL in 3 seconds, which sometime was not enough
to kill the fastcgi processes.  Current model does a workaround at the 
expense of extra process.  ProcB just periodically checks if it has a 
parent and terminates all children and self if it does not (i.e. procA 
died).  Separating procB into a separate process group would be ideal,
as iwould not need extra process in between (which is costly).

> I'm a newbie at process groups, so I can't suggest an API at the moment. 

	Ideally, it would be an extra function, which does fork() and then
before doing func() it would call setpgrp(), so it would be very similar
to what spawn_child() does now... The only problem is that the new process
group should not have any attachments to the previous... those include
signals (which i believe is handled by OS), pools and other stuff.

> > inline pool *mem_init() 
> > {
> > 	return (make_sub_pool(NULL));
> > }
> > inline void memory_destroy(pool *p) 
> > {
> > 	destroy_pool(p);
> > }
> > 
> > which would allow external modules to use  Apache memory pool stuff,
> > without any sideeffects.  If pepople are interested, say "Arg" and
> > I will whip out some patches...
> 
> Can you explain why you need this?
> 

	When we create a new process group, i got a new process which 
is completely unrelated to Apache (process manager in my case).  I would
still like to use all Apache functions, however most of them need a pool
as one of the arguments, which i currently do not have.  So, what I am
forced to do in the module right now is to manually do make_sub_pool(NULL)
since I need to call ap_md5 function which needs a pool.  Do you see
my problem ?  Abstracting above would be ideal...

						Stanley.


Re: [PATCH] (take 1) reliable piped logs

Posted by Dean Gaudet <dg...@arctic.org>.

On Tue, 9 Sep 1997, Stanley Gambarin wrote:

> 
> 
> On Tue, 9 Sep 1997, Dean Gaudet wrote:
> 
> > 
> > - perhaps we need another process group for the log child because otherwise
> >     it will receive SIGHUPs and SIGUSR1s like the rest of the children
> >     during a restart ... we could dictate the logger needs to ignore those
> >     signals, or whatever.  This part of the API is undefined as of yet.
> >     Note that not only does the logger get these signals, but the
> >     maintenance callback gets the restart event, and you'll notice the
> >     piped_log_maintenance function issues a SIGTERM in this case.
> > 
> 	
> 	Having API to allow for a separate process group would be a big
> advantage.. mod_fastcgi needs this functionality in a huge way, since 
> Apache signals mess up the whole processing, as well as 3-second SIGTERM/
> SIGKILL is a real killer (sorry, coult not resists), resulting in a bunch
> of orphan processes (some can't die in 3 seconds).
> 
> 	Sidenote enhancements: (i can probably workaround those, but)
> (a) call_exec() function relies on the request_rec being passed to it...
> what if i would like to exec a process not based on any request... i 
> would have to duplicate the code, which i don't like to do.. so generic
> call_exec would be nice  (right now request_rec structure is only used
> for logging and setrlimit, so it would be easy to separate those into
> a separate data structure)

Yeah this is needed by the piped log stuff too, you'll see I just
duplicated the unix code 'cause I needed something at a lower level than
spawn_child provides.

fastcgi spawning happens in the parent too, right?  Or could it?  'cause
if you look at what I did in this patch I think you could use
register_other_child to handle fastCGI tasks as well.

I'm a newbie at process groups, so I can't suggest an API at the moment. 

> (b) abstract memory pool even further...  mod_fastcgi does some funky
> stuff and needs memory pools, however, i can't use the pools from request
> or config.. i need to create my own pool... I hate doing
> make_sub_pool(NULL), so if someone can do the following:
> 
> inline pool *mem_init() 
> {
> 	return (make_sub_pool(NULL));
> }
> 
> and
> 
> inline void memory_destroy(pool *p) 
> {
> 	destroy_pool(p);
> }
> 
> which would allow external modules to use  Apache memory pool stuff,
> without any sideeffects.  If pepople are interested, say "Arg" and
> I will whip out some patches...

Can you explain why you need this?

Dean


Re: [PATCH] (take 1) reliable piped logs

Posted by Stanley Gambarin <st...@cs.bu.edu>.

On Tue, 9 Sep 1997, Dean Gaudet wrote:

> 
> - perhaps we need another process group for the log child because otherwise
>     it will receive SIGHUPs and SIGUSR1s like the rest of the children
>     during a restart ... we could dictate the logger needs to ignore those
>     signals, or whatever.  This part of the API is undefined as of yet.
>     Note that not only does the logger get these signals, but the
>     maintenance callback gets the restart event, and you'll notice the
>     piped_log_maintenance function issues a SIGTERM in this case.
> 
	
	Having API to allow for a separate process group would be a big
advantage.. mod_fastcgi needs this functionality in a huge way, since 
Apache signals mess up the whole processing, as well as 3-second SIGTERM/
SIGKILL is a real killer (sorry, coult not resists), resulting in a bunch
of orphan processes (some can't die in 3 seconds).

	Sidenote enhancements: (i can probably workaround those, but)
(a) call_exec() function relies on the request_rec being passed to it...
what if i would like to exec a process not based on any request... i 
would have to duplicate the code, which i don't like to do.. so generic
call_exec would be nice  (right now request_rec structure is only used
for logging and setrlimit, so it would be easy to separate those into
a separate data structure)

(b) abstract memory pool even further...  mod_fastcgi does some funky
stuff and needs memory pools, however, i can't use the pools from request
or config.. i need to create my own pool... I hate doing
make_sub_pool(NULL), so if someone can do the following:

inline pool *mem_init() 
{
	return (make_sub_pool(NULL));
}

and

inline void memory_destroy(pool *p) 
{
	destroy_pool(p);
}

which would allow external modules to use  Apache memory pool stuff,
without any sideeffects.  If pepople are interested, say "Arg" and
I will whip out some patches...
							Stanley



Re: [PATCH] (take 1) reliable piped logs

Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 9 Sep 1997, Dean Gaudet wrote:

> The theory here is that someone will write a nice log processing script
> that does log rotation, vhost splitting, and maybe even parallel DNS
> resolution.  Whatever.  My test case was "|cat >>/tmp/access_log".  The
> point is that this script alone should deal with the vagaries of
> site-specific logging, which keeps Apache's code simple (and lets you
> do things that Apache can't, like asynchronous DNS).

I want to emphasize this more.  piped logs solve at least the following
problems: 

    - thousands of vhosts with log splitting in real-time, without the need
	for thousands of descriptors in apache (thousands of descriptors
	cost you when you fork a CGI and have to close them all, for example).

    - DNS lookups occuring asynchronously with the hit

    - rotation of logs *without stopping the server* -- i.e. the logger can
	start writing somewhere else, the server doesn't know or care.  This
	eliminates any problems with USR1.  In fact, it means the server itself
	only needs to be restarted for configuration changes.

    - filtering of hits which are logged

I think that the ideal logger would:

    - setuid itself to another uid/gid, different than the webserver (it
	starts off as root)

    - setrlimit its descriptors to the max available

    - split logs based on the first word of each line, and maintain a cache
	of currently open logs (so that it's not limited at all by the number
	of descriptors)

    - spawn its own children and use them to resolve DNS on the fly, a la
	squid's DNS children.  (This is a portable solution.)

    - include regex matching of lines to be logged/not-logged

    - die gracefully when given a SIGTERM (i.e. stop reading the log pipe
	and flush all its output buffers, and die FAST)

    - be written in C (perl is fine if you have cpu to waste, but you probably
	don't have cpu to waste)

    - be included with apache-1.3, or be available on apache.org ;)

The only thing that held me back from suggesting this before is that Apache
wasn't able to recover well from a dead logging child.

Notice that such a program feeding off stdin when combined with something
like netcat or djb's tcpclient/tcpserver gives you a method of logging
remotely fairly robustly.

Dean