Updated patches for exchange setup support

[simo]

On Wed, 2012-06-13 at 11:34 -0700, Matthieu Patou wrote: 
> On 06/13/2012 10:50 AM, simo wrote:
> > On Tue, 2012-06-12 at 23:28 -0700, Matthieu Patou wrote:
> >> Andrew, Simo, Matthias,
> >>
> >> I've updated my patches here:
> >>
> >> http://gitweb.samba.org/?p=mat/samba.git;a=shortlog;h=refs/heads/schema
> >>
> >> They involves:
> >>
> >> * removing the automatic reload of the schema
> >> * adding periodic of the schema (like in Microsoft implementation)
> >> * finish handling of schemaUpdateNow so that it really cause the schema
> >> to be reloaded right now (or at the end of the transaction)
> >> * add a signaling between process so that if one process had to reload
> >> the schema due to schemaUpdateNow or due to DRS replication then other
> >> process dealing with the samdb are aware of it and reload the schema.
> >> * fix utc/generalized time attributes (we used to treat them as synonyms
> >> but there aren't)
> >> * return the timestamp of the last modified attribute/class as the
> >> modifyTimestamp attribute of CN=Aggregate,<SCHEMA_DN>
> >> * fix crackname
> >>
> >> Can you have a look ?
> > I do not understand why you are trying to access directly the underlying
> > tdb.
> Because metadata.tdb is not an ldb database, it was introduced by amitay 
> in order to have a cross partition serial number.
> >   This sounds completely wrong for various reasons, including the
> > fact it breaks the ldb abstraction (we could decide to move and use a
> > different backend, for example ntdb or something else in future).
> No because metadata is not an LDB, so you can choose to move LDB to ntdb 
> and keep metadata to tdb, of course if you want to move metadata to ntdb 
> then you have to update the calls so that they reflect the new API.

Right, which is why it makes little sense to use a different backend for
the metadata.

> > But I also do not see why you need that at all.
> Because you need to have a way for the process that is doing LDAP to 
> tell the process that is running the DCE-RPC that the schema was 
> reloaded on an explicit demand (schemaUpdateNow) and that it should do 
> so also otherwise you can end up with DRS not being able to send replica 
> update records that have just been created via LDAP.

I know why you need the serial for reloading, what I do not understand
is why you or Amitay decided to use a tdb therefore making code depend
directly no the tdb api instead of using the appropriate abstraction
layer that is LDB. Can you explain that ?

> > Why can't you simply lookup an LDB entry to save/read out the current
> The thing is that it's very very costly even if didn't looks like it, 

very very costly ?
A base search in LDB has the same cost of a TDB lockup, what is costly ?

> because in order to find your LDB entry the ldb_tdb layer has to unpack 
> all the keys up to the moment it find the key that you were looking for.

No this is not true for a base lookup, and you do not need a search
filter and a subtree or onelevel search.

> An analysis with callgrind reveal that the talloc/talloc_free done in 
> the ldb_tdb layer is very expensive (because it's done a lot).

Then maybe we should resolve that problem instead of making special
bypasses ? You pay that cost on every LDB operation already.

> Also you mis-understood the thing, we don't need an entry to save/read 
> the current schema.
> 
> First we need to avoid reloading the schema at _every_ ldb_search if it 
> has changed because it's very very expensive (as we have to basically 
> unpack all the entries in the schema partition and unpack then the data 
> and then sort in order to get quick accessors) and because Microsoft 
> doesn't do it as well, it's the kind of change that brings the schema 
> preparation for exchange from more than 3 hours to less than 10 minutes.

I understand the problem you are trying to solve perfectly. And I
suggested it was done like you do it back then when the schema was
added ... trust me, you do not need to explain to me the general
problem, I am asking about the specific solution.

> Then we need to be able to honor schemaUpdateNow right away when it's 
> received because if you received this request it's most probably because 
> the newly created attributes / classes will be used just after. If you 
> fail to reload, the creation of objects that depends on this new 
> attributes or classes will fail.
> Finally you need, as I said before, for process A to say to other 
> processes using the same samdb "I was asked to reload the schema, you 
> must reloaded it too" if you fail to do so then you are at risk of not 
> being able to send via DRS a newly created or modified object that 
> depends on the schema extensions.

Right, and in this specific case you probably wan to use an even faster
mechanism than writing to a TDB and having all processes need to
constantly poll it. A better way on Linux could be to use inotify on a
special file and have all procsses intersting in schema update check for
changes. When a schema update now command comes in, you 'touch' the file
and all other processes will be notified that said file was touched
signaling they need to check if it is time to update the schema.

> What I really need unless I completely mis-understood is a simple way of 
> doing IPC, I decided to use the existing metadata.tdb (after chatting on 
> IRC) I could have used a newly created tdb dedicated for this and then 
> just using tdb_inc_seqnum tdb_get_seqnum. If you have another way to do 
> the IPC without being very costly please tell me.

If this is such a perf. critical point then you want to use something
even lighter weight than tdb, like inotify.

> Apart from this should I conclude that the pure ldb related patches are OK ?

On a cursory look I didn't see anything grossly wrong.
And the TDB thing is not really 'wrong' either, but it seems to me you
have much better ways to notify processes.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>