Graceful restart also for imapd.conf
gnb at fastmail.fm
Mon Aug 8 22:32:28 EDT 2011
On 08/08/11 19:39, Olivier ROLAND wrote:
> Hi there,
> Since version 2.0.9 Cyrus master can re-read /etc/cyrus.conf on SIGHUP.
> The man say :
> Master rereads its configuration file when it receives a hangup signal,
> SIGHUP. Services and events may be added, deleted or modified when the
> configuration file is reread. Any active services removed from the
> configuration file will be allowed to run until completion.
> Looking at the code I wonder why we don't do the same to re-reread
> Actually, master only send a SIGHUP to children when the service have
> been removed.
> My guess is that we could send a SIGHUP to all services. That way we
> don't have to wait for all preforked processes to finish before taking
> into account the new imapd.conf
> I have made some test and till now all seem working well.
I haven't seen your code so I don't know how you're dealing with
re-reading imapd.conf. But I note that in the code I see there's a
bunch of global variables in the imapd process which are set as a side
effect of reading imapd.conf. Not handling those properly can have all
sorts of unpleasant effects, including memory leaks and pointers to
freed memory. You might want to look at the recent commit in the master
branch that adds a function config_reset() that handles all that; it was
added for unit testing but could be useful when re-reading imapd.conf.
> The only tricky thing I see is that we don't want to SIGHUP children
> in the SERVICE_STATE_DEAD state but the actual code already take care
> of that.
> So is there any problem that justify to limit the SIGHUP propagation
> to children for removed service only ?
> Or could we go a bit further (with very few modification to the code)
> and allow graceful restart when we modify /etc/imapd.conf.
There are some imapd.conf entries which it would probably be a really
bad idea to modify partway through a client session, like
'virtdomains'. There are others which are only consulted during imapd
process initialisation and would be ignored if the file was re-read
later, like 'mupdate_server'. All the code that uses config data was
written under the assumption that the data would not change in the
lifetime of the process, and in some cases that assumption allowed
simplifying short-cuts which would not be valid otherwise.
So while I think it would be great to be able to re-read imapd.conf on
SIGHUP, there may be quite a lot of work to clean up the fallout.
More information about the Cyrus-devel