On Jan 9, 2008 1:38 AM, Ken Murchison <murch 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
|
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 <aspineuxgmail.com> wrote:
> On Jan 9, 2008 1:38 AM, Ken Murchison <murchandrew.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
|
