Merge lp://staging/~niemeyer/gozk/watches-die-on-reconnection into lp://staging/gozk/zookeeper
Proposed by
Gustavo Niemeyer
Status: | Merged |
---|---|
Merged at revision: | 35 |
Proposed branch: | lp://staging/~niemeyer/gozk/watches-die-on-reconnection |
Merge into: | lp://staging/gozk/zookeeper |
Diff against target: |
89 lines (+26/-19) 2 files modified
zk.go (+6/-5) zk_test.go (+20/-14) |
To merge this branch: | bzr merge lp://staging/~niemeyer/gozk/watches-die-on-reconnection |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
The Go Language Gophers | Pending | ||
Review via email: mp+108648@code.staging.launchpad.net |
Description of the change
Any session event on a watcher now fires and closes the watch.
Also test that closing the connection closes the watch.
To post a comment you must log in.
Reviewers: mp+108648_ code.launchpad. net,
Message:
Please take a look.
Description:
Any session event on a watcher now fires and closes the watch.
Also test that closing the connection closes the watch.
https:/ /code.launchpad .net/~niemeyer/ gozk/watches- die-on- reconnection/ +merge/ 108648
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/6292044/
Affected files:
A [revision details]
M zk.go
M zk_test.go
Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>
Index: zk.go SESSION, STATE_AUTH_FAILED: els[watchId]
=== modified file 'zk.go'
--- zk.go 2012-03-15 15:11:23 +0000
+++ zk.go 2012-06-04 22:03:50 +0000
@@ -1043,11 +1043,12 @@
return
}
if event.Type == EVENT_SESSION && watchId != conn.sessionWatchId {
- switch event.State {
- case STATE_EXPIRED_
- default:
- // WTF? Feels like TCP saying "dropped a dup packet, ok?"
- return
+ if event.State == STATE_CONNECTED {
+ // The watch was established while it was still
+ // connecting, but we take all session events on
+ // non-session watches as fatal.
+ // Change the state code to make the intent clear.
+ event.State = STATE_CONNECTING
}
}
ch := conn.watchChann
Index: zk_test.go "/test" , "one", zk.EPHEMERAL, zk.PERM_ ALL))
=== modified file 'zk_test.go'
--- zk_test.go 2012-03-15 15:11:53 +0000
+++ zk_test.go 2012-06-04 22:03:50 +0000
@@ -325,7 +325,7 @@
_, err := conn.Create(
zk.WorldACL(
c.Assert(err, IsNil)
- _, _, _, err = conn.GetW("/test")
+ _, _, watch, err := conn.GetW("/test")
c.Assert(err, IsNil)
c.Assert( zk.CountPending Watches( ), Equals, 2)
@@ -333,6 +333,13 @@
conn.Close()
c.Assert( zk.CountPending Watches( ), Equals, 0)
+
+ select {
+ case _, ok := <-watch:
+ c.Assert(ok, Equals, false)
+ case <-time.After(3e9):
+ c.Fatal("Watch didn't fire")
+ }
}
// By default, the ZooKeeper C client will hang indefinitely if a "Session watch didn't fire")
@@ -634,20 +641,19 @@
c.Fatal(
}
- // The watch channel should not, since it's not affected. "/test" , "", zk.EPHEMERAL, zk.WorldACL( zk.PERM_ ALL)) event.Type, Equals, zk.EVENT_CREATED) event.Path, Equals, "/test") event.State, Equals, zk.STATE_ CONNECTING)
+ // The watch channel should receive just the connecting notification.
select {
case event := <-watch:
- c.Fatalf("Exists watch fired: %s", event)
- default:
- }
-
- // And it should still work.
- _, err = conn.Create(
- c.Assert(err, IsNil)
-
- event = <-watch
- c.Assert(
- c.Assert(
+ c.Assert(
+ case <-time.After(3e9):
+ c.Fatal("Watch didn't fire")
+ }
+ select {
+ case _, ok := <-watch:
+ c.Assert(ok, Equals, false)
+ case <-time.After(3e9):
+ c.Fatal("Watch wasn't closed")
+ }
c.Check( zk.CountPending Watches( ), Equals, 1)
}
@@ -691,7 +697,7 @@
select { event.State, Equals, zk.STATE_ EXPIRED_ SESSION) event.State, Equals, zk.STATE_ CONNECTING)
case event := <-watch:
- c.Assert(
+ c.Assert(
...