From discussing with Mark, this is a blocker for 3.4.0

Mark:
> What used to be a rarely-
> occuring bug that went by unnoticed, will turn into a likely stall with
> only one process remaining available for all but one socket (e.g. on a
> dual-stack host with spamd listening on both IP addresses of a loopback
> interface).
>
> I'd say it is a blocker. We may not bother with Windows-locking and
> just implement a simple flock, which is cheap and works well on Unix
> using a local (non-NFS) lock file, and automatically unlocks if a process
> happens to crash while holding a lock.


My only thought perhaps them we should make an if (windows) loop and if ip4 and v6, then round-robin is not available option as well? 

Beyond that, the work around is trivial and I don't think has a huge impact on the product. Looking at bug 4594, I think is the one I'm remembering.

> > I'd say it is a blocker. We may not bother with Windows-locking and
> > just implement a simple flock, which is cheap and works well on Unix
> > using a local (non-NFS) lock file, and automatically unlocks if a process
> > happens to crash while holding a lock.
> 
> My only thought perhaps them we should make an if (windows) loop and
> if ip4 and v6, then round-robin is not available option as well?

Some digging shows that Perl's flock on Windows is just fine since
Windows XP. It may lack the LOCK_NB (nonblocking) option, but we
don't need it.

Also the perlport man page says:
  flock   Not implemented (VMS, RISC OS, VOS)
i.e. it does not mention Windows.

So this concern was apparently a very old information.


I don't remember exactly why the 'managed' scheduling was introduced,
seems to me the only reason is to provide dynamic scaling of the
number of child processes. When one wants a fixed number of processes,
I think the 'autonomous' is just as good, if no better (for its
simplicity).

Here is now my change. I tried to make no impact on the 'managed'
scheduling (no --round-robin), locking is only active when there are
multiple sockets *and* --round-robin (autonomous child processes)
is used.

Also it adds some more informative debug logging, a status check
in SpamdForkScaling.pm, and cosmetics
(Mail::SpamAssassin::Util::untaint_file_path -> untaint_file_path)

trunk:
Bug 6996 - spamd --round-robin conflicts with multiple listen sockets
  Sending lib/Mail/SpamAssassin/SpamdForkScaling.pm
  Sending spamd/spamd.raw
Committed revision 1563172.

Created attachment 5182 [details]
Proposed patch - adds locking across select+accept on multiple sockets

(In reply to Mark Martinec from comment #2)

> I don't remember exactly why the 'managed' scheduling was introduced,
> seems to me the only reason is to provide dynamic scaling of the
> number of child processes. When one wants a fixed number of processes,
> I think the 'autonomous' is just as good, if no better (for its
> simplicity).

The original reason was better performance under paging. By assigning work to the free child with the lowest pid, a number of "hot" processes can retain their resident memory and the higher pid children can be paged-out without much affect on performance. Also since most relevant OS's that support forking assign increasing PIDs, some most-recently created children will remain in an immediate post-fork state reducing SA's overall resident memory usage.  In comparison round-robin or random child allocation is close to worse-case in all respects.

So to follow up RW's comments, Round-robin is a "not recommended" option anyway but since the patch is done do we want to consider it a fix and move on?  Mark, I believe you committed it.  Do I need to make an rc6 for testing?

(In reply to Kevin A. McGrail from comment #5)
> So to follow up RW's comments, Round-robin is a "not recommended" option
> anyway but since the patch is done do we want to consider it a fix
> and move on? Mark, I believe you committed it.
> Do I need to make an rc6 for testing?

I was hoping for some feedback from Jan Hejl who reported
the issue originally. But yes, the patch has been committed,
I've tested a couple of scenarios (on Unix, not Windows)
with multiple clients, with and without --round-robin,
observing the debug log. Seems to work as expected - although
I'm not using spamd in production (running amavis here), so
it can't hurt to do another RC.


Not directly relevant to this problem report, but regarding
the comment #4: I agree with the explanation there, but need to
add a remark. When the number of child processes roughly matches
the load so that most of them are busy most of the time, and
the number of processes does not exceed the host's memory size,
then autonomous/random scheduling is no worse than the
'managed' approach. On the other hand, if the number of child
processes is large and a sizable share of them are idle, so
they get paged out, then in my opinion the problem is in the
configuration which should not allow more processes to exist
than a host can handle, and not many more than are actually needed.
In the end, it all boils down to how SpamAssassin is integrated
with a mailing system (before-queue or after-queue with MTA,
or invoked at the time of a mail delivery to a mailbox, or when
a mail client accesses its mailbox).

The reason why I use --round-robin option is because of "static" child proccesses allocation. I run spamd with certain amount of childs that resides in memory prepared for incoming connections. Specially when there's an incoming peak on spamd it tooked some time to fork new childs. But this was happening in the past and i haven't been testing it without --round-robin since 2009.

Please give me more time to do some tests, i'll let you know.

You can have a fixed number of children without using round-robin just by setting --min-children.

If the best that can be said about round-robin is that in some circumstances it's as good as the managed version then what's the point in maintaining it?

(In reply to RW from comment #8)
> You can have a fixed number of children without using round-robin just by
> setting --min-children.

Actually this doesn't work for me. With --min-children=20 (or any other value) spamd starts only with two childs.

What is proper syntax to start spamd with 20 idle childs?

(In reply to Jan Hejl from comment #9)
> (In reply to RW from comment #8)
> > You can have a fixed number of children without using round-robin just by
> > setting --min-children.
> 
> Actually this doesn't work for me. With --min-children=20 (or any other
> value) spamd starts only with two childs.

It doesn't work for me too in a 'managed' variant, didn't work with
version 3.3.2 either. Probably related, I remember there were complaints
about stability of managing child processes where --min-children is
the same as --max-children. Should be alright with --round-robin.

Created attachment 5183 [details]
patch to fix min-children in spamd

Or you can set max-spare to the same value as max-children, the number children never drops below max-spare. I'd forgotten that min-children is broken.

> Created attachment 5183 [details]
> patch to fix min-children in spamd

> --- spamd/spamd.raw     (revision 1564602)
> +++ spamd/spamd.raw     (working copy)
> @@ -861,6 +861,9 @@
>    if ($childlimit > $opt{'max-spare'}) {
>      $childlimit = $opt{'max-spare'};
>    }
> +  if ($childlimit < $opt{'min-children'}) {
> +    $childlimit = $opt{'min-children'};
> +  }

Thanks, makes sense, does work.

Bug 6996 #11: spamd to honour --min-children
  Sending spamd/spamd.raw
Committed revision 1564604.

rc6 is out now.  Please make all haste in testing it as we would very much like to build 3.4.0 and get votes to release it down by the 10th.

3.4.0-RC6 looks good. Closing.