[PATCH] [ldb_modules ABI break] Re: [WIP][PATCH] Safe LDB locking for better replication

Andrew Bartlett abartlet at samba.org
Thu Jun 22 21:25:03 UTC 2017

On Thu, 2017-06-22 at 11:08 +0200, Stefan Metzmacher wrote:
> Am 22.06.2017 um 10:36 schrieb Andrew Bartlett:
> > 
> > Without wishing to derail this, I would note that this does change the
> > ldb module ABI (which we never promised anyway),
> I already changed the commit to be ldb-1.2.0.
> > and also is not
> > backwards compatible with Samba < 4.7.
> Why and what to you mean exactly?
> Building Samba 4.6 with the new ldb version?


For the list archives: If you use an old Samba (or old anything,
without a hack forcing an override) and don't rebuild, they will fail
to operate as we have a check.  If you override, then you will supply a
too short modules structure.  But you know all this.

So the answer is yes, Samba 4.6 will build fine, but then fail as

> > It is likely to trigger deadlock detected "failed to upgrade hash
> > locks", LDB_ERR_BUSY on earlier Samba versions, as they came to not
> > expect locking (and the required lock ordering) to be enforced on
> > ldb_tdb.
> Can you give an example commit that is missing in earlier Samba
> versions?

commit 10e7c749e7b7a4155669c6ecf97a9ac52b13110a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 31 12:22:28 2017 +1200

    dsdb: Correctly call ldb_module_done in dsdb_notification

For the 1.1.30 ldb version, the even context changes require this in
Samba (less important, only hits -M single):

commit 7259661467776a76c4fa3aabaf1ae8a3d531e506
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri May 12 01:55:45 2017 +0200

    dsdb: Use ldb_handle_use_global_event_context for rootdse modifies
    The modify operations on the rootDSE turn into IRPC messages, and
these need
    to be handled on the global event context, not the per-operation

And this probably only fails in make test and causes noise (tried to do
transaction writes during a search):

commit 5f0e53f1b90369c649688122c0a8742352f13877
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 3 22:53:14 2017 +0200

    dsdb: Do not write the @INDEXLIST or @ATTRIBUTES records during
schema refresh
    Instead, write it once in the module init, if required, and after a
    modify to the schema partition is detected

But this will fail in normal operation:

commit b8ba0103bf45670c31384c56d6cd63bbef760a0c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 3 22:51:30 2017 +0200

    dsdb: Take out the transaction and prepare_commit locks in the same
    We must, when starting the transaction and preparing to commit the
transaction, take
    out the locks in the same order across all the databases, otherwise
we may deadlock.

> > Naturally, this is balanced by the fact that we absolutely have to
> > merge this, as the read and replication corruption it caused has been
> > observed in the real world, not just our selftest.
> Yes!
> I've looked through the patches and found two things:
> 1)
>  "ldb: Add test encoding current locking behaviour during ldb_search()"
>  should include the lib/ldb/tests/ldb_mod_op_test.c changes
>  from "ldb: Lock the whole backend database for the duration of a search"
>  Or I'm I missing something?

No, what I tried to do is to show what was the behaviour, then fix the
test for the new behaviour when fixing the code.

I would have done the knownfail trick but we don't have that for ldb.

> 2)
>  "dsdb: Teach the Samba partition module how to lock all the DB backends"
>  got the unlocking order wrong, which must match
>  {end,del}_trans() instead of read_lock().

Well spotted, I agree we want to unlock sam.ldb last.

>  The key is that the read locks on the individual partitions
>  are purely a performance optimization to avoid
>  fcntl syscalls, as these looks are never contented,
>  because if the "BIG-Lock" on the metadata (sam.ldb).
> I'm starting private autobuilds with the attached patchset.

Thanks so much for all your careful examination of this!

Do you want to tidy up your comments, squash in the unlock and push
once you are happy with the autobuild, or is there something more you
need from me?


Andrew Bartlett

Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

More information about the samba-technical mailing list