tdb API issues

Howard Chu hyc at highlandsun.com
Fri Apr 3 17:56:42 GMT 2009


Stefan (metze) Metzmacher wrote:
> Howard Chu schrieb:
>> Howard Chu wrote:
>>> Howard Chu wrote:
>>>>> It might also be possible to have a common virtual address space. To
>>>>> do that we'd break up the tdb_context structure into per-thread and
>>>>> per-process parts, and put the mapped pointers in the per-process
>>>>> part. It would require some thought to make sure this is safe, but at
>>>>> first glance I think its doable.
>>>>
>>>> OK, this sounds like a reasonable avenue to explore. If we also
>>>> provide some
>>>> callbacks for creating, locking/unlocking and freeing mutexes then we
>>>> can
>>>> explicitly make the relevant parts safe.
>>>
>>> I have a preliminary patch up on http://highlandsun.com/hyc/tdbdif.txt.
>>
>> The majority of the patch is purely cosmetic; I added prefixes to all
>> the tdb_context member names so that they can all be identified
>> unambiguously.
>
> Please remember we need a small atomic patches, which make sense on
> their own

OK.

> and compile and work.

Now you're just being greedy.  (Yes, it all compiles and works; if you don't 
call tdb_set_mutex() to provide the mutex callbacks then no other behaviors 
change.)

I've attached two separate patches here: one to just rename the tdb_context 
elements, and the other to split tdb_context into two pieces. I'm holding off 
on the mutex bits now because I'm not sure I'll be using them.

> So such a rename only patch can go into master now, and the rest comes
> when it's ready.
>
>> (That also makes future global replaces a lot easier...)
>> Then I split the tdb_context into a tdb_base_context which stores the
>> main state, and the tdb_context which is "per-thread". A thread calls
>> tdb_clone() to get its own working copy of a tdb_context, and all of the
>> clones share the original's tdb_base_context. In the current setup, the
>> original tdb_context must not be closed before any clones. (I guess it
>> would be smarter to allocate the tdb_base_context independently, and
>> refcount it.)

The new patch does the latter, separate alloc + refcount. So there's no 
functional difference between a clone context and the original.

>> I'm not sure yet that I've split things between the base_context and the
>> caller context correctly; this is still all a work in progress but I
>> wanted to get some early feedback.

> I thought about a similar problem but without real threads,
> we open the tdb more than once from within one process.
>
> I think in that case we should make sure that a transaction would only
> be used from one caller.
>
> The idea was to let tdb_transaction_start() return EWOULDBLOCK
> if a transaction was already started on a different tdb_context
> (currently it's tdbwrap_context) (if a transaction is started in
> a different process we would also return EWOULDBLOCK).

That makes sense. My initial approach would allow this, but I didn't keep it: 
We simply add a have_transaction_lock flag in both the tdb_base_context and 
the tdb_context. If both are true, tdb_transaction_start simply nests as 
before. If tbc-> is true but tc-> is not, then return EWOULDBLOCK.

> Somehow the caller need to have way to register for a retry event,
> but I have no specific idea for that. Maybe the caller needs pass some
> callbacks so that tdb doesn't have a dependecy to an events system.

Callbacks don't seem to offer any particular advantage here; the caller still 
has to get back to a particular polling point where it can do something 
useful. If you tried to do actual work in a callback you'd run the risk of 
stack blowout, so all you can safely do in a callback is set a flag. As such, 
it may be best to do nothing and let the caller just try transaction_start 
again later, until it stops returning EWOUDLBLOCK.

> This would solve the problem where we serve multiple LDAP client within
> one process, where each client has its own ldb_context, and we can only
> allow a transaction for one client.
>
> My first idea was to let tdb_transaction_start return a new tdb_context
> with the transaction methods activated, while the existing tdb_context
> gets readonly methods. That would mean only the caller who started the
> transaction sees the intermediate transaction states and all other's
> still see the pre transaction state of the tdb/ldb.
>
> It would be nice we can somehow combine this two tasks.

> I'm not sure if it would be possible, but I'd really like
> if two threads can choose not to block on MUTEX_LOCK.

OK.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Namespace-cleanup-add-prefix-to-tdb_context-field-n.patch
Type: text/x-patch
Size: 109054 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20090403/14a1d486/0001-Namespace-cleanup-add-prefix-to-tdb_context-field-n.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Split-tdb_context-into-two-parts-add-tdb_clone-to.patch
Type: text/x-patch
Size: 7842 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20090403/14a1d486/0002-Split-tdb_context-into-two-parts-add-tdb_clone-to.bin


More information about the samba-technical mailing list