[PATCH] LMDB full patch set

[Stefan Metzmacher]

Hi Andrew,

>>> I've addressed your concerns as far as practical[1].  If you could
>>> please allow this to proceed into master I would most appreciate it. 
>>
>> Can you move all tdb related patch to the start of the patchset,
>> they should work on their own. Also the fork protection tests.
>>
>> I don't see where ldb_tdb_test is used in tests...
> 
> I've fixed all this and pushed those to autobuild with Garming's
> review.

Thanks!

>> From reading the patches it seems that it will no longer possible
>> to build ldb or an AD DC without lmdb, e.g. 32bit support will be gone.
>> I don't think that's acceptable right now, maybe in a few years.
> 
> Yes, you are correct we will require lmdb for the AD DC and ldb
> standlone builds.
> 
> lmdb is shipped on 32 bit ubuntu.  I've set up a test environment and
> will confirm the LDB tests pass on it.  Howard Chu indicates earlier in
> the thread that it is fully functional, but limited in DB size to 2GB.

Are we going to require lmdb at runtime or do you want to require it
at compile time?

Even for the standalone ldb build we have --disable-python.
I think a standalone ldb should be able to build without lmdb
it the user really wants this.

For the AD DC I'm happy to require it (similar to the jansson library)if
selftest is enabled, but I really don't see why we need to require
it at build time (at least an explicit --without-ldb-lmdb should be
possible for the AD DC and standalone builds).

Can you summaries how this is supposed to work at runtime?
(I didn't have time to play with this yet)
Will lmdb only used for the partition files or are all files,
e.g. secrets.ldb and wins.ldb.

In case lmdb is not activated by default (at runtime)
and you prepare a tmp patch to do that and check what autobuild says
about that?

In the configure check we also check for a specific function of lmdb
and only enable HAVE_LMDB on success (together with an unused
conf.env.ENABLE_MDB_BACKEND), but if it's not found we don't complain...

> The DB size issue is also the first issue I've found empirically, as we
> got this while trying to set the default DB size:
>
> [1295/4337] Compiling lib/ldb/ldb_mdb/ldb_mdb.c
> ../lib/ldb/ldb_mdb/ldb_mdb.c: In function lmdb_open_env:
> ../lib/ldb/ldb_mdb/ldb_mdb.c:726:2: error: large integer implicitly
> truncated to unsigned type [-Werror=overflow]
>   ret = mdb_env_set_mapsize(*env, 16LL * GIGABYTE);
> 
> As the operational side is both optional and not a major departure from
> the 4GB limit on TDB, I think this is a reasonable restriction.  Those
> running a RaspberryPi DC won't be running hundreds of users in any
> case.
> 
> To address the broader point:
> 
> My strong preference is that the AD DC be able to rely on this feature
> being available.  I don't mind if the fileserver-with-bundled-libraries 
> has it optional, but for the AD DC I'm actively trying to reduce the
> possible combinations so we can keep everything tested.

For autobuild I'm fine with the requirement, but we need to test the
code path without lmdb anyway and keep it working. That's why I
think we should have a --without-ldb-lmdb option.

> (Once this lands, I'll change some of our selftest environments to use lmdb). 

I want to see the patch that enables it first.

> That is why 'samba' needs to require it for the AD DC.  For standlone
> ldb, that reason I've not made it optional is that otherwise we need to
> declare to samba that the built-standalone 'system' ldb doesn't have
> lmdb, then have a reliable method to detect that.  This seemed like all
> to much complexity for a feature we will likely soon strictly rely on. 

Why do we have to rely on this strictly very soon, that is every tricky
new code without any real world testing.

> (We have been actively removing the ability to run 'make test' without
> optional packages after we found parts of our testsuite silently
> skipped.  I'll propose the same for the ldb package once this is
> agreed.)

Yes, for autobuild that's fine, but there's a real world outside of
autobuild.

> Portability to i386 I can sort out, but can you confirm you are
> otherwise happy with the branch?
> 
> https://gitlab.com/catalyst-samba/samba/commits/metze-master4-lmdb-full

Not is this state sorry and I'd like to play with lmdb activated
and have a look at the files myself...

Thanks for your patience...
metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180509/a479adb7/signature.sig>