process shared robust mutexes for tdb

[Volker Lendecke]

On Tue, Mar 26, 2013 at 03:21:42PM +1030, Rusty Russell wrote:
> Rusty Russell <rusty at samba.org> writes:
> > Will finish review this afternoon, but wanted to send this off now.
> 
> OK, part II:
> 
> OK, I really like the new tests you wrote.

In particular the run-mutex-die one was tricky...

> > +	tdb = tdb_open_ex("mutex.tdb", 0, tdb_flags,
> > +			  O_RDWR|O_CREAT, 0755, &log_ctx, NULL);
> > +	ok(tdb, "tdb_open_ex should succeed\n");
> 
> I'd recommend changing the name "mutex.tdb" for each test, so we can run
> them in parallel.  The simple stuff we have at the moment doesn't of
> course, but it should.

Done.

> > +	ret = tdb_chainlock(tdb, key);
> > +	ok(ret == 0, "tdb_chainlock should succeed\n");
> 
> I use ok1() everywhere: saves typing.
> 
> > @@ -279,6 +281,7 @@ static void usage(void)
> >  	printf("   -s suffix     set the backup suffix\n");
> >  	printf("   -v            verify mode (restore if corrupt)\n");
> >  	printf("   -n hashsize   set the new hash size for the backup\n");
> > +	printf("   -l            open without locking to back up mutex dbs\n");
> >  }
> 
> I'm not comfortable with tying CLEAR_IF_FIRST to TDB_MUTEX_LOCKING.
> This example shows why: it tempts people to do backup without any
> locking, which isn't really a backup.

True. But please be aware that the current code will allow
an open if the ACTIVE_LOCK is still held, even if the caller
did not specify CLEAR_IF_FIRST. So when smbd is still
running, you can do your backup.

The problem is portability and the fact that we can't even
detect that we have an incompatible mutex implementation
within the database. That's why I wanted to restrict the use
of mutexes to the bare necessary minimum. If we figure out
how to fingerprint the current mutex implementation
properly, we could do an automatic re-initialization if the
tdb is ported.

> I think we can do better: why not always hold the ACTIVE_LOCK (fcntl
> lock) if TDB_MUTEX_LOCKING?  If they *don't* specify CLEAR_IF_FIRST, and
> we get a write lock, then reset the mutexes (in theory we could resize
> them at this point if we needed to, too).

I don't think that's safe. You can't hold the ACTIVE_LOCK
from all openers, we did that initially and killed the
linked list flock implementation everywhere.
 
> > diff --git a/lib/tdb/tools/tdbtorture.c b/lib/tdb/tools/tdbtorture.c
> > index a23d154..94bfe80 100644
> > --- a/lib/tdb/tools/tdbtorture.c
> > +++ b/lib/tdb/tools/tdbtorture.c
> > @@ -17,7 +17,7 @@
> >  #define DELETE_PROB 8
> >  #define STORE_PROB 4
> >  #define APPEND_PROB 6
> > -#define TRANSACTION_PROB 10
> > +#define TRANSACTION_PROB 100
> >  #define TRANSACTION_PREPARE_PROB 2
> >  #define LOCKSTORE_PROB 5
> >  #define TRAVERSE_PROB 20
> 
> This makes transaction 10x less likely?

Oops, just removed that leftover.

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de