Add in support for a TryLock to attempt to get a lock within a timeout #138
Conversation
rjemanuele
force-pushed
the
try_lock
branch
2 times, most recently
from
de630ae to
9dcb775
Compare
rjemanuele
force-pushed
the
try_lock
branch
3 times, most recently
from
fd86978 to
6928b24
Compare
rjemanuele
force-pushed
the
try_lock
branch
from
6928b24 to
7705986
Compare
|
Tests added. |
|
@jeffbean can I get a review? |
| } | ||
| val <- 3 | ||
| }() | ||
| time.Sleep(time.Millisecond * 100) |
There was a problem hiding this comment.
Tests that use sleep will produce indeterministic results. If we need to test with time, we will need to either coordinate with events or have something controlling time.
| } | ||
| case <-delay.C: | ||
| // remove the pending lock path on timeout | ||
| delErr := l.c.Delete(path, -1) |
There was a problem hiding this comment.
This would delete a lock from any node that has acquired the lock. we do not want to delete any node during a lock function.
| ev := <-ch | ||
| if ev.Err != nil { | ||
| return ev.Err | ||
| if timeout >= 0 { |
There was a problem hiding this comment.
This is a timeout waiting for events of acquiring a lock. It does not actually bound the lock timing at all outside of how long we wait. If this pattern of Trying to lock within a time boundary it would also bound the creating of znodes as well, or be very clearly documented that this duration is only applied on how long we watch for events on failure to acquire a lock.
| // It will wait to return until the lock is acquired, an error occurs, or the timeout. If | ||
| // this instance already has the lock then ErrDeadlock is returned. A negative | ||
| // timeout is waiting forever | ||
| func (l *Lock) TryLockWithData(data []byte, timeout time.Duration) error { |
There was a problem hiding this comment.
in place of any timeout here it may be best to use context.Context since this function has multiple places where we wait or loop forever. both the unconditional for loop on children, and the channel read from watch events after failing a lock.
I was looking for this feature also as we use ZK to assign workers by taking locks if available.
#116