[Xapian-devel] Full MVCC in Brass

[Austin Clements]

On Tue, 19 Aug 2014, Olly Betts <olly at survex.com> wrote:
> Some brief initial comments, before I get a chance to study the
> more complex parts:
>
> On Tue, Aug 19, 2014 at 09:19:12PM -0400, Austin Clements wrote:
>> This has the annoying side-effect of requiring readers to start a
>> lock-holder subprocess like writers currently do.  I propose also
>> adding support for F_OFD_SETLK, which won't require a lock-holder
>> subprocess for either readers or writers.  F_OFD_SETLK was added in
>> Linux 3.12 (which will be in the next Debian stable) and will
>> hopefully appear in the next version of POSIX.1, so with any luck it
>> will become widely available in the future.  If F_OFD_SETLK isn't
>> available, Xapian can fall back to what it does currently.
>
> F_OFD_SETLK looks like exactly what we've longed wished F_SETLK meant.

I think that goes for anyone who has ever used F_SETLK.  :)

> It'd be good to do that as a separate patch, as I think it would be
> worth backporting to 1.2.x.

Since the locking algorithm is rather different for MVCC, I was planning
to implement it in a new BrassLock class.  However, orthogonal to that,
I don't think it would be hard to also add F_OFD_SETLK support in
FlintLock and backport that.

> The ticket submitting this to POSIX took a little effort to find, so
> here it is:
> http://austingroupbugs.net/view.php?id=768
>
>> Windows has LockFileEx, which is essentially the same as F_OFD_SETLK.
>
> Sounds good.
>
> We might want to keep the current locking code for chert there - need to
> think through if locking between 1.2 using the current approach and
> trunk using LockFileEx() would work without problems (we particularly
> want to avoid two writers managing to hold a lock at the same time).
>
> But if that's hard, I think just documenting that you shouldn't mix
> 1.2.x and newer versions on Windows would do.

I haven't seen anything saying that LockFileEx interacts with CreateFile
locking, but if this goes in a new BrassLock, then it shouldn't be an
issue because we'll always use LockFileEx for brass databases.

>> I don't know what to do about EMX or if we care.
>
> While OS/2 seems to live on, EMX looks pretty dead - last release was
> over 13 years ago.  We've already decided that requiring a compiler
> with good C++11 support for trunk is reasonable, so the one in EMX
> clearly won't cut it.
>
> There's also code to just use flock() (for anything without fcntl()
> locking), but I think we can probably just not support MVCC on platforms
> without a suitable locking API.  I think we'd ideally want to allow
> opting out of reader locking (e.g. if your searches run on a database
> which isn't updated at the same time, the overhead of the reader locks
> is unnecessary), so if there's no byte range locking API, we can use the
> same mechanism.

Okay.  If we're on a platform that can't support MVCC, should an attempt
to open the database for read *without* opting out fail?  Alternatively,
it could fall back to old-fashioned read locking (blocking writers) so
that readers still get the guarantee that DatabaseModifiedError will
never happen (and they could still opt out of all read locking and deal
with the consequences).

> I certainly think we shouldn't hold up progress on this because it isn't
> possible to support everywhere, especially since it can be supported for
> the vast majority of users.
>
>> Bonus points
>> ------------
>> 
>> When a writer exhausts free blocks that are available in the locked
>> revisions, but there are more available in revisions it couldn't lock,
>> it could attempt to extend its range of locked revisions.  If a reader
>> has since closed the database, this could allow the writer to reuse
>> more blocks rather than extend the database file.  This could be
>> important for long-running writers.  This would be easy with
>> F_OFD_SETLK/LockFileEx and huge pain without it (probably requiring a
>> dedicated lock helper binary).
>
> It would be beneficial to have a helper in other ways:
>
> http://trac.xapian.org/ticket/502
>
> But one option is to just not extend the lock range if we're using
> F_SETLK.

I think for a first shot I just won't do lock extension.  The next step
would be to do it unless we're using F_SETLK.  Then the last step would
be to add a lock helper.  Steps 2 and 3 are backwards-compatible, so
they could be done at any point in the future.

> Cheers,
>     Olly