RFC: dbwrap_ctdb and empty vs deleted records
obnox at samba.org
Mon Jul 25 08:57:38 UTC 2016
On 2016-07-23 at 13:47 +0200, Ralph Böhme wrote:
> On Fri, Jul 22, 2016 at 11:49:34PM +0200, Ralph Böhme wrote:
> > On Fri, Jul 22, 2016 at 04:42:35PM +0200, Michael Adam wrote:
> > > Thinking aloud:
> > >
> > > The example in the commit msg is from brlock code.
> > > We'd need to lock a file, release it, have an
> > > empty brl record, and before it gets vacuumed
> > > call do_lock on the file again.
> > >
> > > So possibly by a long vacuum interval and a specially
> > > crafted sequence of file ops...
> > hm, my reading of the commit message of the revert was a bit
> > different:
> > do_lock()
> > -> grabs lock on brl record with brl_get_locks()
> > -> calls brl_lock()
> > -> calls brl_lock_posix or _windows_default()
> > -> calls contend_level2_oplocks_begin()
> > -> calls brl_locks_get_read_only()
> > -> calls dbwrap_parse_record on the same brl record as above
> > This suggest that this can be triggered by a single request calling
> > into do_lock(). Can it?
> no, it can't. Sorry, wooly thinking late at night.
> You need exactly the order of operations you described:
> - lock byterange
> - unlock -> this creates the tombstone record
> - lock byterange again
Yeah, this is exactly what I meant with the above.
(Long vac interval to make sure the tombstone rec
does not get cleaned in the meantime between
> I've added a brl test and with that am able to reproduce the deadlock
> when 925625b52886d40b50fc is reverted.
Oh, very cool! thanks for that.
I think we could bring this into master right now as a
first step, but one question: To my understanding, the
patch does not explicitly but only implicitly test the
ctdb/smbd interaction. It is written such that it should
always pass. Hence we do not need to skip it if clustering
is not active... What do you think?
> My patch does not deadlock, so it seems it's ok. Updated patchset that
> contains the test attached.
Will look again.
Cheers - Michael
> The test is not really needed to trigger the deadlock, earlier tests
> in the smb2.lock testsuite will deadlock as well. But having an
> explicit test will serve as documentation of the issue.
> Please review&push if ok.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 181 bytes
Desc: not available
More information about the samba-technical