cyrus configuration reload without service interuption

Alain Spineux aspineux at gmail.com
Wed Jan 9 15:59:28 EST 2008


Here is my first patch proposal with still the debugging lines.
I will release a more clean in few time.


diff -ur cyrus-imapd-2.3.10.orig/imap/signals.c
cyrus-imapd-2.3.10.asx/imap/signals.c
--- cyrus-imapd-2.3.10.orig/imap/signals.c      2006-11-30
18:11:20.000000000 +0100
+++ cyrus-imapd-2.3.10.asx/imap/signals.c       2008-01-09
17:27:14.000000000 +0100
@@ -54,7 +54,7 @@

 static void sighandler(int sig)
 {
-    /* syslog(LOG_DEBUG, "got signal %d", sig); */
+    syslog(LOG_DEBUG, "got signal %d", sig);
     gotsignal = sig;
 }

@@ -79,12 +79,18 @@
        fatal("unable to install signal handler for %d: %m", SIGALRM);
     }

+    /* 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

     for (i = 0; catch[i] != 0; i++) {
-       if (catch[i] != SIGALRM && sigaction(catch[i], &action, NULL) < 0) {
+       if (catch[i] != SIGALRM && (!alarm || catch[i] != SIGHUP) &&
sigaction(catch[i], &action, NULL) < 0) {
            fatal("unable to install signal handler for %d: %m", catch[i]);
        }
     }
diff -ur cyrus-imapd-2.3.10.orig/master/master.c
cyrus-imapd-2.3.10.asx/master/master.c
--- cyrus-imapd-2.3.10.orig/master/master.c     2007-10-10
15:59:48.000000000 +0200
+++ cyrus-imapd-2.3.10.asx/master/master.c      2008-01-09
18:23:11.000000000 +0100
@@ -512,7 +512,7 @@
        limit_fds(256);

        get_prog(path, sizeof(path), cmd);
-       syslog(LOG_DEBUG, "about to exec %s", path);
+       syslog(LOG_DEBUG, "about to exec ASX1 %s", path);
        execv(path, cmd);
        syslog(LOG_ERR, "can't exec %s for startup: %m", path);
        exit(EX_OSERR);
@@ -638,7 +638,7 @@
        }
        limit_fds(s->maxfds);

-       syslog(LOG_DEBUG, "about to exec %s", path);
+       syslog(LOG_DEBUG, "about to exec ASX2 %s", path);

        /* add service name to environment */
        snprintf(name_env, sizeof(name_env), "CYRUS_SERVICE=%s", s->name);
@@ -741,7 +741,7 @@
                limit_fds(256);

                get_prog(path, sizeof(path), a->exec);
-               syslog(LOG_DEBUG, "about to exec %s", path);
+               syslog(LOG_DEBUG, "about to exec ASX3 %s", path);
                execv(path, a->exec);
                syslog(LOG_ERR, "can't exec %s on schedule: %m", path);
                exit(EX_OSERR);
@@ -1568,7 +1568,27 @@
                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;
+                }
+            }
+
        }
+
     }

     /* remove existing events */
diff -ur cyrus-imapd-2.3.10.orig/master/service.c
cyrus-imapd-2.3.10.asx/master/service.c
--- cyrus-imapd-2.3.10.orig/master/service.c    2007-09-27
22:10:39.000000000 +0200
+++ cyrus-imapd-2.3.10.asx/master/service.c     2008-01-09
12:47:19.000000000 +0100
@@ -432,7 +432,9 @@
            }

            if (soctype == SOCK_STREAM) {
+                syslog(LOG_DEBUG, "ASX in accept");
                fd = accept(LISTEN_FD, NULL, NULL);
+                syslog(LOG_DEBUG, "ASX out accept fd=%d, errorno=%d,
signal=%d", fd, errno, signals_poll());
                if (fd < 0) {
                    switch (errno) {
                    case ENETDOWN:
@@ -460,7 +462,12 @@
                            notify_master(STATUS_FD,
MASTER_SERVICE_UNAVAILABLE);
                        service_abort(EX_OSERR);
                    }
-               }
+                } else {
+                     /* fd >= 0 */
+                     /* we dont want SIGHUP to disrupt a connection before
+                      * the end */
+                     signals_add_handlers(0);
+                }
            } else {
                /* udp */
                struct sockaddr_storage from;
@@ -486,6 +493,7 @@

        if (fd < 0 && (signals_poll() || newfile)) {
            /* timed out (SIGALRM), SIGHUP, or new process file */
+            syslog(LOG_DEBUG, "ASX bye");
            if (MESSAGE_MASTER_ON_EXIT)
                notify_master(STATUS_FD, MASTER_SERVICE_UNAVAILABLE);
            service_abort(0);
@@ -541,6 +549,7 @@

        if (signals_poll() || use_count >= max_use) {
            /* caught SIGHUP or exceeded max use count */
+            syslog(LOG_DEBUG, "ASX bye 2");
            break;
        }




On Jan 9, 2008 11:47 AM, Alain Spineux <aspineux at gmail.com> wrote:
> On Jan 9, 2008 1:38 AM, Ken Murchison <murch at andrew.cmu.edu> wrote:
> > Alain Spineux wrote:
> > > Hi
> > >
> > > When cyrmaster receive a SIGHUP, it reload its configuration file,
> > > stop/start the the required services
> > > and schedule new "EVENTS".
> > > But running imapd, pop3d ... keep running until they "expire" (timout
> > > waiting for a new connection, or reach
> > > the maximum number of connection).
> > >
> > > For example, if you need to replace your certificate or added new
> > > cyrus admins to imapd.conf , you need to make a restart,
> > > (this mean a disconnection, of already connect users) if you want to
> > > use the new stuff immediately.
> > >
> > > I would like to provide a patch to avoid this.
> > >
> > > Here is the idea:
> > > When cyrmaster get a SIGHUP, It does as before, but send a SIGHUP to
> > > all imapd, popd process too. The sig_handler
> > > of these processes will  update their connection counter to a value
> > > above the limit and then will no accept new connection and
> > > then will shutdown!
> >
> > The services already check to see if their binaries have changed out
> > from under them, and do as you explain above.  Simply touch'ing the
> > binaries will have the same effect.
>
> Yes this is exactly what I want !
>
> >
> > I supposed we could also have the services check for a new imapd.conf.
>
> Touching the binary is not very friendly for distribution that keep
> size, time, .... info about
> all files in every packages, they will report them as changed!
> Touching the configuration file is more recommended.
>
> Anyway the existing system is not working as expected!
> The check is done only after the end of a connection and
> before if fall into an accept() ! If the changes appends when the
> service is blocked in the accept,
> the service handle one more connection before to detect the change!
> This is true for signals too, because the handler use
> action.sa_flags |= SA_RESTART; (at least in linux)
> and then the "accept()" doesn't terminate. BUT then the service become sensible
> to a second signal and will terminate (because of the use of
> action.sa_flags |= SA_RESETHAND;)
> , this is maybe a BUG, if this append in the middle of a connection ?
>
> The SIGALRM doen't suffer this probleme because SA_RESTART is not set for it !
> Maybe cyrus should avoid the use of SA_RESTART for HUP too
>
>
>
>
>
> The solution is to send it a SIGHUP to force the accept to fail.
> Anyway the SIGHUP is not working !
>
> >
> > --
> > Kenneth Murchison
> > Systems Programmer
> > Project Cyrus Developer/Maintainer
> > Carnegie Mellon University
> >
>
>
>
>
> --
> 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