Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3api: Bug in watchable ranges with catchup #4191

Closed
barakmich opened this issue Jan 12, 2016 · 1 comment · Fixed by #4202
Closed

v3api: Bug in watchable ranges with catchup #4191

barakmich opened this issue Jan 12, 2016 · 1 comment · Fixed by #4202
Assignees
Labels

Comments

@barakmich
Copy link
Contributor

The API is there, and works if you don't set StartRevision -- ie, in https://github.com/coreos/etcd/blob/master/storage/watchable_store.go if the watcher is unsynced, it will never become synced, for prefixes. I added a quick (failing) test to prove it's right in here:

(add to storage/watcher_test.go)

func TestWatcherPrefix(t *testing.T) {
        b, tmpPath := backend.NewDefaultTmpBackend()
        s := WatchableKV(newWatchableStore(b, &lease.FakeLessor{}))
        defer cleanup(s, b, tmpPath)

        w := s.NewWatchStream()
        defer w.Close()

        idm := make(map[WatchID]struct{})

        for i := 0; i < 10; i++ {
                id := w.Watch([]byte("foo"), true, 0)
                if _, ok := idm[id]; ok {
                        t.Errorf("#%d: id %d exists", i, id)
                }
                idm[id] = struct{}{}

                s.Put([]byte("foobar"), []byte("baz"), lease.NoLease)

                resp := <-w.Chan()
                if resp.WatchID != id {
                        t.Errorf("#%d: watch id in event = %d, want %d", i, resp.WatchID, id)
                }

                if err := w.Cancel(id); err != nil {
                        t.Error(err)
                }
        }
        s.Put([]byte("foo2bar"), []byte("bar"), lease.NoLease)
        // unsynced watchers
        for i := 10; i < 20; i++ {
                id := w.Watch([]byte("foo2"), true, 1)
                if _, ok := idm[id]; ok {
                        t.Errorf("#%d: id %d exists", i, id)
                }
                idm[id] = struct{}{}

                resp := <-w.Chan()
                if resp.WatchID != id {
                        t.Errorf("#%d: watch id in event = %d, want %d", i, resp.WatchID, id)
                }

                if err := w.Cancel(id); err != nil {
                        t.Error(err)
                }
        }
}

Everything above unsynced watchers will pass -- but it hangs if we add the unsynced watchers part.

@gyuho gyuho self-assigned this Jan 12, 2016
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
gyuho added a commit to gyuho/etcd that referenced this issue Jan 13, 2016
Current syncWatchers method skips the events that have
prefixes that are being watched when the prefix is not
existent as a key. This fixes etcd-io#4191
by adding prefix checking to not skip those events.
@gyuho
Copy link
Contributor

gyuho commented Jan 13, 2016

@barakmich Please try again with master branch.
#4202 has just been merged.

Please ping me if you still have issues with watch.

Thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants