[MERGE] Add new LockTimeout exception

Martin Pool mbp at sourcefrog.net
Tue Apr 29 09:14:16 BST 2008


On Tue, Apr 22, 2008 at 1:20 AM, John Arbash Meinel
<john at arbash-meinel.com> wrote:

>  The reason 'bzr serve' was setting the lock timeout was because doing:
>
>  bzr SOMETHING bzr+ssh://foobar
>
>  was blocking on a lock, but not telling the client that. This meant that it
>  could hang with no action for 5 minutes (300s) before telling the user that
>  their branch was locked.
>
>  So instead, we made it so the *service* does not block on locks, and
> possibly
>  the client does.
>
>  So I think you need to re-instate the DEFAULT_TIMEOUT_SECONDS portion of
> bzr serve.

You're right, I think I need to reconsider this patch.  It is not very
useful by itself, but I have a later in-progress one that makes the
tests for locking deterministic rather than depending on time passing
or not passing during the tests.  (Which can make the tests flakey, or
slower than necessary, and makes it hard to test whether things would
actually sleep in practice or not.)  I might just wait until I can go
back to that.

>  As for the difference between "waited and timed out" doesn't that mean the
> lock is held by someone else? I'm not sure what you are distinguishing. Is this
> "I didn't wait at all and the lock was held"?.

Yes, that was what I meant.

Possibly it's better to throw the same class of exception and just
have a parameter on it to say this.

>  If so, maybe just make that clearer from the doc strings, etc. (Change the
>  docstring for LockContention to indicate that we did not wait for the lock,
> and then make it clear in LockTimeout that we did, and the lock was still
> held.)

-- 
Martin <http://launchpad.net/~mbp/>



More information about the bazaar mailing list