[0.18][#109169][merge] add lock debugging, remove unnecessary lock peeks
mbp at sourcefrog.net
Wed Jul 4 10:24:19 BST 2007
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
> 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 ?
> I think you should record 'Dlocks' in debug.py.
More information about the bazaar