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

[Stefan Metzmacher]

Hi Andrew,

>>> 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?
> 
> Yes.
> 
> 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
> below:

I've created https://bugzilla.samba.org/show_bug.cgi?id=12859
to crack that problem.

We need to backport something that will allow us to let
future 4.5 and 4.6 releases work with ldb >= 1.1.30.

Any ideas about what we can backport?
Or how we can add some code that works around problems?

>>> 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
> context

Would it help to introduce an
ldb_set_allow_private_event_context() that's called by a new Samba
release. And other callers get the old behavior?

It would still leave us with unusable 1.1.30 and 1.1.31,
but 1.2.0 would be usable again.

> 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

We may need to do the read locking optional as well
only if the callers requested it.

> 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
> order
>     
>     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.

That's more a problem introduced by
"tdb: Remove locking from tdb_traverse_read()" correct?

Would it help if we would grab the write lock in
ldb_transaction_start() instead of prepare_commit()
unless the caller of ldb (a new Samba version) indicates
support for this?

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/20170623/b58f27c0/signature.sig>