[merge] add lock debugging, remove unnecessary lock peeks

Martin Pool mbp at sourcefrog.net
Wed Jun 27 23:33:40 BST 2007


On 6/28/07, John Arbash Meinel <john at arbash-meinel.com> wrote:
> John Arbash Meinel has voted -0.
> Status is now: Semi-approved
> Comment:
> I think I like the lock tracing
>
> but I'm worried about getting rid of the extra peek. Launchpad
> specifically was violating our "rename dir => existing_dir" constraint.
> Which meant that we would have been "acquiring" the lock incorrectly,
> and possibly corrupting data.
>
> LP has been fixed, but I don't know about regressions/other FS which
> might do the same. (Like WebDAV, or just a specific implementation, etc)
>
> Because we have a specific example of where it has been necessary, I
> think I would rather keep it.
>
> However, if Robert/Aaron/Martin feel like it is worth the performance
> boost, I won't block (hence -0 not -1).

OK, if we know of a case where it happened then we should certainly
leave it.  I think we can remove the other peek though, which is done
just to read the token and not to confirm it.

In fact it's good we checked that it's necessary, because now I can
add a comment explaining why we do have to do it.

I'll repost it with that left in, assuming that you thought it
acceptable in other ways.

-- 
Martin



More information about the bazaar mailing list