[WIP][PATCH] Safe LDB locking for better replication

[Andrew Bartlett]

On Sat, 2017-06-17 at 08:19 +1200, Andrew Bartlett via samba-technical
wrote:
> On Fri, 2017-06-16 at 15:13 +0200, Stefan Metzmacher wrote:
> > Am 16.06.2017 um 06:55 schrieb Andrew Bartlett:
> > > On Thu, 2017-06-15 at 16:58 +1200, Andrew Bartlett via samba-technical
> > > wrote:
> > > > On Wed, 2017-06-14 at 22:02 +1200, Andrew Bartlett via samba-
> > > > technical
> > > > wrote:
> > > > > > Further comments most welcome, and I plan to re-write it to use
> > > > > > read_lock() and read_unlock() operations tomorrow. 
> > > > 
> > > > Attached is the patch set against master
> > > > 
> > > > As mentioned previously, my TODO list is:
> > > >  - fix tests not to hang when failing
> > > 
> > > They don't seem to hang, at least when reverting the whole-db lock.
> > > 
> > > >  - add test for the talloc_free(req) unlock case
> > > 
> > > I've added such a test.
> > > 
> > > >  - add some kind of test for the partitions locking 
> > > 
> > > I've added a test for this.
> > > 
> > > >  - a dbcheck rule for out of date @ATTRIBUTES and @INDEXLIST (rather
> > > > than a runtime check)
> > > 
> > > This is still TODO.
> > > 
> > > With the proviso that I need that dbcheck rule (or drop this aspect of
> > > the patches), can I please get your review on this series.  I'll run
> > > some more builds, but I'm pretty confident on the patch series at this
> > > point and would like to get this into master as soon as you are
> > > comfortable. 
> > > 
> > > It would be good to have at least a week of this in master before rc1
> > > freezes, so we can deal with any unexpected fallout, despite our
> > > thorough testing.
> > 
> > I've pushed the first bunch of patches.
> > 
> > You can also push the attaches two patches (which I split from one
> > single commit that looked strange).
> 
> I agree, it was strange due to the dev history, and you were right to
> pick as not ready.  I'll do that once I sort out a dbcheck rule and
> explain that the change in behaviour is highly desired.
> 
> It seems likely that we will want the behaviour change, which is not to
> rewrite @ATTRIBUTES automatically any more, as currently a 4.7 upgrade
> will force a re-index, and at scale this can be very slow.  (The re-
> index is to add the tag to show that the the feature-support is
> supported). 
> 
> Anyway, that is a task for Monday :-)

Sadly it won't happen so fast, but if you can review the rest then I
can sort out the schema mess.

Now we have proper locking, we should remove the whole
schema_refresh_fn concept, and just explicitly read the schema (via the
old schema_referesh_fn) at the start of the the transaction and search,
rather than just implicity.

Because of the read/write locks, we know they can't change so there is
no need to check at every module.

Additionally, we need to lock the DB during the schema search in the
init phase, as that isn't locked from the top level. 

Finally, (or perhaps better, first), we need to remove the auto-rewrite 
of the @INDEXLIST, as otherwise the 4.7 upgrade will involve a
compulsory transaction-held full scan and rewrite.  I had thought that
an adequate cost to make the supported-features stuff backwards
compatible, but I'm having second thoughts. 

(We just need a another way to force a re-index when we really do
change the index schema or canonical forms). 

Thanks,

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