From 96387ebcf032e1fba3e40ef35dad0b5cd00fa32c Mon Sep 17 00:00:00 2001 From: Yaron Sumel Date: Sat, 29 Jul 2017 21:04:51 +0300 Subject: [PATCH 1/5] tryLock --- zk/lock.go | 36 +++++++++++++++++++++++++++++++++--- zk/lock_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/zk/lock.go b/zk/lock.go index 3c35a427..d0a228a9 100644 --- a/zk/lock.go +++ b/zk/lock.go @@ -5,6 +5,7 @@ import ( "fmt" "strconv" "strings" + "time" ) var ( @@ -12,6 +13,8 @@ var ( ErrDeadlock = errors.New("zk: trying to acquire a lock twice") // ErrNotLocked is returned by Unlock when trying to release a lock that has not first be acquired. ErrNotLocked = errors.New("zk: not locked") + // ErrTimeout is returned by Lock when trying to lock and timeout is reached before lock is acquired. + ErrTimeout = errors.New("zk: ErrTimeout") ) // Lock is a mutual exclusion lock. @@ -20,6 +23,7 @@ type Lock struct { path string acl []ACL lockPath string + locked bool seq int } @@ -39,6 +43,29 @@ func parseSeq(path string) (int, error) { return strconv.Atoi(parts[len(parts)-1]) } +// Lock attempts to acquire the lock. It will wait up to its timeout duration +// to return until the lock is acquired or an error occurs. +// If this instance already has the lock then ErrDeadlock is returned. If timeout +// reached return ErrTimeout is returned. +func (l *Lock) TryLock(timeout time.Duration) error { + var err error + var done = make(chan struct{}, 1) + go func() { + err = l.Lock() + close(done) + }() + select { + case <-time.After(timeout): + l.locked = false + if err := l.c.Delete(l.lockPath, -1); err != nil { + return err + } + return ErrTimeout + case <-done: + return err + } +} + // Lock attempts to acquire the lock. It will wait to return until the lock // is acquired or an error occurs. If this instance already has the lock // then ErrDeadlock is returned. @@ -87,6 +114,9 @@ func (l *Lock) Lock() error { return err } + l.seq = seq + l.lockPath = path + for { children, _, err := l.c.Children(l.path) if err != nil { @@ -130,15 +160,14 @@ func (l *Lock) Lock() error { } } - l.seq = seq - l.lockPath = path + l.locked = true return nil } // Unlock releases an acquired lock. If the lock is not currently acquired by // this Lock instance than ErrNotLocked is returned. func (l *Lock) Unlock() error { - if l.lockPath == "" { + if !l.locked || l.lockPath == "" { return ErrNotLocked } if err := l.c.Delete(l.lockPath, -1); err != nil { @@ -146,5 +175,6 @@ func (l *Lock) Unlock() error { } l.lockPath = "" l.seq = 0 + l.locked = false return nil } diff --git a/zk/lock_test.go b/zk/lock_test.go index 8a3478a3..aa61d7e6 100644 --- a/zk/lock_test.go +++ b/zk/lock_test.go @@ -5,6 +5,34 @@ import ( "time" ) +func TestTryLock(t *testing.T) { + ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "}) + if err != nil { + t.Fatal(err) + } + defer ts.Stop() + zk, _, err := ts.ConnectAll() + if err != nil { + t.Fatalf("Connect returned error: %+v", err) + } + defer zk.Close() + acls := WorldACL(PermAll) + l := NewLock(zk, "/test", acls) + if err := l.TryLock(time.Second); err != nil { + t.Fatal(err) + } + if err := l.Unlock(); err != nil { + t.Fatal(err) + } + // lock to create timeout + if err := l.Lock(); err != nil { + t.Fatal(err) + } + if err := l.TryLock(time.Second); err == nil { + t.Fatal(err) + } +} + func TestLock(t *testing.T) { ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "}) if err != nil { From 60886372a65e86bee22e9d5ef8fff5bb447e7ce5 Mon Sep 17 00:00:00 2001 From: Yaron Sumel Date: Sat, 29 Jul 2017 21:07:39 +0300 Subject: [PATCH 2/5] commit to trigger travis --- zk/lock_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/zk/lock_test.go b/zk/lock_test.go index aa61d7e6..a03a0bc7 100644 --- a/zk/lock_test.go +++ b/zk/lock_test.go @@ -33,6 +33,7 @@ func TestTryLock(t *testing.T) { } } + func TestLock(t *testing.T) { ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "}) if err != nil { From 737c0a507428c6f0ff0c1cfe61de63730363a0aa Mon Sep 17 00:00:00 2001 From: Yaron Sumel Date: Sat, 29 Jul 2017 21:13:54 +0300 Subject: [PATCH 3/5] tryLock comment --- zk/lock.go | 2 +- zk/lock_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/zk/lock.go b/zk/lock.go index d0a228a9..8b1692b9 100644 --- a/zk/lock.go +++ b/zk/lock.go @@ -14,7 +14,7 @@ var ( // ErrNotLocked is returned by Unlock when trying to release a lock that has not first be acquired. ErrNotLocked = errors.New("zk: not locked") // ErrTimeout is returned by Lock when trying to lock and timeout is reached before lock is acquired. - ErrTimeout = errors.New("zk: ErrTimeout") + ErrTimeout = errors.New("zk: acquire timeout") ) // Lock is a mutual exclusion lock. diff --git a/zk/lock_test.go b/zk/lock_test.go index a03a0bc7..aa61d7e6 100644 --- a/zk/lock_test.go +++ b/zk/lock_test.go @@ -33,7 +33,6 @@ func TestTryLock(t *testing.T) { } } - func TestLock(t *testing.T) { ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "}) if err != nil { From 487d0a1cbe17a23350ebf5e2c93c1cf6253e89a9 Mon Sep 17 00:00:00 2001 From: Yaron Sumel Date: Sat, 29 Jul 2017 21:30:42 +0300 Subject: [PATCH 4/5] improve cover --- zk/lock_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/zk/lock_test.go b/zk/lock_test.go index aa61d7e6..55118d09 100644 --- a/zk/lock_test.go +++ b/zk/lock_test.go @@ -24,10 +24,16 @@ func TestTryLock(t *testing.T) { if err := l.Unlock(); err != nil { t.Fatal(err) } - // lock to create timeout - if err := l.Lock(); err != nil { - t.Fatal(err) - } + go func() { + // lock to create timeout + if err := l.Lock(); err != nil { + t.Fatal(err) + } + time.Sleep(time.Second * 10) + if err := l.Unlock(); err != nil { + t.Fatal(err) + } + }() if err := l.TryLock(time.Second); err == nil { t.Fatal(err) } From bb6d1f799eb49cdd6eb0eff2e1acb63112494cbf Mon Sep 17 00:00:00 2001 From: Yaron Sumel Date: Sat, 29 Jul 2017 22:11:40 +0300 Subject: [PATCH 5/5] fix data race in testing --- zk/lock_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/zk/lock_test.go b/zk/lock_test.go index 55118d09..11f28290 100644 --- a/zk/lock_test.go +++ b/zk/lock_test.go @@ -24,16 +24,12 @@ func TestTryLock(t *testing.T) { if err := l.Unlock(); err != nil { t.Fatal(err) } - go func() { - // lock to create timeout - if err := l.Lock(); err != nil { - t.Fatal(err) - } - time.Sleep(time.Second * 10) - if err := l.Unlock(); err != nil { - t.Fatal(err) - } - }() + // + if err := l.Lock(); err != nil { + t.Fatal(err) + } + defer l.Unlock() + // should return timeout err since lock is not released if err := l.TryLock(time.Second); err == nil { t.Fatal(err) }