2011/8/9 Dave McMurtrie <dav...@andrew.cmu.edu>: > > > On Aug 9, 2011, at 4:50 AM, Olivier ROLAND <cyrus-...@edla.org> wrote: > >> 2011/8/9 Greg Banks <g...@fastmail.fm>: >>> 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. >>> >>> >> >> Thanks a lot Greg for your feedback but you missed the point ;-) >> I don't change anything during the lifetime of the process. >> Cyrus let the last active child finally terminate with the old >> imapd.conf and then force the process to exit cleanly immediatly >> without recycling. (no additional code is needed, all is already >> there, see the code used when a service is disabled) >> We are with a brand new process for the new imapd.conf, so no problem >> with global variables, no problem with initialisation, etc ... >> >> The idea here is to use this code not only when a service is disabled >> but every time master receive a SIGHUP. >> I see no side effect during all my tests but prefer to ask the mailing >> list just in case I missed something. >> Seem that we could have re-read imapd.conf functionality for free. In >> fact I'm very surprised that this was not done before ;-) >> > > If you're interested, you can already accomplish this by updating the mtime > (touch) of the on-disk executable for whichever process you want to re-read > imapd.conf. This check is actually in place to detect if a new binary has > been installed so you could (in theory) upgrade while Cyrus is still running, > but changing the mtime of the binary should have the effect you desire. > > Hth, > > Dave
Thank's Dave. Yes indeed your trick works (calling exactly the same piece of code). Good to know but can we reasonably tell a sysadmin to proceed like that ? I don't think so. Olivier ROLAND AtoS
