[PATCHES] Convert gencache to dbwrap to enable mutexes

Stefan (metze) Metzmacher metze at samba.org
Tue Jun 17 05:19:32 MDT 2014


Am 13.06.2014 23:41, schrieb Christof Schmitt:
> On Fri, Jun 06, 2014 at 04:27:00PM -0700, Christof Schmitt wrote:
>> On Sat, Jun 07, 2014 at 12:47:15AM +0200, Michael Adam wrote:
>>> On 2014-06-06 at 12:40 -0700, Christof Schmitt wrote:
>>>> On Fri, Jun 06, 2014 at 09:23:35PM +0200, Michael Adam wrote:
>>>>> sn-devel-104 currently does not support robust mutexes, sorry...
>>>>>
>>>>> The sn-devel-124 based on ubuntu 12.04 that metze is preparing
>>>>> will support them. But having one test host which does support
>>>>> them is a good test as this case proves.
>>>>>
>>>>> Wouldn't the proper fix be to convert the db to us dbwrap
>>>>> instead of tdb_open_log ?
>>>>
>>>> I can take a look at that, but doesn't the current dbwrap code try to
>>>> open the database in clustering mode first? Do we need an extra flag for
>>>> dbwrap_open to force a db to be local?
>>>
>>> You can either use "ctdb:gencache_notrans.tdb = no" in
>>> smb.conf and use db_open(), (see source3/lib/dbwrap/dbwrap_open.c)
>>> or directly use dbwrap_local_open() (or db_open_tdb()).
>>>
>>> Using db_open, you can also control mutexes via
>>> "dbwrap_tdb_mutexes:gencache_notrans.tdb = yes/no".
>>
>> The check for mutex usage is in db_open and that function automatically
>> uses ctdb (unless disabled through the config). I don't think that it
>> makes sense to have the gencache_notrans tdb managed by ctdb, so
>> disabling the cluster usage should be the default for this db. I see two
>> options here:
>>  1) Add a flag to db_open to specify that this db should not be clustered.
>>  2) Also add the mutex check to dbwrap_local_open, maybe in helper function.
>>
>>> Right, at the tdb level this is deliberately so.
>>> If you pass TDB_MUTEX_LOCKING to tdb_open, it will fail
>>> the open if the runtime check fails. I.e. if the caller
>>> wants mutexes and tdb_open succees, the callser should
>>> have mutexes. The one dynamic caller-level check we have
>>> is in db_open() and I think it should stay the only one.
>>
>> Does this mean that you would prefer all tdbs to be used through
>> db_open? I agree with having only one check, i am just trying to find
>> out where that should be.
> 
> The attached patches convert gencache to use dbwrap and also introduce a
> flag to only use local databases in db_open. I have not really tested
> them, so there might still be bugs. Any comments if that is a good
> approach?

I don't like DBWRAP_FLAG_LOCAL_ONLY much. I think using dbwrap_local_open()
would be better. Even if it means we have the TDB_MUTEX_LOCKING magic twice
(maybe we can have a helper function).

While having a look at it I may found a problem with mutexes and ctdb.

        /* only pass through specific flags */
        tdb_flags &= TDB_SEQNUM|TDB_VOLATILE;

in db_open_ctdb() seems to completely useless (or wrong).
ctdbd_db_attach() is called before and gets the raw value of tdb_flags.
It has only has effect for

        db_ctdb->wtdb = tdb_wrap_open(db_ctdb, db_path, hash_size,
                                      lpcfg_tdb_flags(lp_ctx, tdb_flags),
                                      O_RDWR, 0);

So I think we can just remove tdb_flags &= TDB_SEQNUM|TDB_VOLATILE; above
and let lpcfg_tdb_flags() remove TDB_MUTEX_LOCKING when adding TDB_NOMMAP.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140617/6d141794/attachment.pgp>


More information about the samba-technical mailing list