Subject: Re: [SAdev]  New: Bayes current scan-count resets 


> Following a successfull expiry, the current scan count sometimes resets
> to th e value modulo 5000 (ie the size of the
> ~/.spamassassin/bayes_msgcount file) instead of continuing to increment.

Got it -- the lock failed.  We should modify the code to ensure the lock
succeeds before zeroing the file.

--j.

Created attachment 730 [details]
suggested patch

I just sent in a suggested patch.  It changes a bunch of lines, but here are the basics:

there are a bunch of places in BayesStore that do "die", which isn't good since those are the places which can get called by scan() so we could potentially lose mail depending on what is calling SA in the first place.  I changed these to do a warn and return false.

The unlink areas weren't checking the results of being able to tie R/W or whether or not the call to scan_count_increment_big_counter() was successful.  I added in some error checking for that.

I would like the patch looked over before anything else happens with it. :)

Subject: Re: [SAdev]  Bayes current scan-count resets

On Thu, Mar 06, 2003 at 03:26:20PM -0800, bugzilla-daemon@hughes-family.org wrote:
> I would like the patch looked over before anything else happens with it. :)

Oh, not included in the patch but I was just thinking may be a good idea
to subtly fix this issue in bayes dbs all around...  Change:

  if ($now - $last > $limit/2 && $now - $oldest > $limit) {
    return 1;
  }

to:

  if (($now - $last > $limit/2 && $now - $oldest > $limit) || ($now<$last)) {
    return 1;
  }

in BayesStore::expiry_due() ...  $now should never be < $last unless
this bug hits and screws up $now.


BTW: I think there's a problem beyond the lock fail issue.  I just ran
a force-expire to reset the token count and without any locking errors:

0.000        0     1958        0  non-token data: current scan-count
0.000        0     6957        0  non-token data: last expiry scan-count

I previously had a scan-count around 6940 and a last expiring scan-count
of 7506.  I then ran the force-expire, and the above is what came out.

I then ran it again:

0.000        0     1966        0  non-token data: current scan-count
0.000        0     1965        0  non-token data: last expiry scan-count

So for some reason it didn't update the expiry count the first time. :|

Hmmm...  I've been running with a number of patches in my SA 2.50, including 
the suggested patch for this bug, but I still see this issue:

0.000        0        0        0  non-token data: db format = on-the-fly 
probs, expiry, scan-counting
0.000        0    25186        0  non-token data: nspam
0.000        0    12516        0  non-token data: nham
0.000        0   103238        0  non-token data: ntokens
0.000        0     1958        0  non-token data: oldest age
0.000        0      173        0  non-token data: current scan-count
0.000        0    10002        0  non-token data: last expiry scan-count

where 173 is the size of msgcount.  Poking around a bit, it seems that the 
scancount_base DB entry is blown away (it's not set to 0, it's undef...)

<theo does some digging>

Ok, I found it.  And it's really stupid.  Basically, expire_tokens_trapped() 
is called, but it only knows about 5 special tokens that it ignores when 
expiring and makes sure to copy over to the new DB.  Guess which one it forgot 
about...  Right: $SCANCOUNT_BASE_MAGIC_TOKEN.

Doh!

setting back to 2.50 and increasing the priority since the bug is very 
important.

Created attachment 738 [details]
new version of the suggested patch

Created attachment 739 [details]
patch to expiry_due to check current_scan_count < last_expire, should never happen.

Subject: Re: [SAdev]  Bayes current scan-count resets

> Ok, I found it.  And it's really stupid.  Basically, expire_tokens_trapped() 
> is called, but it only knows about 5 special tokens that it ignores when 
> expiring and makes sure to copy over to the new DB.  Guess which one it forgot 
> about...  Right: $SCANCOUNT_BASE_MAGIC_TOKEN.

Oh my ghod.  That's the stupidest mistake I've made yet...
I guarantee my brain will work a whole lot better once I get to CA ;)

--j.

Subject: Re: [SAdev]  Bayes current scan-count resets

> I guarantee my brain will work a whole lot better once I get to CA ;)

Judging by the local populace, I wouldn't count on it...

OKAY: on both

applied to 2.5x branch.  closing ticket. :)