G'day Olivier, 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 /etc/imapd.conf. 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.
http://git.cyrusimap.org/cyrus-imapd/commit/?id=03b4fcc4d956ccbfbbd5220b6d1ca9107707821f
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.
-- Greg.
