I opened a new bug on bugzilla. https://bugzilla.andrew.cmu.edu/show_bug.cgi?id=2987 Alain On 4/2/08, Alain Spineux <aspineux@xxxxxxxxx> wrote: > On Wed, Apr 2, 2008 at 10:30 AM, Simon Matter <simon.matter@xxxxxxxxx> wrote: > > > Before this patch, cyrus was already able to detect a configuration > > > file change, > > > but only at the end of a connection. And any process waiting for a > > > new connection was ignoring the change during the next connection, > > > until they have finished. > > > In other word, after a configuration change, any waiting process can > > > still serve a new > > > connection with the old configuration and let the administrator > > > believe its change is not > > > working. > > > > > > My patch force cyrmaster to send a SIGHUP to all its children when it > > > detect a configuration change: > > > - any working process will ignore the SIGHUP when processing a > > > connection (what my patch does) > > > and at the end of the connection, it will detect the configuration > > > change and terminate > > > (this is what is doing the original code). > > > - any waiting process, will catch the SIGHUP and handle it like SIGALARM > > > (what my patch does) and make the process believe its waiting period > > > (-T option of imapd command) is expired and then terminate. (like was > > > doing the original code) > > > > Hi Alain, > > > > Thanks for the patch, I'll try it out and may include it in my rpms later. > > If I understand this correctly one drawback could be that on a heavy > > loaded server sending SIGHUP will resuilt in much more newly created > > processes after the SIGHUP has been sent. Could that be a problem somehow? > > > The only difference, is that without the patch, > any waiting process can still serve one connection before to terminate > and this during the timeout > period of 60sec (the cyrus default) > Then, it should not impact the load, this should not be measurable. > > Some more details, because your question was a little unclear : > > - After having made change, you have to SIGHUP cyrmaster only, like > before! Not all imapd, pop3, ... processes. > - When an inactive imapd nor pop3d, ... process get the SIGHUP, it > will terminate immediately. Their > is no reasons to terminate an active process. Active process, will > detect the configuartion change by temself > at the end of the connection and terminate. > - cyrmaster will start a new process when new connections arrive or to > match "prefork" in cyrus.conf. > > Regards > > > > > > > Simon > > > > > > > > > > > > Here is the patch and also in attachment. > > > > > > Regards > > > > > > --- cyrus-imapd-2.3.11/imap/signals.c.orig 2006-11-30 > > > 18:11:20.000000000 +0100 > > > +++ cyrus-imapd-2.3.11/imap/signals.c 2008-03-23 17:24:54.000000000 > > > +0100 > > > @@ -79,6 +79,12 @@ > > > fatal("unable to install signal handler for %d: %m", SIGALRM); > > > } > > > > > > + /* ASX: SIGHUP is used to force a configuration reload, and the > > > waiting > > > + * period is a privileged moment, so we don't set SA_RESTART */ > > > + if (alarm && sigaction(SIGHUP, &action, NULL) < 0) { > > > + fatal("unable to install signal handler for %d: %m", SIGHUP); > > > + } > > > + > > > #ifdef SA_RESTART > > > action.sa_flags |= SA_RESTART; > > > #endif > > > --- cyrus-imapd-2.3.11/master/master.c.orig 2007-10-10 > > > 15:59:48.000000000 +0200 > > > +++ cyrus-imapd-2.3.11/master/master.c 2008-03-23 17:29:33.000000000 > > > +0100 > > > @@ -1568,6 +1568,23 @@ > > > syslog(LOG_DEBUG, "init: service %s socket %d pipe %d %d", > > > Services[i].name, Services[i].socket, > > > Services[i].stat[0], Services[i].stat[1]); > > > + } else if (Services[i].exec && Services[i].socket) { > > > + /* refresh the old one */ > > > + syslog(LOG_DEBUG, "ASX SOMETHING TO DO service %s socket > > > %d pipe %d %d %d", > > > + Services[i].name, Services[i].socket, > > > + Services[i].stat[0], Services[i].stat[1], > > > + Services[i].exec ? 1:0); > > > + /* send SIGHUP to all children */ > > > + for (j = 0 ; j < child_table_size ; j++ ) { > > > + c = ctable[j]; > > > + while (c != NULL) { > > > + if ((c->si == i) && (c->service_state != > > > SERVICE_STATE_DEAD)) { > > > + syslog(LOG_DEBUG, "ASX send SIGHUP to %d", > > > c->pid); > > > + kill(c->pid, SIGHUP); > > > + } > > > + c = c->next; > > > + } > > > + } > > > } > > > } > > > > > > --- cyrus-imapd-2.3.11/master/service.c.orig 2007-09-27 > > > 22:10:39.000000000 +0200 > > > +++ cyrus-imapd-2.3.11/master/service.c 2008-03-23 17:32:46.000000000 > > > +0100 > > > @@ -460,7 +460,12 @@ > > > notify_master(STATUS_FD, > > > MASTER_SERVICE_UNAVAILABLE); > > > service_abort(EX_OSERR); > > > } > > > - } > > > + } else { > > > + /* fd >= 0 */ > > > + /* ASX we dont want SIGHUP to disrupt a connection > > > + * before the end */ > > > + signals_add_handlers(0); > > > + } > > > } else { > > > /* udp */ > > > struct sockaddr_storage from; > > > > > > > > > -- > > > Alain Spineux > > > aspineux gmail com > > > May the sources be with you > > > ---- > > > Cyrus Home Page: http://cyrusimap.web.cmu.edu/ > > > Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki > > > List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html > > > > > > > > > > > -- > > Alain Spineux > aspineux gmail com > May the sources be with you > -- Alain Spineux aspineux gmail com May the sources be with you ---- Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
