[PATCH] LMDB full patch set

[Andrew Bartlett]

On Thu, 2018-04-12 at 17:48 +0200, Stefan Metzmacher wrote:
> Hi Andrew,
> 
> > On Wed, 2018-04-11 at 14:15 +0200, Stefan Metzmacher via samba-
> > technical wrote:
> > > I'd like to have a look, but that might take a few days.
> > 
> > Can you please be specific?  Last week you said you would have feedback
> > for me in a day, and it has been over a week.
> 
> I looked at it today.
> 
> > > And as you seem to still find bugs, I don't understand why this needs
> > > to get to master as soon as possible. 
> > 
> > I've not found any bugs in the LMDB patch set.  It has not changed
> > since the re-order you requested last week. 
> > 
> > What I have done is address yet another serious locking bug.  Indeed
> > rare but existing locking bugs have taken up most of the time for this
> > project!
> 
> As always:-)
> 
> > Perhaps it happens more often with this patch, or perhaps it is just
> > like the branch point before Samba 4.8 where flapping tests came out of
> > nowhere.  Either way, what I've done is find and fix it.
> > 
> > (Gary did his due testing the entire time he was testing, and when he
> > first proposed it, and they didn't fail then). 
> > 
> > > The branch point for 4.9.0rc1
> > > won't happen in near future.
> > 
> > I really, really don't want to leave this any longer.  I've got a set
> > of patches that pass the Gitlab CI.  They also show good results on
> > Catalyst Cloud autobuilds, results being:
> 
> I reordered the patchset a bit further:
> 
> The patches in
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-lmdb-pre1
> should be fine for master if they pass autobuild.
> 
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-lmdb-pre2
> 
> Contains the test_transaction_start_across_fork and
> test_lock_read_across_fork tests, but it tries to run them
> also with tdb.
> 
> I think it's very important to have the same external behavior between
> tdb and lmdb. We should also protect against a fork between
> transaction_start and transaction_[prepare]_commit. Maybe even more
> situations.
> 
> This should then pass a build without having lmdb.h on the system.
> 
> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-lmdb-full
> 
> Contains the rest with some build fixes, whitespace cleanups
> and some more TODO markers in commit messages.

I've pushed the first set.

ON the rest Most of those comments and fixes are fine, but just one of
those before I finish up tonight:

commit 61d58289826576c27e521d6f98e9c1bcea572441
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Mar 6 15:30:10 2018 +1300

    SQ??? ldb_mdb/tests: Run api and index test also on lmdb WHY not
the same dict for all tests?  

The different index dict is deliberate, we don't want to leave the
traditional TDB without an index untested. 

Otherwise, beyond how to handle the destructor after the fork(), I
generally agree. 

and I think leaking it is the safest option so far: we can't get more
than 1 leaked object and fd per actual DB per actual parent anyway.

Finally, on:
# we don't want to run ldb_lmdb_size_test

Correct, we don't want to create 4GB files on the disk-limited test
instances. 

Gary can sort the rest out with you next week.

Thanks!

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180413/aa1ffdea/signature.sig>