SA Bugzilla – Bug 5131
RFE: graceful config re-reading in spamd
Last modified: 2010-04-21 20:42:47 UTC
It should be possible for spamd to reload the configuration "gracefully" -- ie. in the same process, without closing the accept socket or losing the listen queue of incoming connections, somehow. (In fact I think some of the commercial derivatives support it. ;) I would suggest this algorithm: in spamd parent, when SIGHUP is received: - a flag is set, and signal handler returns in the spamd mainline, at the "if ( defined $got_sighup ) {" check: - if that flag is set: - the Mail::SpamAssassin object ->finish() method is called - a new Mail::SpamAssassin object is created instead - all child PIDs are signalled with a new sig indicating "exit asap" for each child PID: - if they're busy scanning a mail, it continues to completion, then exits - if they're idle, they exit immediately before accepting another conn Benefits: - this should be faster than the existing method of killing all spamd processes - issues regarding "old" spamd processes holding onto the listen socket, resulting in EADDRINUSE, will not happen - since it allows existing scan processes to complete, instead of killing them, there's less possibility of cases where mail will be delivered unscanned Downsides: - failure to clear up circular refs or generated methods will become more obvious, since those will now create ongoing memory leaks over multiple restarts. mind you, t/recreate.t should be finding them nowadays anyway.
optimistic aiming at 3.2.0
See also Bug 4914 and Bug 4774 comment 2.
*** Bug 4914 has been marked as a duplicate of this bug. ***
thanks Tom -- they are indeed dups. I'll keep this GSoC description from bug 4914... 'This was a suggested idea for the Google Summer of Code 2006; I'm adding it to the bugzilla for future use, and in case anyone feels like implementing it. Subject ID: spamassassin-better-reload Keywords: reload, spamd, sighup, restart, perl Description: http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5131 : we currently have a very heavyweight configuration-rereading system where the entire process restarts. This is too heavyweight, and can be improved. Possible Mentors: Justin Mason (jm at jmason.org)'
Simply purging the current Mail::SpamAssassin object and creating a new one to load a new configuration set into doesn't work. Nearly all rules are stored in some global area. This means that when you kill the Mail::SpamAssassin object you don't actually kill all of the loaded rules. Now look at how rules are executed in, for instance, PerMsgStatus.pm::do_head_tests(). We loop over the static data in Mail::SpamAssassin::PerMsgStatus::_head_tests_*. So, imagine for a moment what happens when you remove rules from a rules file between HUPs. That's right: the rules continue to be run because nothing actually purges all of the in-memory rules. A valid next step in trying to solve this is to head over to Mail::SpamAssassin::PerMsgStatus::_head_tests_ and start deleting all of the loaded rules. Do this for head tests, body tests, raw body tests etc. Great, the hack works. Wrong. There are some plugins that come with SA that store their data in some made-up global namespace. Now you have special cases to clean up. Ugg. Why not fix this correctly by cleaning up the config namespace such that all config data is stored in one nice container. Purge the container: purge all of the rules. HUP is the easy part. It's like a 20-line patch at most. This isn't the problem though. The problem is finding and refactoring all of the different rule plugins that store their data in ruletype-specific places and changing them to be stored in a nice container that is a member of a Mail::SpamAssassin instance.
actually, Bob, the deletion of stuff from _do_head_tests etc. is already done in 3.2.0, as part of Mail::SA::finish(). plugins that are stuffing things into the global namespace, instead of using the 'conf' object that's passed around for this purpose -- that's a bug, and they will simply need to be fixed.
The Check plugin branch moves finish_tests to be a plugin API call, so different plugins can be responsible for cleaning up their own mess.
no code, punting.
moving most remaining 3.3.0 bugs to 3.3.1 milestone
these were not supposed to go to security! moving back out of that component, but now probably to the wrong components :( , but better than nothing.
reassigning, too
moving all open 3.3.1 bugs to 3.3.2
Moving back off of Security, which got changed by accident during the mass Target Milestone move.