storage: support basic watch #3507
Conversation
| @@ -38,6 +42,17 @@ type KV interface { | |||
| TxnPut(txnID int64, key, value []byte) (rev int64, err error) | |||
| TxnDeleteRange(txnID int64, key, end []byte) (n, rev int64, err error) | |||
|
|
|||
| // Watcher watches the events happening or happened in etcd. The whole | |||
There was a problem hiding this comment.
How about having another interface called WatchableKV that embeds KV?
There was a problem hiding this comment.
That sounds to abstract it better. It may cause KV to expose some new API to facilitate watch in the future or even this PR in my design. Will try.
There was a problem hiding this comment.
This can replace the old KV api. And we only expose WatchableKV to outside world.
yichengq
force-pushed
the
watch
branch
2 times, most recently
from
2d7d0e7
to
db2783d
Compare
|
Have updated the architecture based on discussion. Head to implement the left part: 1. startRev 2. endRev |
| tdelete map[string]bool | ||
| } | ||
|
|
||
| func newWatchableStore(path string) *watchableStore { |
There was a problem hiding this comment.
Let's return the interface
|
@yichengq The basic idea looks good to me. |
yichengq
force-pushed
the
watch
branch
3 times, most recently
from
b98420d
to
4ab6140
Compare
yichengq
changed the title
|
Support full Watcher feature now. The test only covers the most basic one, and it works fine. Will add more tests to follow up. This is the initial code for watch. It mostly focuses on the API and structures. PTAL |
| @@ -118,6 +120,35 @@ func (ti *treeIndex) Tombstone(key []byte, rev revision) error { | |||
| return ki.tombstone(rev.main, rev.sub) | |||
| } | |||
|
|
|||
| func (ti *treeIndex) RangeEvents(key, end []byte, rev int64) []revision { | |||
There was a problem hiding this comment.
Can we add a comment for this function? It is not quite obvious.
| // filtering without allocating | ||
| // https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating | ||
| b := s.check[:0] | ||
| for _, w := range s.check { |
There was a problem hiding this comment.
let us make this code block another func
|
LGTM in general. But we probably need one or two more rounds on this one. |
|
Addressed all comments. Ready for the next round review. |
| @@ -132,6 +132,41 @@ func (ki *keyIndex) get(atRev int64) (modified, created revision, ver int64, err | |||
| return revision{}, revision{}, 0, ErrRevisionNotFound | |||
| } | |||
|
|
|||
| // since returns recorded valid revisions since the given rev. | |||
There was a problem hiding this comment.
since returns revisions since the give rev.
Only the revision with the largest sub revision will be returned if multiple revisions have the same main revision.
|
@yichengq The code looks good to me. However, we still need to improve the comments. Basically, when we use a adj we need to be careful. The adj. has to be clear and understandable. Or we need to explain it in detail. |
yichengq
force-pushed
the
watch
branch
2 times, most recently
from
f50c5cf
to
77bab36
Compare
|
I would say the first step is to not use random adj.. |
yichengq
force-pushed
the
watch
branch
3 times, most recently
from
6dacc2b
to
be2485a
Compare
|
Let's merge this as it is. |
|
@xiang90 I will squash all commits into one. |
WatchableKV is an interface upon KV, and supports watch feature.
Each watch acclamis
446 B/op 6 allocs/op. Watch uses fixed length-10 channel, and if httpHandler wants to cache more, it could cache events at its side.