newsyslog(8) can send signals to the process ids contained in the PID files, per newsyslog.conf configuration files. Unfortunately a separate signal is sent for every configuration line. IMHO it would be better to send a signal for every process, covering the situation that a process may have multiple log files open that will all be closed when the signal is received. One example might be an Apache server with many virtual hosts, each of which writes into two separate logfiles (access.log and error.log), viz: /var/log/virt1-httpd-access.log 644 7 * @T00 ZB /var/run/httpd.pid 30 /var/log/virt1-httpd-error.log 644 7 * @T00 ZB /var/run/httpd.pid 30 /var/log/virt2-httpd-access.log 644 7 * @T00 ZB /var/run/httpd.pid 30 /var/log/virt2-httpd-error.log 644 7 * @T00 ZB /var/run/httpd.pid 30 /var/log/virt3-httpd-access.log 644 7 * @T00 ZB /var/run/httpd.pid 30 /var/log/virt3-httpd-error.log 644 7 * @T00 ZB /var/run/httpd.pid 30 This will currently restart Apache six (!) times when logfiles are due for rotation. As there is a delay of 10 seconds after each signal to allow the daemon to idle down, a longer newslog.conf file may easily take many minutes to complete. Fix: This patch will delay sending signals and logfile compression until the entire newslog.conf file has been processed. All lines not involving signals are precessed immediately. Then, we send one signal (unique per PID and per signal number, hence it is possible to send different signals to one PID), we wait 10 seconds and finally we compress the old logs. This patch was meant to minimize changes to newsyslog.c. Of course the structure and logic should be reorganized to include this in the source in a more orthogonal fashion.
Here's an updated patch for newsyslog.c. Applies to 1.25.2.5, but should work as well for 1.25.2.6 and 1.41. Can we please commit this useful stuff? Where can I apply for committer status? Helge --- newsyslog/newsyslog.c.ORIG Thu Mar 7 23:56:15 2002 +++ newsyslog/newsyslog.c Fri Apr 12 17:28:32 2002 @@ -85,6 +85,23 @@ struct conf_entry *next;/* Linked list pointer */ }; +struct log_entry { + char *log; /* name of the log to be compressed */ + char bz; /* gzip or bzip2 */ + struct log_entry* next; /* Linked list pointer */ +}; + +struct kill_entry { + pid_t pid; /* PID to kill */ + int sig; /* Signal to send */ + struct kill_entry* next;/* Linked list pointer */ +}; + +struct kill_entry* kill_pending = NULL; + /* List of PIDs to be killed */ +struct log_entry* log_pending = NULL; + /* List of logs to be compressed */ + int archtodir = 0; /* Archive old logfiles to other directory */ int verbose = 0; /* Print out what's going on */ int needroot = 1; /* Root privs are necessary */ @@ -103,6 +120,8 @@ static char *sob(char *p); static char *son(char *p); static char *missing_field(char *p, char *errline); +static int save_kill(pid_t pid, int sig); +static void save_compress_log(char* file, char bz); static void do_entry(struct conf_entry * ent); static void PRS(int argc, char **argv); static void usage(void); @@ -124,6 +143,9 @@ main(int argc, char **argv) { struct conf_entry *p, *q; + struct kill_entry* k; + struct log_entry* l; + int notified; PRS(argc, argv); if (needroot && getuid() && geteuid()) @@ -136,6 +158,34 @@ free((char *) q); q = p; } + + notified = 0; + while (kill_pending) { + if (kill(kill_pending->pid, kill_pending->sig)) + warn("can't notify daemon, pid %d", (int) kill_pending->pid); + else { + notified = 1; + if (verbose) + printf("daemon pid %d notified\n", (int) kill_pending->pid); + } + k = kill_pending; + kill_pending = kill_pending->next; + free((char *) k); + } + if (notified) { + if (verbose) + printf("small pause to allow daemons to close logs\n"); + sleep(10); + } + + while (log_pending) { + (log_pending->bz ? bzcompress_log : compress_log)(log_pending->log); + free(log_pending->log); + l = log_pending; + log_pending = log_pending->next; + free((char *) l); + } + return (0); } @@ -505,6 +555,39 @@ return (p); } +static int +save_kill(pid_t pid, int sig) +{ + struct kill_entry* p; + + for (p = kill_pending; p != NULL; p = p->next) + if (p->pid == pid && p->sig == sig) + return (0); + + p = (struct kill_entry *) malloc(sizeof(struct kill_entry)); + p->pid = pid; + p->sig = sig; + p->next = kill_pending; + kill_pending = p; + return (0); +} + +static void +save_compress_log(char *file, char bz) +{ + struct log_entry* p; + + for (p = log_pending; p != NULL; p = p->next) + if (!strcmp(p->log, file)) + return; + + p = (struct log_entry *) malloc(sizeof(struct log_entry)); + p->log = strdup(file); + p->bz = bz; + p->next = log_pending; + log_pending = p; +} + static void dotrim(char *log, const char *pid_file, int numdays, int flags, int perm, int owner_uid, int group_gid, int sig) @@ -669,12 +752,12 @@ if (noaction) { notified = 1; printf("kill -%d %d\n", sig, (int) pid); - } else if (kill(pid, sig)) + } else if (save_kill(pid, sig)) warn("can't notify daemon, pid %d", (int) pid); else { notified = 1; if (verbose) - printf("daemon pid %d notified\n", (int) pid); + printf("will notify daemon pid %d\n", (int) pid); } } if ((flags & CE_COMPACT) || (flags & CE_BZCOMPACT)) { @@ -685,23 +768,18 @@ else if (noaction) printf("Compress %s.0\n", log); else { - if (notified) { - if (verbose) - printf("small pause to allow daemon to close log\n"); - sleep(10); - } if (archtodir) { (void) snprintf(file1, sizeof(file1), "%s/%s", dirpart, namepart); if (flags & CE_COMPACT) - compress_log(file1); + if (pid) save_compress_log(file1, 0); else compress_log(file1); else if (flags & CE_BZCOMPACT) - bzcompress_log(file1); + if (pid) save_compress_log(file1, 1); else compress_log(file1); } else { if (flags & CE_COMPACT) - compress_log(log); + if (pid) save_compress_log(log, 0); else compress_log(log); else if (flags & CE_BZCOMPACT) - bzcompress_log(log); + if (pid) save_compress_log(log, 1); else compress_log(log); } } }
Responsible Changed From-To: freebsd-bugs->gad I will look into this as part of some other newsyslog changes that I am working on. I suspect I will implement it a different way than it is done in this PR (because there's some other reorganization I want to do), but I would definitely like to see it doing only one HUP per daemon.
State Changed From-To: open->patched I have committed a test-version of a change which implements this (among some other changes). I will let that sit in 5.x-current for awhile, and then make it the default behavior, and then MFC it into 4.x-stable. I imagine that will take a few weeks, as this is a big change and I want to be sure it has been sufficiently tested.
State Changed From-To: patched->closed This improvement was installed in 4.x-stable back on July 2, 2004. It seems to be working fine.