patch to force cyrus to reload its configuration in a more rigorous way
Alain Spineux
aspineux at gmail.com
Sun Apr 6 11:28:27 EDT 2008
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 at gmail.com> wrote:
> On Wed, Apr 2, 2008 at 10:30 AM, Simon Matter <simon.matter at invoca.ch> 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
More information about the Cyrus-devel
mailing list