Patches welcome, but I don't think there's a way to make the update atomic.

As for SA being confused, it doesn't care if the rules are changed around while
it's running.  If you run sa-learn or spamassassin or restart spamd during an
update, there could be an issue.  Since that's a standard issue for every daemon
I can think of, I'm not sure there's much we should do about it.

A rough strategy for making updates atomic, at least per rules directory, is:

(1) When SA is starting it chdirs to each rules directory and reads the rules
files using just their basenames. This might not be necessary if it doesn't
matter that SA loads some rules from the old set and some from the new set.

(2) Updates populate a new directory with rules then atomically move it into
place. This might not be portable to non-Unix filesystems.

The problem with sa-update at the moment is that SA might see only a partial
ruleset when it starts, which is worse than no ruleset (in which case it falls
back to the original rules distributed with the code) or a changing ruleset.

Theo's right, btw -- there's really no way to make the updates *really* atomic.
 however we could certainly get it *nearer* to being atomic, I think, by
reducing the available race window duration. ;)




'(1) When SA is starting it chdirs to each rules directory and reads the rules
files using just their basenames. This might not be necessary if it doesn't
matter that SA loads some rules from the old set and some from the new set.'

I'd rather not do this; we try to avoid chdir()ing around since
Mail::SpamAssassin may be run from a third-party application, also it could
effect the environment seen by third-party plugins too with respect to paths. 

'(2) Updates populate a new directory with rules then atomically move it into
place. This might not be portable to non-Unix filesystems.'

This is preferred IMO.  but then, there's no atomic way to do that, as noted ;)

There are two options:

1. the double-rename-directory approach:

- download update into "$target.tmp" (where $target =
"/var/lib/spamassassin/3.002000/updates_spamassassin_org" or whatever)

- perform lints etc. on that

- rename '/var/lib/spamassassin/3.002000/updates_spamassassin_org',
'/var/lib/spamassassin/3.002000/updates_spamassassin_org.old' or die

- rename '/var/lib/spamassassin/3.002000/updates_spamassassin_org.tmp',
'/var/lib/spamassassin/3.002000/updates_spamassassin_org' or rollback old dir
and die

This is generally best UNIXy practice I think.  There's a much, much smaller
race window -- two rename() syscalls.  however the spamassassin script would
still see an empty rules dir if it ran right between the two rename()s (since
the "include" cf file would include a nonexistent dir).


2. the "valid rules dir" flag file:

I've suggested this before for other reasons; however it helps with this one
too!  Basically it's similar to the above, except with the addition of a "this
directory contains valid rules" flag file.  The ruleset is not used unless that
flag exists.  before the rename()s, this file is unlinked; after they complete,
it's recreated.  Since the creation/deletion of a file is atomic, this would
make things atomic in turn.

The way my script atomically replaces the rules directory is by replacing a
symlink to the old rules with a sylmink to the new rules using rename().

(I've just had another look at the POSIX rules about replacing a directory using
rename() and it's no use for us because the directory to be replaced must be empty.)

Created attachment 3759 [details]
Atomic update wrapper for sa-update

'replacing a
symlink to the old rules with a sylmink to the new rules using rename()'

hmm, interesting trick! this may be the best option -- modify the script to
have a symlink at a certain point so that we can do this.

unfortunately though, that would need to be conditional on UNIX vs win32 --
win32 updaters obviously can't use that.

Another possibility that would be compatible with win32 is an elaboration of the
flag file idea. SA could glob for rulesdir/version/*/flag to find out which
directory currently contains the flag, and load the rules from there. (Note the
glob isn't atomic so it may see the flag both before and after a rename, in
which case SA should probably stat the directories and pick the newest.) When
sa-update runs it can pick an unused directory name in rulesdir/version for the
new rules, and when they have been verified it can atomically rename the flag
file from its current location into the new directory. Then wait a short time in
case SA is loading old rules, and finally delete the old rules directory.

(In reply to comment #3)
> Theo's right, btw -- there's really no way to make the updates *really* atomic.
>  however we could certainly get it *nearer* to being atomic, I think, by
> reducing the available race window duration. ;)

Sure.  I just don't think we need to go crazy overboard here w/ flag files and
so on.  We operate in the same way as every other tool that has config files.  I
don't want to start putting in a bunch of complicated strange hacks to deal with
very uncommon problems.

What's the old maxim? If it ain't broke, don't fix it?

Even on a loaded system the window isn't all *that* long. I'm not a dev here so
I really don't have any say but my 2 cents is leave it alone.

Any reasonable system admin is going to test changes first and then implement.
So I don't see what the real issue is.

Created attachment 3945 [details]
atomic rules update with sa-compile support

This is the current version of my update script, which includes support for
sa-compile.

I've marked this bug as depending on 5446 because sa-update-atomic requires the
--updatedir option for sa-compile.

Closing old stale bug. Current sa-update is atomic enough imo.