txn performance penalty

[Matthias Andree]

On Mon, 15 Nov 2004, David Relson wrote:

> I've spotted something odd in function lookup() in file score.c.  In
> outline form, the code is:
> 
>     for (list=word_lists; list != NULL; list=list->next)
>     {
> 	ds_txn_begin(list->dsh)
> 	ret = ds_read(list->dsh, token, &val);
> 	ds_txn_commit(list->dsh)
>     }
> 
> Why is ds_read() wrapped in a transaction?

Because it must, so that Berkeley DB knows for how long it must present
the state that was valid when we requested input.

The code, at a second glance, is bogus though.

We can and must make the transactions longer-lasting, i. e. open these
in the lookup() callers. I also see ds_write that "Protect against
negatives". I'm not convinced that code ever worked, and I do believe we
don't have this in our test suite. A transaction must last from before
we look up the first token of a mail until we've read the last token of
a mail.

There's also code under the "Protect against negatives" comment, which
is bogus unless the database was opened for update or registration
(bogofilter -u), but we don't appear to check that and skip the ds_write
for read-only databases. And we're probably not catching these lines in
our test suite.

I wrote about the need to disentangle code, and this is one such place.
More later, gotta go.

-- 
Matthias Andree