R: More Skiplist Stuff!
brong at fastmail.fm
Tue Jan 15 08:13:43 EST 2008
On Tue, Jan 15, 2008 at 11:37:14AM +0000, David Carter wrote:
> On Tue, 15 Jan 2008, Toschi Pietro wrote:
>> I'm afraid to bother on the devel list since I'm not a developer, but I've
>> got some confusion about what patches should I apply to a fresh 2.3.11
>> cyrus-imapd tarball in order to have a not-broken skiplist mailboxes.db
>> (and possibly all the remaining backends too) and avoid those problematic
>> DB ERRORS and reconstruct? Would you be so kind and please summarize and
>> put some light about the correct patching-path that we (simply humans)
>> should follow (possibly before your escape to the Tasmanian bushes, Bron
I'm escaped! Thankfuly we still have internet here.
> I think that you only really need to worry if you are running Fastmail's
> folder_limit patch. The following two cleanups have been merged upstream:
> and should apply either way around.
> But this was really just Bron hunting down the root cause of the problem:
> the fact that the folder_limit patch was accidently blowing away the write
> lock on the mailboxes.db skiplist database. This is what the third patch:
> works around, by reusing the existing transaction and write lock if a
> mboxlist_count_inferiors() occurs in the middle of a transaction.
If any read operation without a transaction happens in the middle of a
transaction actually (a write without a transaction will cause it to
die with an assertion failure - that's just plain bogus with skiplist
how it is)
That said, you probably want David's reconstruct patch - the most recent
one is very nice code, much better structurally. It doesn't apply at
all nicely on 2.3.11, but you can find it backported at:
Glancing over my patch series, I don't think there's anything else
you really need. The really important skiplist fixes are all merged
upstream and made it into 2.3.11. The only bug that's not yet fixed
in 2.3.11 (but is in those patches merged above that David mentioned)
is the failure to re-read logstart when a file gets changed by a
checkpoint, leading to the first writer afterwards incorrectly thinking
it needs to run recovery. This is only a performance and strangeness
niggle, it doesn't change correctness.
There's no reason _not_ to apply those two patches if you want. They
convert things that could potentially break skiplists into an assert
failure and termination of the offending process instead. Much safer.
More information about the Cyrus-devel