On Fri, Sep 11, 2015, at 23:19, Vladislav Bogdanov wrote:
11.09.2015 14:49, Bron Gondwana wrote:
On Fri, Sep 11, 2015, at 20:00, Vladislav Bogdanov wrote:
11.09.2015 12:01, Bron Gondwana wrote:
On Fri, Sep 11, 2015, at 16:48, Vladislav Bogdanov wrote:
11.09.2015 03:52, Bron Gondwana wrote:
sort: spamscore search: spamabove / spambelow
These use the X-Spam-score header which is a floating point number
with a single decimal place usually, i.e. 5.0, 17.3. spamabove is GE
and spambelow is LT.
I'm going to push this back, because it doesn't clash with anything.
It's kinda nice to be able to sort by spamscore to quickly put the
focus on the most likely to be be wrongly classified messages, and
we're going to support that in our interface at some stage.
Bron.
Ah, I have a nice patch for spamtest extension against 2.4.17.
It connects to spamd itself from lmtpd, checks the message and sets
additional headers. Sieve integration is done too.
Need to send it here.
That would be great.
Attached. Just found that it inconsistently uses tabs/spaces. I hope
that is not an issue at least for initial review.
Initial impressions:
I would rename 's' to 'fd'. Everyone knows what 'fd' does, s could be anything,
and being an int, it's totally un-typesafe.
It was initially written against newly-released 2.3.8. That time socket
fd was usually named 's' or 'sock'. 's' is shorter ;) Did that change?
Fair enough :) I haven't dealt with that area of the code quite so much,
more the index and database filehandles.
Yep, that never resulted in lost messages for last N+1 years in quite
busy setups (not as fastmail, but anyways...).
So, yes, that "works for me" and such change would be really cosmetic.
I just ported it to 2.4.17 recently without even looking much at the
code (but yes, that is tested on newer setups right from the patch date).
I can send previous revision (against 2.3.13) as well.
No, that's fine. We can work with this.
I've got a sneaking feeling that your entire spamtest_parse_hosts could be
turned into a tight little piece of code based on strarray_split() - but it
looks fine.
No opinion. It was not available in 2.3.x. IMHO spamtest_parse_hosts is
very straight-forward.
And, that _may_ conflict with the line in TODO list (which I probably
will never do anyways because I have no idea how to make that fair
enough without initial lookups):
* Make load-balancing work if hostname that resolve to multiple A
records is used in "spamtest_spamd_hosts".
One interesting possibility (more for spam than virus checking) is to
send requests
for the same user to the same host always, which would require some form
of
consistent hashing. We do that with nginx at FastMail for web requests:
upstream internalbackend {
server web1.nyi.internal:8080;
server web2.nyi.internal:8080;
server web3.nyi.internal:8080 down;
server web4.nyi.internal:8080;
server web5.nyi.internal:8080;
server web6.nyi.internal:8080;
}
So it knows web3 is down and hashes elsewhere, but when it comes back up,
the
same users will move back.
We do the same for postfix lmtp delivery with some magic code in a thing
called lmtpforwardd,
which just listens on localhost and forwards to the correct server based
on the list of up spam
scan machines.
All the code looks like it works (which is not a surprise, because it's been
used).
My main concerns would be around signal safety in the file IO syscalls.
Feel free to convert them to prot ones, but I do not feel it is strictly
required.
Sure thing :)
Bron.
--
Bron Gondwana
br...@fastmail.fm
