[0.18][#109169][merge] add lock debugging, remove unnecessary lock peeks

[Martin Pool]

On 7/4/07, Robert Collins <robertc at robertcollins.net> wrote:
> On Wed, 2007-07-04 at 10:05 +1000, Martin Pool wrote:
> > > There doesn't seem to be a patch to review. But I'm fine with removing .wait()
> > > in favor of .attempt_lock()
> >
> > Sorry, here it is.  I'll post an additional patch which removes wait.
>
> +1 conditional.
>
> As long as you've considered these points I'm in favour of merging it.
>
> _lock_core is a rather ugly name. Sure its at the core, but what does it
> do? What is it responsible for?

I've renamed it to _attempt_lock, as it's the internal interface for
that function.

> Why not create brokenrename transport and server within the test module?
> Its testing code, and you even have a comment that you don't want it to
> be required to pass the transport conformance tests.

Do you mean moving it to tests.transport.brokenrename or something?

Actually I think it would/should pass the tests, but it is so trivial
as to probably not be worth running through every test.  I  put it
there because that's where vfat and similar things are.

> Why not just use delete_tree rather than trying, failing and trying
> again? The smart server does delete_tree in a single round trip (even on
> older versions) rather than many, and its possible webdav etc can do
> that too in future.

Mostly because I didn't want to make things slower on sftp, and we
should rarely need to do the delete_tree - only on servers with that
rename behaviour and only when there was lock contention.  On the
other hand just always doing the recursive deletion is simpler.

> I note that you still double-peek - why is that ?

Where?

> I think you should record 'Dlocks' in debug.py.

OK.


-- 
Martin