From 5394c8a894b2934b064b6eaf1b612290effc36a5 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 3 Nov 2013 23:34:47 -0800 Subject: [PATCH 01/31] feat add ttl heap --- store/heap_test.go | 36 +++++++++++++++++++++++++++++++ store/ttl_key_heap.go | 49 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 store/heap_test.go create mode 100644 store/ttl_key_heap.go diff --git a/store/heap_test.go b/store/heap_test.go new file mode 100644 index 000000000..02682b973 --- /dev/null +++ b/store/heap_test.go @@ -0,0 +1,36 @@ +package store + +import ( + "container/heap" + "fmt" + "testing" + "time" +) + +func TestHeapPushPop(t *testing.T) { + h := &TTLKeyHeap{Map: make(map[*Node]int)} + heap.Init(h) + + kvs := make([]*Node, 10) + + // add from older expire time to earlier expire time + // the path is equal to ttl from now + for i, n := range kvs { + path := fmt.Sprintf("%v", 10-i) + m := time.Duration(10 - i) + n = newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) + heap.Push(h, n) + } + + min := time.Now() + + for i := 0; i < 9; i++ { + iNode := heap.Pop(h) + node, _ := iNode.(*Node) + if node.ExpireTime.Before(min) { + t.Fatal("heap sort wrong!") + } + min = node.ExpireTime + } + +} diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go new file mode 100644 index 000000000..abb12fede --- /dev/null +++ b/store/ttl_key_heap.go @@ -0,0 +1,49 @@ +package store + +import ( + "container/heap" +) + +// An TTLKeyHeap is a min-heap of TTLKeys order by expiration time +type TTLKeyHeap struct { + Array []*Node + Map map[*Node]int +} + +func (h TTLKeyHeap) Len() int { + return len(h.Array) +} + +func (h TTLKeyHeap) Less(i, j int) bool { + return h.Array[i].ExpireTime.Before(h.Array[j].ExpireTime) +} + +func (h TTLKeyHeap) Swap(i, j int) { + // swap node + h.Array[i], h.Array[j] = h.Array[j], h.Array[i] + + // update map + h.Map[h.Array[i]] = i + h.Map[h.Array[j]] = j +} + +func (h *TTLKeyHeap) Push(x interface{}) { + n, _ := x.(*Node) + h.Map[n] = len(h.Array) + h.Array = append(h.Array, n) +} + +func (h *TTLKeyHeap) Pop() interface{} { + old := h.Array + n := len(old) + x := old[n-1] + h.Array = old[0 : n-1] + delete(h.Map, x) + return x +} + +func (h *TTLKeyHeap) Update(n *Node) { + index := h.Map[n] + heap.Remove(h, index) + heap.Push(h, n) +} From b9593b80ecd569453784e05481e944d180e344c1 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 3 Nov 2013 23:51:48 -0800 Subject: [PATCH 02/31] feat add update heap test --- store/heap_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/store/heap_test.go b/store/heap_test.go index 02682b973..2ae573724 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -11,20 +11,18 @@ func TestHeapPushPop(t *testing.T) { h := &TTLKeyHeap{Map: make(map[*Node]int)} heap.Init(h) - kvs := make([]*Node, 10) - // add from older expire time to earlier expire time // the path is equal to ttl from now - for i, n := range kvs { + for i := 0; i < 10; i++ { path := fmt.Sprintf("%v", 10-i) m := time.Duration(10 - i) - n = newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) + n := newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) heap.Push(h, n) } min := time.Now() - for i := 0; i < 9; i++ { + for i := 0; i < 10; i++ { iNode := heap.Pop(h) node, _ := iNode.(*Node) if node.ExpireTime.Before(min) { @@ -34,3 +32,54 @@ func TestHeapPushPop(t *testing.T) { } } + +func TestHeapUpdate(t *testing.T) { + h := &TTLKeyHeap{Map: make(map[*Node]int)} + heap.Init(h) + + kvs := make([]*Node, 10) + + // add from older expire time to earlier expire time + // the path is equal to ttl from now + for i, n := range kvs { + path := fmt.Sprintf("%v", 10-i) + m := time.Duration(10 - i) + n = newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) + kvs[i] = n + heap.Push(h, n) + } + + // Path 7 + kvs[3].ExpireTime = time.Now().Add(time.Second * 11) + + // Path 5 + kvs[5].ExpireTime = time.Now().Add(time.Second * 12) + + h.Update(kvs[3]) + h.Update(kvs[5]) + + min := time.Now() + + for i := 0; i < 10; i++ { + iNode := heap.Pop(h) + node, _ := iNode.(*Node) + if node.ExpireTime.Before(min) { + t.Fatal("heap sort wrong!") + } + min = node.ExpireTime + + if i == 8 { + if node.Path != "7" { + t.Fatal("heap sort wrong!", node.Path) + } + } + + if i == 9 { + if node.Path != "5" { + t.Fatal("heap sort wrong!") + } + } + + } + +} From c05df9e3f5ef37e401a959a13e7b25955cd1d15e Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 20:31:24 -0800 Subject: [PATCH 03/31] refactor add newTTLKeyHeap function --- store/heap_test.go | 3 +-- store/ttl_key_heap.go | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/store/heap_test.go b/store/heap_test.go index 2ae573724..1da81d312 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -8,8 +8,7 @@ import ( ) func TestHeapPushPop(t *testing.T) { - h := &TTLKeyHeap{Map: make(map[*Node]int)} - heap.Init(h) + h := newTTLKeyHeap() // add from older expire time to earlier expire time // the path is equal to ttl from now diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index abb12fede..23a9997f0 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -10,6 +10,12 @@ type TTLKeyHeap struct { Map map[*Node]int } +func newTTLKeyHeap() *TTLKeyHeap { + h := &TTLKeyHeap{Map: make(map[*Node]int)} + heap.Init(h) + return h +} + func (h TTLKeyHeap) Len() int { return len(h.Array) } From 3f6d6cf4c60f601162bc79e424c10cbdf90b9089 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 20:56:41 -0800 Subject: [PATCH 04/31] refactor use time.IsZero --- store/node.go | 12 +++++------- store/store.go | 4 +++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/store/node.go b/store/node.go index 0905d4d1b..b95a46e87 100644 --- a/store/node.go +++ b/store/node.go @@ -9,15 +9,13 @@ import ( etcdErr "github.com/coreos/etcd/error" ) -var ( - Permanent time.Time -) - const ( normal = iota removed ) +var Permanent time.Time + // Node is the basic element in the store system. // A key-value pair will have a string value // A directory will have a children map @@ -97,7 +95,7 @@ func (n *Node) IsHidden() bool { // IsPermanent function checks if the node is a permanent one. func (n *Node) IsPermanent() bool { - return n.ExpireTime.Sub(Permanent) == 0 + return !n.ExpireTime.IsZero() } // IsExpired function checks if the node has been expired. @@ -146,7 +144,7 @@ func (n *Node) Write(value string, index uint64, term uint64) *etcdErr.Error { } func (n *Node) ExpirationAndTTL() (*time.Time, int64) { - if n.ExpireTime.Sub(Permanent) != 0 { + if n.IsPermanent() { return &n.ExpireTime, int64(n.ExpireTime.Sub(time.Now())/time.Second) + 1 } return nil, 0 @@ -376,7 +374,7 @@ func (n *Node) UpdateTTL(expireTime time.Time) { } n.ExpireTime = expireTime - if expireTime.Sub(Permanent) != 0 { + if !n.IsPermanent() { n.Expire() } } diff --git a/store/store.go b/store/store.go index f752f6935..93278bf31 100644 --- a/store/store.go +++ b/store/store.go @@ -37,6 +37,7 @@ type Store interface { type store struct { Root *Node WatcherHub *watcherHub + TTLKeyHeap *TTLKeyHeap Index uint64 Term uint64 Stats *Stats @@ -54,6 +55,7 @@ func newStore() *store { s.Root = newDir(s, "/", UndefIndex, UndefTerm, nil, "", Permanent) s.Stats = newStats() s.WatcherHub = newWatchHub(1000) + s.TTLKeyHeap = newTTLKeyHeap() return s } @@ -390,7 +392,7 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla d.Add(n) // Node with TTL - if expireTime.Sub(Permanent) != 0 { + if !n.IsPermanent() { n.Expire() e.Expiration, e.TTL = n.ExpirationAndTTL() } From c5a6f9bb6b40e16fa9190947f75e3a1be8838d2d Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 21:22:22 -0800 Subject: [PATCH 05/31] fix iszero --- store/heap_test.go | 4 ++-- store/node.go | 33 +++++++++++++++++++++++++++++++-- store/store.go | 3 +++ store/ttl_key_heap.go | 7 ++++++- 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/store/heap_test.go b/store/heap_test.go index 1da81d312..f55413ee1 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -54,8 +54,8 @@ func TestHeapUpdate(t *testing.T) { // Path 5 kvs[5].ExpireTime = time.Now().Add(time.Second * 12) - h.Update(kvs[3]) - h.Update(kvs[5]) + h.update(kvs[3]) + h.update(kvs[5]) min := time.Now() diff --git a/store/node.go b/store/node.go index b95a46e87..50518f926 100644 --- a/store/node.go +++ b/store/node.go @@ -1,6 +1,7 @@ package store import ( + "container/heap" "path" "sort" "sync" @@ -95,7 +96,7 @@ func (n *Node) IsHidden() bool { // IsPermanent function checks if the node is a permanent one. func (n *Node) IsPermanent() bool { - return !n.ExpireTime.IsZero() + return n.ExpireTime.IsZero() } // IsExpired function checks if the node has been expired. @@ -144,7 +145,7 @@ func (n *Node) Write(value string, index uint64, term uint64) *etcdErr.Error { } func (n *Node) ExpirationAndTTL() (*time.Time, int64) { - if n.IsPermanent() { + if !n.IsPermanent() { return &n.ExpireTime, int64(n.ExpireTime.Sub(time.Now())/time.Second) + 1 } return nil, 0 @@ -239,6 +240,10 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { callback(n.Path) } + if !n.IsPermanent() { + n.store.TTLKeyHeap.remove(n) + } + // the stop channel has a buffer. just send to it! n.stopExpire <- true return @@ -257,6 +262,10 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { callback(n.Path) } + if !n.IsPermanent() { + n.store.TTLKeyHeap.remove(n) + } + n.stopExpire <- true } } @@ -362,6 +371,26 @@ func (n *Node) Pair(recurisive, sorted bool) KeyValuePair { } func (n *Node) UpdateTTL(expireTime time.Time) { + + if !n.IsPermanent() { + if expireTime.IsZero() { + // from ttl to permanent + // remove from ttl heap + n.store.TTLKeyHeap.remove(n) + } else { + // update ttl + // update ttl heap + n.store.TTLKeyHeap.update(n) + } + + } else { + if !expireTime.IsZero() { + // from permanent to ttl + // push into ttl heap + heap.Push(n.store.TTLKeyHeap, n) + } + } + if !n.IsPermanent() { // check if the node has been expired // if the node is not expired, we need to stop the go routine associated with diff --git a/store/store.go b/store/store.go index 93278bf31..aae06f92d 100644 --- a/store/store.go +++ b/store/store.go @@ -1,6 +1,7 @@ package store import ( + "container/heap" "encoding/json" "fmt" "path" @@ -393,6 +394,8 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla // Node with TTL if !n.IsPermanent() { + heap.Push(s.TTLKeyHeap, n) + n.Expire() e.Expiration, e.TTL = n.ExpirationAndTTL() } diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index 23a9997f0..9694a6abe 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -48,8 +48,13 @@ func (h *TTLKeyHeap) Pop() interface{} { return x } -func (h *TTLKeyHeap) Update(n *Node) { +func (h *TTLKeyHeap) update(n *Node) { index := h.Map[n] heap.Remove(h, index) heap.Push(h, n) } + +func (h *TTLKeyHeap) remove(n *Node) { + index := h.Map[n] + heap.Remove(h, index) +} From efe431ead0597b669b6e3c74c84806da51372b0f Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 21:33:23 -0800 Subject: [PATCH 06/31] refactor add push/pop function --- store/heap_test.go | 10 ++++------ store/ttl_key_heap.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/store/heap_test.go b/store/heap_test.go index f55413ee1..e9cda376e 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -16,14 +16,13 @@ func TestHeapPushPop(t *testing.T) { path := fmt.Sprintf("%v", 10-i) m := time.Duration(10 - i) n := newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) - heap.Push(h, n) + h.push(n) } min := time.Now() for i := 0; i < 10; i++ { - iNode := heap.Pop(h) - node, _ := iNode.(*Node) + node := h.pop() if node.ExpireTime.Before(min) { t.Fatal("heap sort wrong!") } @@ -45,7 +44,7 @@ func TestHeapUpdate(t *testing.T) { m := time.Duration(10 - i) n = newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) kvs[i] = n - heap.Push(h, n) + h.push(n) } // Path 7 @@ -60,8 +59,7 @@ func TestHeapUpdate(t *testing.T) { min := time.Now() for i := 0; i < 10; i++ { - iNode := heap.Pop(h) - node, _ := iNode.(*Node) + node := h.pop() if node.ExpireTime.Before(min) { t.Fatal("heap sort wrong!") } diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index 9694a6abe..34fa4ba63 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -48,6 +48,16 @@ func (h *TTLKeyHeap) Pop() interface{} { return x } +func (h *TTLKeyHeap) pop() *Node { + x := heap.Pop(h) + n, _ := x.(*Node) + return n +} + +func (h *TTLKeyHeap) push(x interface{}) { + heap.Push(h, x) +} + func (h *TTLKeyHeap) update(n *Node) { index := h.Map[n] heap.Remove(h, index) From 0d8510df338e88c9e422636e3e395afbfce6cc23 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 21:36:05 -0800 Subject: [PATCH 07/31] refactor use push --- store/node.go | 3 +-- store/store.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/store/node.go b/store/node.go index 50518f926..72510260d 100644 --- a/store/node.go +++ b/store/node.go @@ -1,7 +1,6 @@ package store import ( - "container/heap" "path" "sort" "sync" @@ -387,7 +386,7 @@ func (n *Node) UpdateTTL(expireTime time.Time) { if !expireTime.IsZero() { // from permanent to ttl // push into ttl heap - heap.Push(n.store.TTLKeyHeap, n) + n.store.TTLKeyHeap.push(n) } } diff --git a/store/store.go b/store/store.go index aae06f92d..babb5a932 100644 --- a/store/store.go +++ b/store/store.go @@ -1,7 +1,6 @@ package store import ( - "container/heap" "encoding/json" "fmt" "path" @@ -394,7 +393,7 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla // Node with TTL if !n.IsPermanent() { - heap.Push(s.TTLKeyHeap, n) + s.TTLKeyHeap.push(n) n.Expire() e.Expiration, e.TTL = n.ExpirationAndTTL() From 07b52ee24c768b338c5b249e409c52870b8d8e7f Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 21:51:14 -0800 Subject: [PATCH 08/31] fix save and recovery --- store/heap_test.go | 6 ++--- store/node.go | 14 +++++++----- store/store.go | 9 +++++--- store/ttl_key_heap.go | 52 +++++++++++++++++++++---------------------- 4 files changed, 43 insertions(+), 38 deletions(-) diff --git a/store/heap_test.go b/store/heap_test.go index e9cda376e..aa0b9caf6 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -1,14 +1,13 @@ package store import ( - "container/heap" "fmt" "testing" "time" ) func TestHeapPushPop(t *testing.T) { - h := newTTLKeyHeap() + h := newTtlKeyHeap() // add from older expire time to earlier expire time // the path is equal to ttl from now @@ -32,8 +31,7 @@ func TestHeapPushPop(t *testing.T) { } func TestHeapUpdate(t *testing.T) { - h := &TTLKeyHeap{Map: make(map[*Node]int)} - heap.Init(h) + h := newTtlKeyHeap() kvs := make([]*Node, 10) diff --git a/store/node.go b/store/node.go index 72510260d..38a969203 100644 --- a/store/node.go +++ b/store/node.go @@ -240,7 +240,7 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { } if !n.IsPermanent() { - n.store.TTLKeyHeap.remove(n) + n.store.ttlKeyHeap.remove(n) } // the stop channel has a buffer. just send to it! @@ -262,7 +262,7 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { } if !n.IsPermanent() { - n.store.TTLKeyHeap.remove(n) + n.store.ttlKeyHeap.remove(n) } n.stopExpire <- true @@ -375,18 +375,18 @@ func (n *Node) UpdateTTL(expireTime time.Time) { if expireTime.IsZero() { // from ttl to permanent // remove from ttl heap - n.store.TTLKeyHeap.remove(n) + n.store.ttlKeyHeap.remove(n) } else { // update ttl // update ttl heap - n.store.TTLKeyHeap.update(n) + n.store.ttlKeyHeap.update(n) } } else { if !expireTime.IsZero() { // from permanent to ttl // push into ttl heap - n.store.TTLKeyHeap.push(n) + n.store.ttlKeyHeap.push(n) } } @@ -442,5 +442,9 @@ func (n *Node) recoverAndclean() { n.stopExpire = make(chan bool, 1) + if !n.ExpireTime.IsZero() { + n.store.ttlKeyHeap.push(n) + } + n.Expire() } diff --git a/store/store.go b/store/store.go index babb5a932..f7abf50df 100644 --- a/store/store.go +++ b/store/store.go @@ -37,11 +37,11 @@ type Store interface { type store struct { Root *Node WatcherHub *watcherHub - TTLKeyHeap *TTLKeyHeap Index uint64 Term uint64 Stats *Stats CurrentVersion int + ttlKeyHeap *ttlKeyHeap // need to recovery manually worldLock sync.RWMutex // stop the world lock } @@ -55,7 +55,7 @@ func newStore() *store { s.Root = newDir(s, "/", UndefIndex, UndefTerm, nil, "", Permanent) s.Stats = newStats() s.WatcherHub = newWatchHub(1000) - s.TTLKeyHeap = newTTLKeyHeap() + s.ttlKeyHeap = newTtlKeyHeap() return s } @@ -393,7 +393,7 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla // Node with TTL if !n.IsPermanent() { - s.TTLKeyHeap.push(n) + s.ttlKeyHeap.push(n) n.Expire() e.Expiration, e.TTL = n.ExpirationAndTTL() @@ -477,6 +477,7 @@ func (s *store) Save() ([]byte, error) { b, err := json.Marshal(clonedStore) if err != nil { + fmt.Println(err) return nil, err } @@ -496,6 +497,8 @@ func (s *store) Recovery(state []byte) error { return err } + s.ttlKeyHeap = newTtlKeyHeap() + s.Root.recoverAndclean() return nil } diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index 34fa4ba63..28c38ea31 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -5,66 +5,66 @@ import ( ) // An TTLKeyHeap is a min-heap of TTLKeys order by expiration time -type TTLKeyHeap struct { - Array []*Node - Map map[*Node]int +type ttlKeyHeap struct { + array []*Node + keyMap map[*Node]int } -func newTTLKeyHeap() *TTLKeyHeap { - h := &TTLKeyHeap{Map: make(map[*Node]int)} +func newTtlKeyHeap() *ttlKeyHeap { + h := &ttlKeyHeap{keyMap: make(map[*Node]int)} heap.Init(h) return h } -func (h TTLKeyHeap) Len() int { - return len(h.Array) +func (h ttlKeyHeap) Len() int { + return len(h.array) } -func (h TTLKeyHeap) Less(i, j int) bool { - return h.Array[i].ExpireTime.Before(h.Array[j].ExpireTime) +func (h ttlKeyHeap) Less(i, j int) bool { + return h.array[i].ExpireTime.Before(h.array[j].ExpireTime) } -func (h TTLKeyHeap) Swap(i, j int) { +func (h ttlKeyHeap) Swap(i, j int) { // swap node - h.Array[i], h.Array[j] = h.Array[j], h.Array[i] + h.array[i], h.array[j] = h.array[j], h.array[i] // update map - h.Map[h.Array[i]] = i - h.Map[h.Array[j]] = j + h.keyMap[h.array[i]] = i + h.keyMap[h.array[j]] = j } -func (h *TTLKeyHeap) Push(x interface{}) { +func (h *ttlKeyHeap) Push(x interface{}) { n, _ := x.(*Node) - h.Map[n] = len(h.Array) - h.Array = append(h.Array, n) + h.keyMap[n] = len(h.array) + h.array = append(h.array, n) } -func (h *TTLKeyHeap) Pop() interface{} { - old := h.Array +func (h *ttlKeyHeap) Pop() interface{} { + old := h.array n := len(old) x := old[n-1] - h.Array = old[0 : n-1] - delete(h.Map, x) + h.array = old[0 : n-1] + delete(h.keyMap, x) return x } -func (h *TTLKeyHeap) pop() *Node { +func (h *ttlKeyHeap) pop() *Node { x := heap.Pop(h) n, _ := x.(*Node) return n } -func (h *TTLKeyHeap) push(x interface{}) { +func (h *ttlKeyHeap) push(x interface{}) { heap.Push(h, x) } -func (h *TTLKeyHeap) update(n *Node) { - index := h.Map[n] +func (h *ttlKeyHeap) update(n *Node) { + index := h.keyMap[n] heap.Remove(h, index) heap.Push(h, n) } -func (h *TTLKeyHeap) remove(n *Node) { - index := h.Map[n] +func (h *ttlKeyHeap) remove(n *Node) { + index := h.keyMap[n] heap.Remove(h, index) } From 1d49098954434c1d8edefe09b071b39a3df78cfc Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 4 Nov 2013 22:13:26 -0800 Subject: [PATCH 09/31] feat add heap top --- store/store.go | 4 ++++ store/ttl_key_heap.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/store/store.go b/store/store.go index f7abf50df..3fc8aaf2e 100644 --- a/store/store.go +++ b/store/store.go @@ -457,6 +457,10 @@ func (s *store) checkDir(parent *Node, dirName string) (*Node, *etcdErr.Error) { return n, nil } +func (s *store) MonitorTTLKeys() { + +} + // Save function saves the static state of the store system. // Save function will not be able to save the state of watchers. // Save function will not save the parent field of the node. Or there will diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index 28c38ea31..feb2ad500 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -48,6 +48,10 @@ func (h *ttlKeyHeap) Pop() interface{} { return x } +func (h *ttlKeyHeap) top() *Node { + return h.array[0] +} + func (h *ttlKeyHeap) pop() *Node { x := heap.Pop(h) n, _ := x.(*Node) From 797d996535f28c988cbc9d7a0b570e6cc0622906 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Nov 2013 21:47:25 -0800 Subject: [PATCH 10/31] init sync to delete expiring keys --- store/node.go | 108 +----------------------------------------- store/stats_test.go | 16 +++---- store/store.go | 23 +++++++-- store/store_test.go | 26 +++++++--- store/ttl_key_heap.go | 19 +++++--- 5 files changed, 61 insertions(+), 131 deletions(-) diff --git a/store/node.go b/store/node.go index 38a969203..47874514f 100644 --- a/store/node.go +++ b/store/node.go @@ -37,10 +37,6 @@ type Node struct { // A reference to the store this node is attached to. store *store - // a ttl node will have an expire routine associated with it. - // we need a channel to stop that routine when the expiration changes. - stopExpire chan bool - // ensure we only delete the node once // expire and remove may try to delete a node twice once sync.Once @@ -59,7 +55,6 @@ func newKV(store *store, nodePath string, value string, createIndex uint64, Parent: parent, ACL: ACL, store: store, - stopExpire: make(chan bool, 1), ExpireTime: expireTime, Value: value, } @@ -75,7 +70,6 @@ func newDir(store *store, nodePath string, createIndex uint64, createTerm uint64 CreateTerm: createTerm, Parent: parent, ACL: ACL, - stopExpire: make(chan bool, 1), ExpireTime: expireTime, Children: make(map[string]*Node), store: store, @@ -98,20 +92,6 @@ func (n *Node) IsPermanent() bool { return n.ExpireTime.IsZero() } -// IsExpired function checks if the node has been expired. -func (n *Node) IsExpired() (bool, time.Duration) { - if n.IsPermanent() { - return false, 0 - } - - duration := n.ExpireTime.Sub(time.Now()) - if duration <= 0 { - return true, 0 - } - - return false, duration -} - // IsDir function checks whether the node is a directory. // If the node is a directory, the function will return true. // Otherwise the function will return false. @@ -214,19 +194,6 @@ func (n *Node) Remove(recursive bool, callback func(path string)) *etcdErr.Error return etcdErr.NewError(etcdErr.EcodeNotFile, "", UndefIndex, UndefTerm) } - onceBody := func() { - n.internalRemove(recursive, callback) - } - - // this function might be entered multiple times by expire and delete - // every node will only be deleted once. - n.once.Do(onceBody) - - return nil -} - -// internalRemove function will be called by remove() -func (n *Node) internalRemove(recursive bool, callback func(path string)) { if !n.IsDir() { // key-value pair _, name := path.Split(n.Path) @@ -243,9 +210,7 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { n.store.ttlKeyHeap.remove(n) } - // the stop channel has a buffer. just send to it! - n.stopExpire <- true - return + return nil } for _, child := range n.Children { // delete all children @@ -265,61 +230,9 @@ func (n *Node) internalRemove(recursive bool, callback func(path string)) { n.store.ttlKeyHeap.remove(n) } - n.stopExpire <- true - } -} - -// Expire function will test if the node is expired. -// if the node is already expired, delete the node and return. -// if the node is permanent (this shouldn't happen), return at once. -// else wait for a period time, then remove the node. and notify the watchhub. -func (n *Node) Expire() { - expired, duration := n.IsExpired() - - if expired { // has been expired - // since the parent function of Expire() runs serially, - // there is no need for lock here - e := newEvent(Expire, n.Path, UndefIndex, UndefTerm) - n.store.WatcherHub.notify(e) - - n.Remove(true, nil) - n.store.Stats.Inc(ExpireCount) - - return } - if duration == 0 { // Permanent Node - return - } - - go func() { // do monitoring - select { - // if timeout, delete the node - case <-time.After(duration): - - // before expire get the lock, the expiration time - // of the node may be updated. - // we have to check again when get the lock - n.store.worldLock.Lock() - defer n.store.worldLock.Unlock() - - expired, _ := n.IsExpired() - - if expired { - e := newEvent(Expire, n.Path, UndefIndex, UndefTerm) - n.store.WatcherHub.notify(e) - - n.Remove(true, nil) - n.store.Stats.Inc(ExpireCount) - } - - return - - // if stopped, return - case <-n.stopExpire: - return - } - }() + return nil } func (n *Node) Pair(recurisive, sorted bool) KeyValuePair { @@ -390,21 +303,7 @@ func (n *Node) UpdateTTL(expireTime time.Time) { } } - if !n.IsPermanent() { - // check if the node has been expired - // if the node is not expired, we need to stop the go routine associated with - // that node. - expired, _ := n.IsExpired() - - if !expired { - n.stopExpire <- true // suspend it to modify the expiration - } - } - n.ExpireTime = expireTime - if !n.IsPermanent() { - n.Expire() - } } // Clone function clone the node recursively and return the new node. @@ -440,11 +339,8 @@ func (n *Node) recoverAndclean() { } } - n.stopExpire = make(chan bool, 1) - if !n.ExpireTime.IsZero() { n.store.ttlKeyHeap.push(n) } - n.Expire() } diff --git a/store/stats_test.go b/store/stats_test.go index b62473264..212c56c0a 100644 --- a/store/stats_test.go +++ b/store/stats_test.go @@ -2,7 +2,7 @@ package store import ( "testing" - "time" + //"time" "github.com/stretchr/testify/assert" ) @@ -85,10 +85,10 @@ func TestStoreStatsDeleteFail(t *testing.T) { } // Ensure that the number of expirations is recorded in the stats. -func TestStoreStatsExpireCount(t *testing.T) { - s := newStore() - s.Create("/foo", "bar", false, time.Now().Add(5 * time.Millisecond), 3, 1) - assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") - time.Sleep(10 * time.Millisecond) - assert.Equal(t, uint64(1), s.Stats.ExpireCount, "") -} +// func TestStoreStatsExpireCount(t *testing.T) { +// s := newStore() +// s.Create("/foo", "bar", false, time.Now().Add(5 * time.Millisecond), 3, 1) +// assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") +// time.Sleep(10 * time.Millisecond) +// assert.Equal(t, uint64(1), s.Stats.ExpireCount, "") +// } diff --git a/store/store.go b/store/store.go index 3fc8aaf2e..39eb1618c 100644 --- a/store/store.go +++ b/store/store.go @@ -395,7 +395,6 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla if !n.IsPermanent() { s.ttlKeyHeap.push(n) - n.Expire() e.Expiration, e.TTL = n.ExpirationAndTTL() } @@ -435,6 +434,24 @@ func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, return f, nil } +// deleteExpiredKyes will delete all +func (s *store) deleteExpiredKeys(cutoff time.Time) { + s.worldLock.Lock() + defer s.worldLock.Unlock() + + for { + node := s.ttlKeyHeap.top() + if node == nil || node.ExpireTime.After(cutoff) { + return + } + + s.ttlKeyHeap.pop() + node.Remove(true, nil) + + s.WatcherHub.notify(newEvent(Expire, node.Path, s.Index, s.Term)) + } +} + // checkDir function will check whether the component is a directory under parent node. // If it is a directory, this function will return the pointer to that node. // If it does not exist, this function will create a new directory and return the pointer to that node. @@ -457,10 +474,6 @@ func (s *store) checkDir(parent *Node, dirName string) (*Node, *etcdErr.Error) { return n, nil } -func (s *store) MonitorTTLKeys() { - -} - // Save function saves the static state of the store system. // Save function will not be able to save the state of watchers. // Save function will not save the parent field of the node. Or there will diff --git a/store/store_test.go b/store/store_test.go index 263e628ac..013656c76 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -142,12 +142,13 @@ func TestStoreUpdateFailsIfDirectory(t *testing.T) { // Ensure that the store can update the TTL on a value. func TestStoreUpdateValueTTL(t *testing.T) { s := newStore() + go mockSyncService(s.deleteExpiredKeys) s.Create("/foo", "bar", false, Permanent, 2, 1) - _, err := s.Update("/foo", "baz", time.Now().Add(1*time.Millisecond), 3, 1) + _, err := s.Update("/foo", "baz", time.Now().Add(500*time.Millisecond), 3, 1) e, _ := s.Get("/foo", false, false, 3, 1) assert.Equal(t, e.Value, "baz", "") - time.Sleep(2 * time.Millisecond) + time.Sleep(600 * time.Millisecond) e, err = s.Get("/foo", false, false, 3, 1) assert.Nil(t, e, "") assert.Equal(t, err.(*etcdErr.Error).ErrorCode, etcdErr.EcodeKeyNotFound, "") @@ -156,13 +157,14 @@ func TestStoreUpdateValueTTL(t *testing.T) { // Ensure that the store can update the TTL on a directory. func TestStoreUpdateDirTTL(t *testing.T) { s := newStore() + go mockSyncService(s.deleteExpiredKeys) s.Create("/foo", "", false, Permanent, 2, 1) s.Create("/foo/bar", "baz", false, Permanent, 3, 1) - _, err := s.Update("/foo", "", time.Now().Add(1*time.Millisecond), 3, 1) + _, err := s.Update("/foo", "", time.Now().Add(500*time.Millisecond), 3, 1) e, _ := s.Get("/foo/bar", false, false, 3, 1) assert.Equal(t, e.Value, "baz", "") - time.Sleep(2 * time.Millisecond) + time.Sleep(600 * time.Millisecond) e, err = s.Get("/foo/bar", false, false, 3, 1) assert.Nil(t, e, "") assert.Equal(t, err.(*etcdErr.Error).ErrorCode, etcdErr.EcodeKeyNotFound, "") @@ -340,11 +342,12 @@ func TestStoreWatchRecursiveCompareAndSwap(t *testing.T) { // Ensure that the store can watch for key expiration. func TestStoreWatchExpire(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, time.Now().Add(1*time.Millisecond), 2, 1) + go mockSyncService(s.deleteExpiredKeys) + s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond), 2, 1) c, _ := s.Watch("/foo", false, 0, 0, 1) e := nbselect(c) assert.Nil(t, e, "") - time.Sleep(2 * time.Millisecond) + time.Sleep(600 * time.Millisecond) e = nbselect(c) assert.Equal(t, e.Action, "expire", "") assert.Equal(t, e.Key, "/foo", "") @@ -373,6 +376,7 @@ func TestStoreRecover(t *testing.T) { // Ensure that the store can recover from a previously saved state that includes an expiring key. func TestStoreRecoverWithExpiration(t *testing.T) { s := newStore() + go mockSyncService(s.deleteExpiredKeys) s.Create("/foo", "", false, Permanent, 2, 1) s.Create("/foo/x", "bar", false, Permanent, 3, 1) s.Create("/foo/y", "baz", false, time.Now().Add(5*time.Millisecond), 4, 1) @@ -381,8 +385,11 @@ func TestStoreRecoverWithExpiration(t *testing.T) { time.Sleep(10 * time.Millisecond) s2 := newStore() + go mockSyncService(s2.deleteExpiredKeys) s2.Recovery(b) + time.Sleep(600 * time.Millisecond) + e, err := s.Get("/foo/x", false, false, 4, 1) assert.Nil(t, err, "") assert.Equal(t, e.Value, "bar", "") @@ -401,3 +408,10 @@ func nbselect(c <-chan *Event) *Event { return nil } } + +func mockSyncService(f func(now time.Time)) { + ticker := time.Tick(time.Millisecond * 500) + for now := range ticker { + f(now) + } +} diff --git a/store/ttl_key_heap.go b/store/ttl_key_heap.go index feb2ad500..0cda91d8d 100644 --- a/store/ttl_key_heap.go +++ b/store/ttl_key_heap.go @@ -49,7 +49,10 @@ func (h *ttlKeyHeap) Pop() interface{} { } func (h *ttlKeyHeap) top() *Node { - return h.array[0] + if h.Len() != 0 { + return h.array[0] + } + return nil } func (h *ttlKeyHeap) pop() *Node { @@ -63,12 +66,16 @@ func (h *ttlKeyHeap) push(x interface{}) { } func (h *ttlKeyHeap) update(n *Node) { - index := h.keyMap[n] - heap.Remove(h, index) - heap.Push(h, n) + index, ok := h.keyMap[n] + if ok { + heap.Remove(h, index) + heap.Push(h, n) + } } func (h *ttlKeyHeap) remove(n *Node) { - index := h.keyMap[n] - heap.Remove(h, index) + index, ok := h.keyMap[n] + if ok { + heap.Remove(h, index) + } } From 49c55477e567695b068cf1d94f52e1d437d4bf51 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Nov 2013 21:56:21 -0800 Subject: [PATCH 11/31] refactor clean up --- store/node.go | 5 ----- store/stats_test.go | 19 ++++++++++--------- store/store.go | 1 + 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/store/node.go b/store/node.go index 47874514f..b3daa1dfc 100644 --- a/store/node.go +++ b/store/node.go @@ -3,7 +3,6 @@ package store import ( "path" "sort" - "sync" "time" etcdErr "github.com/coreos/etcd/error" @@ -36,10 +35,6 @@ type Node struct { // A reference to the store this node is attached to. store *store - - // ensure we only delete the node once - // expire and remove may try to delete a node twice - once sync.Once } // newKV creates a Key-Value pair diff --git a/store/stats_test.go b/store/stats_test.go index 212c56c0a..9b3172ad8 100644 --- a/store/stats_test.go +++ b/store/stats_test.go @@ -2,7 +2,7 @@ package store import ( "testing" - //"time" + "time" "github.com/stretchr/testify/assert" ) @@ -84,11 +84,12 @@ func TestStoreStatsDeleteFail(t *testing.T) { assert.Equal(t, uint64(1), s.Stats.DeleteFail, "") } -// Ensure that the number of expirations is recorded in the stats. -// func TestStoreStatsExpireCount(t *testing.T) { -// s := newStore() -// s.Create("/foo", "bar", false, time.Now().Add(5 * time.Millisecond), 3, 1) -// assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") -// time.Sleep(10 * time.Millisecond) -// assert.Equal(t, uint64(1), s.Stats.ExpireCount, "") -// } +//Ensure that the number of expirations is recorded in the stats. +func TestStoreStatsExpireCount(t *testing.T) { + s := newStore() + go mockSyncService(s.deleteExpiredKeys) + s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond), 3, 1) + assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") + time.Sleep(600 * time.Millisecond) + assert.Equal(t, uint64(1), s.Stats.ExpireCount, "") +} diff --git a/store/store.go b/store/store.go index 39eb1618c..abffabf74 100644 --- a/store/store.go +++ b/store/store.go @@ -448,6 +448,7 @@ func (s *store) deleteExpiredKeys(cutoff time.Time) { s.ttlKeyHeap.pop() node.Remove(true, nil) + s.Stats.Inc(ExpireCount) s.WatcherHub.notify(newEvent(Expire, node.Path, s.Index, s.Term)) } } From 779195eb4fd741a2f29ddbd46531d2fe8a5db553 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Nov 2013 22:18:54 -0800 Subject: [PATCH 12/31] fix bug in update ttl --- store/node.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/store/node.go b/store/node.go index b3daa1dfc..33ae76692 100644 --- a/store/node.go +++ b/store/node.go @@ -286,6 +286,7 @@ func (n *Node) UpdateTTL(expireTime time.Time) { n.store.ttlKeyHeap.remove(n) } else { // update ttl + n.ExpireTime = expireTime // update ttl heap n.store.ttlKeyHeap.update(n) } @@ -293,12 +294,11 @@ func (n *Node) UpdateTTL(expireTime time.Time) { } else { if !expireTime.IsZero() { // from permanent to ttl + n.ExpireTime = expireTime // push into ttl heap n.store.ttlKeyHeap.push(n) } } - - n.ExpireTime = expireTime } // Clone function clone the node recursively and return the new node. From 55058c64f55a340b1b78a4cfa55aaae9c6775051 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Tue, 5 Nov 2013 23:30:48 -0800 Subject: [PATCH 13/31] feat wathch for expiring need to be pending --- store/store.go | 2 ++ store/watcher_hub.go | 26 ++++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/store/store.go b/store/store.go index abffabf74..14d480b9c 100644 --- a/store/store.go +++ b/store/store.go @@ -451,6 +451,8 @@ func (s *store) deleteExpiredKeys(cutoff time.Time) { s.Stats.Inc(ExpireCount) s.WatcherHub.notify(newEvent(Expire, node.Path, s.Index, s.Term)) } + + s.WatcherHub.clearPendingWatchers() } // checkDir function will check whether the component is a directory under parent node. diff --git a/store/watcher_hub.go b/store/watcher_hub.go index 4c4bfd29d..9e0285b47 100644 --- a/store/watcher_hub.go +++ b/store/watcher_hub.go @@ -16,9 +16,10 @@ import ( // event happens between the end of the first watch command and the start // of the second command. type watcherHub struct { - watchers map[string]*list.List - count int64 // current number of watchers. - EventHistory *EventHistory + watchers map[string]*list.List + count int64 // current number of watchers. + EventHistory *EventHistory + pendingWatchers *list.List } // newWatchHub creates a watchHub. The capacity determines how many events we will @@ -27,8 +28,9 @@ type watcherHub struct { // Ideally, it should smaller than 20K/s[max throughput] * 2 * 50ms[RTT] = 2000 func newWatchHub(capacity int) *watcherHub { return &watcherHub{ - watchers: make(map[string]*list.List), - EventHistory: newEventHistory(capacity), + watchers: make(map[string]*list.List), + EventHistory: newEventHistory(capacity), + pendingWatchers: list.New(), } } @@ -117,9 +119,13 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { // if we successfully notify a watcher // we need to remove the watcher from the list // and decrease the counter - l.Remove(curr) atomic.AddInt64(&wh.count, -1) + + if e.Action == Expire { + wh.pendingWatchers.PushBack(w) + } + } else { // once there is a watcher in the list is not interested // in the event, we should keep the list in the map @@ -131,6 +137,14 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { } } +func (wh *watcherHub) clearPendingWatchers() { + for e := wh.pendingWatchers.Front(); e != nil; e = e.Next() { + w, _ := e.Value.(*watcher) + w.eventChan <- nil + } + wh.pendingWatchers = list.New() +} + // clone function clones the watcherHub and return the cloned one. // only clone the static content. do not clone the current watchers. func (wh *watcherHub) clone() *watcherHub { From c307b6abcad43b022a368659d901a3669c5d9601 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Wed, 6 Nov 2013 21:19:37 -0800 Subject: [PATCH 14/31] fix watcher_hub --- store/event_history.go | 30 ++++++++++++------- store/event_test.go | 12 ++++---- store/store.go | 5 ++-- store/store_test.go | 11 +++++-- store/watcher.go | 1 - store/watcher_hub.go | 65 ++++++++++++++++++++++++++---------------- 6 files changed, 77 insertions(+), 47 deletions(-) diff --git a/store/event_history.go b/store/event_history.go index 3ddd38206..ce21ec1fc 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -31,19 +31,14 @@ func (eh *EventHistory) addEvent(e *Event) *Event { eh.rwl.Lock() defer eh.rwl.Unlock() - var duped uint64 - - if e.Index == UndefIndex { - e.Index = eh.LastIndex - e.Term = eh.LastTerm - duped = 1 + if e.Index == eh.LastIndex { + eh.DupCnt += 1 } eh.Queue.insert(e) eh.LastIndex = e.Index eh.LastTerm = e.Term - eh.DupCnt += duped eh.StartIndex = eh.Queue.Events[eh.Queue.Front].Index @@ -52,7 +47,7 @@ func (eh *EventHistory) addEvent(e *Event) *Event { // scan function is enumerating events from the index in history and // stops till the first point where the key has identified prefix -func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Error) { +func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Error) { eh.rwl.RLock() defer eh.rwl.RUnlock() @@ -73,16 +68,29 @@ func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Erro i := int((start + uint64(eh.Queue.Front)) % uint64(eh.Queue.Capacity)) + events := make([]*Event, 0) + var eventIndex uint64 + for { e := eh.Queue.Events[i] + + if eventIndex != 0 && eventIndex != e.Index { + return events, nil + } + if strings.HasPrefix(e.Key, prefix) && index <= e.Index { // make sure we bypass the smaller one - return e, nil + eventIndex = e.Index + events = append(events, e) } i = (i + 1) % eh.Queue.Capacity - if i == eh.Queue.back() { // find nothing, return and watch from current index - return nil, nil + if i == eh.Queue.back() { + if eventIndex == 0 { // find nothing, return and watch from current index + return nil, nil + } + + return events, nil } } } diff --git a/store/event_test.go b/store/event_test.go index c02a4d70e..aedf7f7da 100644 --- a/store/event_test.go +++ b/store/event_test.go @@ -42,20 +42,20 @@ func TestScanHistory(t *testing.T) { eh.addEvent(newEvent(Create, "/foo/foo/foo", 5, 1)) e, err := eh.scan("/foo", 1) - if err != nil || e.Index != 1 { - t.Fatalf("scan error [/foo] [1] %v", e.Index) + if err != nil || e[0].Index != 1 { + t.Fatalf("scan error [/foo] [1] %v", e[0].Index) } e, err = eh.scan("/foo/bar", 1) - if err != nil || e.Index != 2 { - t.Fatalf("scan error [/foo/bar] [2] %v", e.Index) + if err != nil || e[0].Index != 2 { + t.Fatalf("scan error [/foo/bar] [2] %v", e[0].Index) } e, err = eh.scan("/foo/bar", 3) - if err != nil || e.Index != 4 { - t.Fatalf("scan error [/foo/bar/bar] [4] %v", e.Index) + if err != nil || e[0].Index != 4 { + t.Fatalf("scan error [/foo/bar/bar] [4] %v", e[0].Index) } e, err = eh.scan("/foo/bar", 6) diff --git a/store/store.go b/store/store.go index 14d480b9c..6190e354d 100644 --- a/store/store.go +++ b/store/store.go @@ -435,10 +435,12 @@ func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, } // deleteExpiredKyes will delete all -func (s *store) deleteExpiredKeys(cutoff time.Time) { +func (s *store) deleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { s.worldLock.Lock() defer s.worldLock.Unlock() + s.Index, s.Term = index, term + for { node := s.ttlKeyHeap.top() if node == nil || node.ExpireTime.After(cutoff) { @@ -497,7 +499,6 @@ func (s *store) Save() ([]byte, error) { b, err := json.Marshal(clonedStore) if err != nil { - fmt.Println(err) return nil, err } diff --git a/store/store_test.go b/store/store_test.go index 013656c76..1fc242d91 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -344,13 +344,18 @@ func TestStoreWatchExpire(t *testing.T) { s := newStore() go mockSyncService(s.deleteExpiredKeys) s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond), 2, 1) - c, _ := s.Watch("/foo", false, 0, 0, 1) + s.Create("/foofoo", "barbarbar", false, time.Now().Add(500*time.Millisecond), 2, 1) + + c, _ := s.Watch("/", true, 0, 0, 1) e := nbselect(c) assert.Nil(t, e, "") time.Sleep(600 * time.Millisecond) e = nbselect(c) assert.Equal(t, e.Action, "expire", "") assert.Equal(t, e.Key, "/foo", "") + e = nbselect(c) + assert.Equal(t, e.Action, "expire", "") + assert.Equal(t, e.Key, "/foofoo", "") } // Ensure that the store can recover from a previously saved state. @@ -409,9 +414,9 @@ func nbselect(c <-chan *Event) *Event { } } -func mockSyncService(f func(now time.Time)) { +func mockSyncService(f func(now time.Time, index uint64, term uint64)) { ticker := time.Tick(time.Millisecond * 500) for now := range ticker { - f(now) + f(now, 2, 1) } } diff --git a/store/watcher.go b/store/watcher.go index b9cb5499c..2015d0072 100644 --- a/store/watcher.go +++ b/store/watcher.go @@ -24,7 +24,6 @@ func (w *watcher) notify(e *Event, originalPath bool, deleted bool) bool { // at the file we need to delete. // For example a watcher is watching at "/foo/bar". And we deletes "/foo". The watcher // should get notified even if "/foo" is not the path it is watching. - if (w.recursive || originalPath || deleted) && e.Index >= w.sinceIndex { w.eventChan <- e return true diff --git a/store/watcher_hub.go b/store/watcher_hub.go index 9e0285b47..33eda248e 100644 --- a/store/watcher_hub.go +++ b/store/watcher_hub.go @@ -19,7 +19,8 @@ type watcherHub struct { watchers map[string]*list.List count int64 // current number of watchers. EventHistory *EventHistory - pendingWatchers *list.List + pendingWatchers map[*list.Element]*list.List + pendingList map[*list.List]string } // newWatchHub creates a watchHub. The capacity determines how many events we will @@ -30,7 +31,8 @@ func newWatchHub(capacity int) *watcherHub { return &watcherHub{ watchers: make(map[string]*list.List), EventHistory: newEventHistory(capacity), - pendingWatchers: list.New(), + pendingWatchers: make(map[*list.Element]*list.List), + pendingList: make(map[*list.List]string), } } @@ -39,23 +41,30 @@ func newWatchHub(capacity int) *watcherHub { // If recursive is false, the first change after index at prefix will be sent to the event channel. // If index is zero, watch will start from the current index + 1. func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan *Event, *etcdErr.Error) { - eventChan := make(chan *Event, 1) - - e, err := wh.EventHistory.scan(prefix, index) + events, err := wh.EventHistory.scan(prefix, index) if err != nil { return nil, err } - if e != nil { - eventChan <- e + eventChan := make(chan *Event, len(events)+5) // use a buffered channel + + if events != nil { + for _, e := range events { + eventChan <- e + } + + if len(events) > 1 { + eventChan <- nil + } + return eventChan, nil } w := &watcher{ eventChan: eventChan, recursive: recursive, - sinceIndex: index - 1, // to catch Expire() + sinceIndex: index, } l, ok := wh.watchers[prefix] @@ -95,19 +104,16 @@ func (wh *watcherHub) notify(e *Event) { func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { l, ok := wh.watchers[path] - if ok { curr := l.Front() - notifiedAll := true for { if curr == nil { // we have reached the end of the list - if notifiedAll { + if l.Len() == 0 { // if we have notified all watcher in the list // we can delete the list delete(wh.watchers, path) } - break } @@ -116,20 +122,18 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { w, _ := curr.Value.(*watcher) if w.notify(e, e.Key == path, deleted) { - // if we successfully notify a watcher - // we need to remove the watcher from the list - // and decrease the counter - l.Remove(curr) - atomic.AddInt64(&wh.count, -1) if e.Action == Expire { - wh.pendingWatchers.PushBack(w) + wh.pendingWatchers[curr] = l + wh.pendingList[l] = path + } else { + // if we successfully notify a watcher + // we need to remove the watcher from the list + // and decrease the counter + l.Remove(curr) + atomic.AddInt64(&wh.count, -1) } - } else { - // once there is a watcher in the list is not interested - // in the event, we should keep the list in the map - notifiedAll = false } curr = next // update current to the next @@ -138,11 +142,24 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { } func (wh *watcherHub) clearPendingWatchers() { - for e := wh.pendingWatchers.Front(); e != nil; e = e.Next() { + if len(wh.pendingWatchers) == 0 { // avoid making new maps + return + } + + for e, l := range wh.pendingWatchers { + l.Remove(e) + + if l.Len() == 0 { + path := wh.pendingList[l] + delete(wh.watchers, path) + } + w, _ := e.Value.(*watcher) w.eventChan <- nil } - wh.pendingWatchers = list.New() + + wh.pendingWatchers = make(map[*list.Element]*list.List) + wh.pendingList = make(map[*list.List]string) } // clone function clones the watcherHub and return the cloned one. From 4c1d86409503d20b6661cfd78a05463c78057586 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Wed, 6 Nov 2013 21:27:39 -0800 Subject: [PATCH 15/31] fix cleanup --- store/store.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/store/store.go b/store/store.go index 6190e354d..debf1192e 100644 --- a/store/store.go +++ b/store/store.go @@ -444,7 +444,7 @@ func (s *store) deleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { for { node := s.ttlKeyHeap.top() if node == nil || node.ExpireTime.After(cutoff) { - return + break } s.ttlKeyHeap.pop() @@ -453,7 +453,6 @@ func (s *store) deleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { s.Stats.Inc(ExpireCount) s.WatcherHub.notify(newEvent(Expire, node.Path, s.Index, s.Term)) } - s.WatcherHub.clearPendingWatchers() } From 28ac516f68be7d715eb47df889c8abea9bd721f0 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 7 Nov 2013 20:55:26 -0800 Subject: [PATCH 16/31] fix sinceIndex --- server/v2/tests/get_handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index d6ceae58a..3b365872c 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -118,7 +118,7 @@ func TestV2WatchKeyWithIndex(t *testing.T) { var body map[string]interface{} c := make(chan bool) go func() { - resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=5")) + resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=4")) body = tests.ReadBodyJSON(resp) c <- true }() From b4f4528ef46bbf6dd291697375953b5fb9c3945c Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 7 Nov 2013 22:29:15 -0800 Subject: [PATCH 17/31] feat upgrade get_handler --- server/v2/get_handler.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index 39e932fe0..afa44c9b8 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -16,6 +16,7 @@ import ( func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { var err error var event *store.Event + events := make([]*store.Event, 0) vars := mux.Vars(req) key := "/" + vars["key"] @@ -54,10 +55,17 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { cn, _ := w.(http.CloseNotifier) closeChan := cn.CloseNotify() - select { - case <-closeChan: - return nil - case event = <-eventChan: + for { + select { + case <-closeChan: + return nil + case event = <-eventChan: + if event != nil && event.Action == store.Expire { + events = append(events, event) + } else { + goto finish + } + } } } else { //get @@ -68,11 +76,19 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { } } +finish: + w.Header().Add("X-Etcd-Index", fmt.Sprint(event.Index)) w.Header().Add("X-Etcd-Term", fmt.Sprint(event.Term)) w.WriteHeader(http.StatusOK) - b, _ := json.Marshal(event) + var b []byte + + if len(events) == 0 { + b, _ = json.Marshal(event) + } else { + b, _ = json.Marshal(events) + } w.Write(b) return nil From 5a4e764d7a2f39ea5b0984dec24775c4d8bdc71b Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 8 Nov 2013 13:24:23 -0800 Subject: [PATCH 18/31] refactor add comments for receiving expire commands --- server/v2/get_handler.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index afa44c9b8..212c32297 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -60,6 +60,9 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { case <-closeChan: return nil case event = <-eventChan: + // for events other than expire, just one event for one watch + // for expire event, we might have a stream of events + // we use a nil item to terminate the expire event stream if event != nil && event.Action == store.Expire { events = append(events, event) } else { From acd940a450dc620d9b3484a8f0369a17d5a394ba Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 8 Nov 2013 13:37:30 -0800 Subject: [PATCH 19/31] refactor comments on IsPermanent --- store/node.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/store/node.go b/store/node.go index 33ae76692..6b11727c8 100644 --- a/store/node.go +++ b/store/node.go @@ -84,6 +84,9 @@ func (n *Node) IsHidden() bool { // IsPermanent function checks if the node is a permanent one. func (n *Node) IsPermanent() bool { + // we use a uninitialized time.Time to indicate the node is a + // permanent one. + // the uninitialized time.Time should equal zero. return n.ExpireTime.IsZero() } From 0372cdea2326506a4bb6d6e7ddab43b81e686649 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 8 Nov 2013 16:00:58 -0800 Subject: [PATCH 20/31] refactor move sync command into etcd --- server/peer_server.go | 14 ++++++++++++++ server/v2/get_handler.go | 20 ++++++++------------ store/command_factory.go | 1 + store/event_history.go | 11 ++++------- store/store.go | 3 ++- store/v2/command_factory.go | 22 +++++++++++++--------- store/v2/delete_command.go | 2 +- store/watcher_hub.go | 2 +- 8 files changed, 44 insertions(+), 31 deletions(-) diff --git a/server/peer_server.go b/server/peer_server.go index bfa2ea741..018c48110 100644 --- a/server/peer_server.go +++ b/server/peer_server.go @@ -136,6 +136,8 @@ func (s *PeerServer) ListenAndServe(snapshot bool, cluster []string) error { log.Debugf("%s restart as a follower", s.name) } + go s.monitorSync() + // open the snapshot if snapshot { go s.monitorSnapshot() @@ -424,3 +426,15 @@ func (s *PeerServer) monitorSnapshot() { } } } + +func (s *PeerServer) monitorSync() { + ticker := time.Tick(time.Millisecond * 500) + for { + select { + case now := <-ticker: + if s.raftServer.State() == raft.Leader { + s.raftServer.Do(s.store.CommandFactory().CreateSyncCommand(now)) + } + } + } +} diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index 212c32297..dba491cbd 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -55,6 +55,7 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { cn, _ := w.(http.CloseNotifier) closeChan := cn.CloseNotify() + eventLoop: for { select { case <-closeChan: @@ -66,7 +67,8 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { if event != nil && event.Action == store.Expire { events = append(events, event) } else { - goto finish + events = append(events, event) + break eventLoop } } } @@ -79,19 +81,13 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { } } -finish: - - w.Header().Add("X-Etcd-Index", fmt.Sprint(event.Index)) - w.Header().Add("X-Etcd-Term", fmt.Sprint(event.Term)) - w.WriteHeader(http.StatusOK) - var b []byte - if len(events) == 0 { - b, _ = json.Marshal(event) - } else { - b, _ = json.Marshal(events) - } + w.Header().Add("X-Etcd-Index", fmt.Sprint(events[0].Index)) + w.Header().Add("X-Etcd-Term", fmt.Sprint(events[0].Term)) + w.WriteHeader(http.StatusOK) + b, _ = json.Marshal(events) + w.Write(b) return nil diff --git a/store/command_factory.go b/store/command_factory.go index 9b52f42f9..fc9199058 100644 --- a/store/command_factory.go +++ b/store/command_factory.go @@ -21,6 +21,7 @@ type CommandFactory interface { CreateUpdateCommand(key string, value string, expireTime time.Time) raft.Command CreateDeleteCommand(key string, recursive bool) raft.Command CreateCompareAndSwapCommand(key string, value string, prevValue string, prevIndex uint64, expireTime time.Time) raft.Command + CreateSyncCommand(now time.Time) raft.Command } // RegisterCommandFactory adds a command factory to the global registry. diff --git a/store/event_history.go b/store/event_history.go index ce21ec1fc..4d11a8883 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -51,10 +51,8 @@ func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Er eh.rwl.RLock() defer eh.rwl.RUnlock() - start := index - eh.StartIndex - // the index should locate after the event history's StartIndex - if start < 0 { + if index-eh.StartIndex < 0 { return nil, etcdErr.NewError(etcdErr.EcodeEventIndexCleared, fmt.Sprintf("the requested history has been cleared [%v/%v]", @@ -62,11 +60,11 @@ func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Er } // the index should locate before the size of the queue minus the duplicate count - if start >= (uint64(eh.Queue.Size) - eh.DupCnt) { // future index + if index > eh.LastIndex { // future index return nil, nil } - i := int((start + uint64(eh.Queue.Front)) % uint64(eh.Queue.Capacity)) + i := eh.Queue.Front events := make([]*Event, 0) var eventIndex uint64 @@ -85,11 +83,10 @@ func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Er i = (i + 1) % eh.Queue.Capacity - if i == eh.Queue.back() { + if i > eh.Queue.back() { if eventIndex == 0 { // find nothing, return and watch from current index return nil, nil } - return events, nil } } diff --git a/store/store.go b/store/store.go index debf1192e..5e374f1ff 100644 --- a/store/store.go +++ b/store/store.go @@ -32,6 +32,7 @@ type Store interface { Recovery(state []byte) error TotalTransactions() uint64 JsonStats() []byte + DeleteExpiredKeys(cutoff time.Time, index uint64, term uint64) } type store struct { @@ -435,7 +436,7 @@ func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, } // deleteExpiredKyes will delete all -func (s *store) deleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { +func (s *store) DeleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { s.worldLock.Lock() defer s.worldLock.Unlock() diff --git a/store/v2/command_factory.go b/store/v2/command_factory.go index 4f0e7260c..8332891d6 100644 --- a/store/v2/command_factory.go +++ b/store/v2/command_factory.go @@ -2,7 +2,7 @@ package v2 import ( "time" - + "github.com/coreos/etcd/store" "github.com/coreos/go-raft" ) @@ -28,8 +28,8 @@ func (f *CommandFactory) CreateUpgradeCommand() raft.Command { // CreateSetCommand creates a version 2 command to set a key to a given value in the store. func (f *CommandFactory) CreateSetCommand(key string, value string, expireTime time.Time) raft.Command { return &SetCommand{ - Key: key, - Value: value, + Key: key, + Value: value, ExpireTime: expireTime, } } @@ -37,18 +37,18 @@ func (f *CommandFactory) CreateSetCommand(key string, value string, expireTime t // CreateCreateCommand creates a version 2 command to create a new key in the store. func (f *CommandFactory) CreateCreateCommand(key string, value string, expireTime time.Time, unique bool) raft.Command { return &CreateCommand{ - Key: key, - Value: value, + Key: key, + Value: value, ExpireTime: expireTime, - Unique: unique, + Unique: unique, } } // CreateUpdateCommand creates a version 2 command to update a key to a given value in the store. func (f *CommandFactory) CreateUpdateCommand(key string, value string, expireTime time.Time) raft.Command { return &UpdateCommand{ - Key: key, - Value: value, + Key: key, + Value: value, ExpireTime: expireTime, } } @@ -56,7 +56,7 @@ func (f *CommandFactory) CreateUpdateCommand(key string, value string, expireTim // CreateDeleteCommand creates a version 2 command to delete a key from the store. func (f *CommandFactory) CreateDeleteCommand(key string, recursive bool) raft.Command { return &DeleteCommand{ - Key: key, + Key: key, Recursive: recursive, } } @@ -71,3 +71,7 @@ func (f *CommandFactory) CreateCompareAndSwapCommand(key string, value string, p ExpireTime: expireTime, } } + +func (f *CommandFactory) CreateSyncCommand(now time.Time) raft.Command { + return &SyncCommand{time.Now()} +} diff --git a/store/v2/delete_command.go b/store/v2/delete_command.go index 6bd48368f..3e8bac81c 100644 --- a/store/v2/delete_command.go +++ b/store/v2/delete_command.go @@ -1,8 +1,8 @@ package v2 import ( - "github.com/coreos/etcd/store" "github.com/coreos/etcd/log" + "github.com/coreos/etcd/store" "github.com/coreos/go-raft" ) diff --git a/store/watcher_hub.go b/store/watcher_hub.go index 33eda248e..52af68c21 100644 --- a/store/watcher_hub.go +++ b/store/watcher_hub.go @@ -54,7 +54,7 @@ func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan eventChan <- e } - if len(events) > 1 { + if events[0].Action == Expire { eventChan <- nil } From 6156d5c790b7a3c112353ade581cbea90cb2d0f6 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 17:55:54 -0800 Subject: [PATCH 21/31] refactor separate etcd index from raft index --- error/error.go | 5 +- server/join_command.go | 20 +-- server/registry.go | 18 +-- server/remove_command.go | 2 +- server/server.go | 8 +- server/v1/get_key_handler.go | 2 +- server/v1/set_key_handler.go | 4 +- server/v1/watch_key_handler.go | 7 +- server/v2/get_handler.go | 36 ++--- server/v2/post_handler.go | 2 +- server/v2/put_handler.go | 8 +- server/v2/tests/delete_handler_test.go | 2 +- server/v2/tests/get_handler_test.go | 23 ++- server/v2/tests/post_handler_test.go | 8 +- server/v2/tests/put_handler_test.go | 22 +-- store/event.go | 11 +- store/event_history.go | 28 +--- store/event_test.go | 24 +-- store/heap_test.go | 4 +- store/node.go | 37 ++--- store/stats_test.go | 38 ++--- store/store.go | 151 ++++++++++--------- store/store_test.go | 200 ++++++++++++------------- store/v2/compare_and_swap_command.go | 3 +- store/v2/create_command.go | 2 +- store/v2/delete_command.go | 2 +- store/v2/set_command.go | 2 +- store/v2/update_command.go | 2 +- store/watcher_hub.go | 64 ++------ store/watcher_test.go | 6 +- 30 files changed, 334 insertions(+), 407 deletions(-) diff --git a/error/error.go b/error/error.go index 22b55906b..e4555eb6e 100644 --- a/error/error.go +++ b/error/error.go @@ -64,16 +64,14 @@ type Error struct { Message string `json:"message"` Cause string `json:"cause,omitempty"` Index uint64 `json:"index"` - Term uint64 `json:"term"` } -func NewError(errorCode int, cause string, index uint64, term uint64) *Error { +func NewError(errorCode int, cause string, index uint64) *Error { return &Error{ ErrorCode: errorCode, Message: errors[errorCode], Cause: cause, Index: index, - Term: term, } } @@ -93,7 +91,6 @@ func (e Error) toJsonString() string { func (e Error) Write(w http.ResponseWriter) { w.Header().Add("X-Etcd-Index", fmt.Sprint(e.Index)) - w.Header().Add("X-Etcd-Term", fmt.Sprint(e.Term)) // 3xx is reft internal error if e.ErrorCode/100 == 3 { http.Error(w, e.toJsonString(), http.StatusInternalServerError) diff --git a/server/join_command.go b/server/join_command.go index 49dab4f2e..6e7f7471b 100644 --- a/server/join_command.go +++ b/server/join_command.go @@ -14,20 +14,20 @@ func init() { // The JoinCommand adds a node to the cluster. type JoinCommand struct { - MinVersion int `json:"minVersion"` - MaxVersion int `json:"maxVersion"` - Name string `json:"name"` - RaftURL string `json:"raftURL"` - EtcdURL string `json:"etcdURL"` + MinVersion int `json:"minVersion"` + MaxVersion int `json:"maxVersion"` + Name string `json:"name"` + RaftURL string `json:"raftURL"` + EtcdURL string `json:"etcdURL"` } func NewJoinCommand(minVersion int, maxVersion int, name, raftUrl, etcdUrl string) *JoinCommand { return &JoinCommand{ MinVersion: minVersion, MaxVersion: maxVersion, - Name: name, - RaftURL: raftUrl, - EtcdURL: etcdUrl, + Name: name, + RaftURL: raftUrl, + EtcdURL: etcdUrl, } } @@ -54,11 +54,11 @@ func (c *JoinCommand) Apply(server raft.Server) (interface{}, error) { // Check machine number in the cluster if ps.registry.Count() == ps.MaxClusterSize { log.Debug("Reject join request from ", c.Name) - return []byte{0}, etcdErr.NewError(etcdErr.EcodeNoMoreMachine, "", server.CommitIndex(), server.Term()) + return []byte{0}, etcdErr.NewError(etcdErr.EcodeNoMoreMachine, "", server.CommitIndex()) } // Add to shared machine registry. - ps.registry.Register(c.Name, c.RaftURL, c.EtcdURL, server.CommitIndex(), server.Term()) + ps.registry.Register(c.Name, c.RaftURL, c.EtcdURL) // Add peer in raft err := server.AddPeer(c.Name, "") diff --git a/server/registry.go b/server/registry.go index 05cccc10c..b7d238099 100644 --- a/server/registry.go +++ b/server/registry.go @@ -38,20 +38,20 @@ func NewRegistry(s store.Store) *Registry { } // Adds a node to the registry. -func (r *Registry) Register(name string, peerURL string, url string, commitIndex uint64, term uint64) error { +func (r *Registry) Register(name string, peerURL string, url string) error { r.Lock() defer r.Unlock() // Write data to store. key := path.Join(RegistryKey, name) value := fmt.Sprintf("raft=%s&etcd=%s", peerURL, url) - _, err := r.store.Create(key, value, false, store.Permanent, commitIndex, term) + _, err := r.store.Create(key, value, false, store.Permanent) log.Debugf("Register: %s", name) return err } // Removes a node from the registry. -func (r *Registry) Unregister(name string, commitIndex uint64, term uint64) error { +func (r *Registry) Unregister(name string) error { r.Lock() defer r.Unlock() @@ -59,14 +59,14 @@ func (r *Registry) Unregister(name string, commitIndex uint64, term uint64) erro // delete(r.nodes, name) // Remove the key from the store. - _, err := r.store.Delete(path.Join(RegistryKey, name), false, commitIndex, term) + _, err := r.store.Delete(path.Join(RegistryKey, name), false) log.Debugf("Unregister: %s", name) return err } // Returns the number of nodes in the cluster. func (r *Registry) Count() int { - e, err := r.store.Get(RegistryKey, false, false, 0, 0) + e, err := r.store.Get(RegistryKey, false, false) if err != nil { return 0 } @@ -133,7 +133,7 @@ func (r *Registry) urls(leaderName, selfName string, url func(name string) (stri } // Retrieve a list of all nodes. - if e, _ := r.store.Get(RegistryKey, false, false, 0, 0); e != nil { + if e, _ := r.store.Get(RegistryKey, false, false); e != nil { // Lookup the URL for each one. for _, pair := range e.KVPairs { _, name := filepath.Split(pair.Key) @@ -160,7 +160,7 @@ func (r *Registry) load(name string) { } // Retrieve from store. - e, err := r.store.Get(path.Join(RegistryKey, name), false, false, 0, 0) + e, err := r.store.Get(path.Join(RegistryKey, name), false, false) if err != nil { return } @@ -173,7 +173,7 @@ func (r *Registry) load(name string) { // Create node. r.nodes[name] = &node{ - url: m["etcd"][0], - peerURL: m["raft"][0], + url: m["etcd"][0], + peerURL: m["raft"][0], } } diff --git a/server/remove_command.go b/server/remove_command.go index 42e2c5078..bb1e2bb96 100644 --- a/server/remove_command.go +++ b/server/remove_command.go @@ -27,7 +27,7 @@ func (c *RemoveCommand) Apply(server raft.Server) (interface{}, error) { ps, _ := server.Context().(*PeerServer) // Remove node from the shared registry. - err := ps.registry.Unregister(c.Name, server.CommitIndex(), server.Term()) + err := ps.registry.Unregister(c.Name) // Delete from stats delete(ps.followersStats.Followers, c.Name) diff --git a/server/server.go b/server/server.go index 48757ad2b..e2b54f8b5 100644 --- a/server/server.go +++ b/server/server.go @@ -234,7 +234,7 @@ func (s *Server) Dispatch(c raft.Command, w http.ResponseWriter, req *http.Reque } if result == nil { - return etcdErr.NewError(300, "Empty result from raft", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(300, "Empty result from raft", s.Store().Index()) } // response for raft related commands[join/remove] @@ -268,7 +268,7 @@ func (s *Server) Dispatch(c raft.Command, w http.ResponseWriter, req *http.Reque // No leader available. if leader == "" { - return etcdErr.NewError(300, "", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(300, "", s.Store().Index()) } var url string @@ -317,7 +317,7 @@ func (s *Server) GetVersionHandler(w http.ResponseWriter, req *http.Request) err func (s *Server) GetLeaderHandler(w http.ResponseWriter, req *http.Request) error { leader := s.peerServer.RaftServer().Leader() if leader == "" { - return etcdErr.NewError(etcdErr.EcodeLeaderElect, "", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeLeaderElect, "", s.Store().Index()) } w.WriteHeader(http.StatusOK) url, _ := s.registry.PeerURL(leader) @@ -348,7 +348,7 @@ func (s *Server) GetLeaderStatsHandler(w http.ResponseWriter, req *http.Request) leader := s.peerServer.RaftServer().Leader() if leader == "" { - return etcdErr.NewError(300, "", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(300, "", s.Store().Index()) } hostname, _ := s.registry.ClientURL(leader) redirect(hostname, w, req) diff --git a/server/v1/get_key_handler.go b/server/v1/get_key_handler.go index 53558e142..880bf289e 100644 --- a/server/v1/get_key_handler.go +++ b/server/v1/get_key_handler.go @@ -13,7 +13,7 @@ func GetKeyHandler(w http.ResponseWriter, req *http.Request, s Server) error { key := "/" + vars["key"] // Retrieve the key from the store. - event, err := s.Store().Get(key, false, false, s.CommitIndex(), s.Term()) + event, err := s.Store().Get(key, false, false) if err != nil { return err } diff --git a/server/v1/set_key_handler.go b/server/v1/set_key_handler.go index 7acfe7ecb..b1b4390a2 100644 --- a/server/v1/set_key_handler.go +++ b/server/v1/set_key_handler.go @@ -19,13 +19,13 @@ func SetKeyHandler(w http.ResponseWriter, req *http.Request, s Server) error { // Parse non-blank value. value := req.Form.Get("value") if len(value) == 0 { - return etcdErr.NewError(200, "Set", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(200, "Set", s.Store().Index()) } // Convert time-to-live to an expiration time. expireTime, err := store.TTL(req.Form.Get("ttl")) if err != nil { - return etcdErr.NewError(202, "Set", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(202, "Set", s.Store().Index()) } // If the "prevValue" is specified then test-and-set. Otherwise create a new key. diff --git a/server/v1/watch_key_handler.go b/server/v1/watch_key_handler.go index e8db56c30..de5ed0656 100644 --- a/server/v1/watch_key_handler.go +++ b/server/v1/watch_key_handler.go @@ -6,7 +6,6 @@ import ( "strconv" etcdErr "github.com/coreos/etcd/error" - "github.com/coreos/etcd/store" "github.com/gorilla/mux" ) @@ -21,14 +20,14 @@ func WatchKeyHandler(w http.ResponseWriter, req *http.Request, s Server) error { if req.Method == "POST" { sinceIndex, err = strconv.ParseUint(string(req.FormValue("index")), 10, 64) if err != nil { - return etcdErr.NewError(203, "Watch From Index", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(203, "Watch From Index", s.Store().Index()) } } // Start the watcher on the store. - c, err := s.Store().Watch(key, false, sinceIndex, s.CommitIndex(), s.Term()) + c, err := s.Store().Watch(key, false, sinceIndex) if err != nil { - return etcdErr.NewError(500, key, store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(500, key, s.Store().Index()) } event := <-c diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index dba491cbd..f2d05c507 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -16,7 +16,6 @@ import ( func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { var err error var event *store.Event - events := make([]*store.Event, 0) vars := mux.Vars(req) key := "/" + vars["key"] @@ -42,51 +41,36 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { if waitIndex != "" { sinceIndex, err = strconv.ParseUint(string(req.FormValue("waitIndex")), 10, 64) if err != nil { - return etcdErr.NewError(etcdErr.EcodeIndexNaN, "Watch From Index", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeIndexNaN, "Watch From Index", s.Store().Index()) } } // Start the watcher on the store. - eventChan, err := s.Store().Watch(key, recursive, sinceIndex, s.CommitIndex(), s.Term()) + eventChan, err := s.Store().Watch(key, recursive, sinceIndex) if err != nil { - return etcdErr.NewError(500, key, store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(500, key, s.Store().Index()) } cn, _ := w.(http.CloseNotifier) closeChan := cn.CloseNotify() - eventLoop: - for { - select { - case <-closeChan: - return nil - case event = <-eventChan: - // for events other than expire, just one event for one watch - // for expire event, we might have a stream of events - // we use a nil item to terminate the expire event stream - if event != nil && event.Action == store.Expire { - events = append(events, event) - } else { - events = append(events, event) - break eventLoop - } - } + select { + case <-closeChan: + return nil + case event = <-eventChan: } } else { //get // Retrieve the key from the store. - event, err = s.Store().Get(key, recursive, sorted, s.CommitIndex(), s.Term()) + event, err = s.Store().Get(key, recursive, sorted) if err != nil { return err } } - var b []byte - - w.Header().Add("X-Etcd-Index", fmt.Sprint(events[0].Index)) - w.Header().Add("X-Etcd-Term", fmt.Sprint(events[0].Term)) + w.Header().Add("X-Etcd-Index", fmt.Sprint(event.Index)) w.WriteHeader(http.StatusOK) - b, _ = json.Marshal(events) + b, _ := json.Marshal(event) w.Write(b) diff --git a/server/v2/post_handler.go b/server/v2/post_handler.go index 4dc98b925..2338d0250 100644 --- a/server/v2/post_handler.go +++ b/server/v2/post_handler.go @@ -15,7 +15,7 @@ func PostHandler(w http.ResponseWriter, req *http.Request, s Server) error { value := req.FormValue("value") expireTime, err := store.TTL(req.FormValue("ttl")) if err != nil { - return etcdErr.NewError(etcdErr.EcodeTTLNaN, "Create", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeTTLNaN, "Create", s.Store().Index()) } c := s.Store().CommandFactory().CreateCreateCommand(key, value, expireTime, true) diff --git a/server/v2/put_handler.go b/server/v2/put_handler.go index 3afb018b7..5b61e58b3 100644 --- a/server/v2/put_handler.go +++ b/server/v2/put_handler.go @@ -22,7 +22,7 @@ func PutHandler(w http.ResponseWriter, req *http.Request, s Server) error { value := req.Form.Get("value") expireTime, err := store.TTL(req.Form.Get("ttl")) if err != nil { - return etcdErr.NewError(etcdErr.EcodeTTLNaN, "Update", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeTTLNaN, "Update", s.Store().Index()) } _, valueOk := req.Form["prevValue"] @@ -59,7 +59,7 @@ func PutHandler(w http.ResponseWriter, req *http.Request, s Server) error { // bad previous index if err != nil { - return etcdErr.NewError(etcdErr.EcodeIndexNaN, "CompareAndSwap", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeIndexNaN, "CompareAndSwap", s.Store().Index()) } } else { prevIndex = 0 @@ -67,7 +67,7 @@ func PutHandler(w http.ResponseWriter, req *http.Request, s Server) error { if valueOk { if prevValue == "" { - return etcdErr.NewError(etcdErr.EcodePrevValueRequired, "CompareAndSwap", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodePrevValueRequired, "CompareAndSwap", s.Store().Index()) } } @@ -88,7 +88,7 @@ func CreateHandler(w http.ResponseWriter, req *http.Request, s Server, key, valu func UpdateHandler(w http.ResponseWriter, req *http.Request, s Server, key, value string, expireTime time.Time) error { // Update should give at least one option if value == "" && expireTime.Sub(store.Permanent) == 0 { - return etcdErr.NewError(etcdErr.EcodeValueOrTTLRequired, "Update", store.UndefIndex, store.UndefTerm) + return etcdErr.NewError(etcdErr.EcodeValueOrTTLRequired, "Update", s.Store().Index()) } c := s.Store().CommandFactory().CreateUpdateCommand(key, value, expireTime) diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index ab710c5ac..2905c68c4 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -24,6 +24,6 @@ func TestV2DeleteKey(t *testing.T) { resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), url.Values{}) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","index":4,"term":0}`, "") + assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","index":2}`, "") }) } diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index 3b365872c..0f0054055 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -27,8 +27,7 @@ func TestV2GetKey(t *testing.T) { assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["index"], 3, "") - assert.Equal(t, body["term"], 0, "") + assert.Equal(t, body["index"], 1, "") }) } @@ -55,7 +54,7 @@ func TestV2GetKeyRecursively(t *testing.T) { assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["index"], 4, "") + assert.Equal(t, body["index"], 2, "") assert.Equal(t, len(body["kvs"].([]interface{})), 2, "") kv0 := body["kvs"].([]interface{})[0].(map[string]interface{}) @@ -81,9 +80,11 @@ func TestV2GetKeyRecursively(t *testing.T) { func TestV2WatchKey(t *testing.T) { tests.RunServer(func(s *server.Server) { var body map[string]interface{} + c := make(chan bool) go func() { resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true")) body = tests.ReadBodyJSON(resp) + c <- true }() // Make sure response didn't fire early. @@ -98,12 +99,19 @@ func TestV2WatchKey(t *testing.T) { // A response should follow from the GET above. time.Sleep(1 * time.Millisecond) + + select { + case <-c: + + default: + t.Fatal("cannot get watch result") + } + assert.NotNil(t, body, "") assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["index"], 3, "") - assert.Equal(t, body["term"], 0, "") + assert.Equal(t, body["index"], 1, "") }) } @@ -118,7 +126,7 @@ func TestV2WatchKeyWithIndex(t *testing.T) { var body map[string]interface{} c := make(chan bool) go func() { - resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=4")) + resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=2")) body = tests.ReadBodyJSON(resp) c <- true }() @@ -156,7 +164,6 @@ func TestV2WatchKeyWithIndex(t *testing.T) { assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 4, "") - assert.Equal(t, body["term"], 0, "") + assert.Equal(t, body["index"], 2, "") }) } diff --git a/server/v2/tests/post_handler_test.go b/server/v2/tests/post_handler_test.go index 8e7e23a8c..655278f22 100644 --- a/server/v2/tests/post_handler_test.go +++ b/server/v2/tests/post_handler_test.go @@ -21,18 +21,18 @@ func TestV2CreateUnique(t *testing.T) { resp, _ := tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), nil) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "create", "") - assert.Equal(t, body["key"], "/foo/bar/3", "") + assert.Equal(t, body["key"], "/foo/bar/1", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["index"], 3, "") + assert.Equal(t, body["index"], 1, "") // Second POST should add next index to list. resp, _ = tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), nil) body = tests.ReadBodyJSON(resp) - assert.Equal(t, body["key"], "/foo/bar/4", "") + assert.Equal(t, body["key"], "/foo/bar/2", "") // POST to a different key should add index to that list. resp, _ = tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/baz"), nil) body = tests.ReadBodyJSON(resp) - assert.Equal(t, body["key"], "/foo/baz/5", "") + assert.Equal(t, body["key"], "/foo/baz/3", "") }) } diff --git a/server/v2/tests/put_handler_test.go b/server/v2/tests/put_handler_test.go index 1341d03b3..73f6be91c 100644 --- a/server/v2/tests/put_handler_test.go +++ b/server/v2/tests/put_handler_test.go @@ -22,7 +22,7 @@ func TestV2SetKey(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","index":3,"term":0}`, "") + assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","index":1}`, "") }) } @@ -42,7 +42,7 @@ func TestV2SetKeyWithTTL(t *testing.T) { // Make sure the expiration date is correct. expiration, _ := time.Parse(time.RFC3339Nano, body["expiration"].(string)) - assert.Equal(t, expiration.Sub(t0) / time.Second, 20, "") + assert.Equal(t, expiration.Sub(t0)/time.Second, 20, "") }) } @@ -110,7 +110,7 @@ func TestV2UpdateKeySuccess(t *testing.T) { v.Set("value", "XXX") resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - + v.Set("value", "YYY") v.Set("prevExist", "true") resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) @@ -160,7 +160,7 @@ func TestV2UpdateKeyFailOnMissingDirectory(t *testing.T) { // Ensures that a key is set only if the previous index matches. // // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=YYY -d prevIndex=3 +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=YYY -d prevIndex=1 // func TestV2SetKeyCASOnIndexSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { @@ -169,13 +169,13 @@ func TestV2SetKeyCASOnIndexSuccess(t *testing.T) { resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") - v.Set("prevIndex", "3") + v.Set("prevIndex", "1") resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 4, "") + assert.Equal(t, body["index"], 2, "") }) } @@ -196,8 +196,8 @@ func TestV2SetKeyCASOnIndexFail(t *testing.T) { body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Test Failed", "") - assert.Equal(t, body["cause"], "[ != XXX] [10 != 3]", "") - assert.Equal(t, body["index"], 4, "") + assert.Equal(t, body["cause"], "[ != XXX] [10 != 1]", "") + assert.Equal(t, body["index"], 1, "") }) } @@ -236,7 +236,7 @@ func TestV2SetKeyCASOnValueSuccess(t *testing.T) { assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 4, "") + assert.Equal(t, body["index"], 2, "") }) } @@ -257,8 +257,8 @@ func TestV2SetKeyCASOnValueFail(t *testing.T) { body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Test Failed", "") - assert.Equal(t, body["cause"], "[AAA != XXX] [0 != 3]", "") - assert.Equal(t, body["index"], 4, "") + assert.Equal(t, body["cause"], "[AAA != XXX] [0 != 1]", "") + assert.Equal(t, body["index"], 1, "") }) } diff --git a/store/event.go b/store/event.go index ef007c642..50d2872da 100644 --- a/store/event.go +++ b/store/event.go @@ -14,11 +14,6 @@ const ( Expire = "expire" ) -const ( - UndefIndex = 0 - UndefTerm = 0 -) - type Event struct { Action string `json:"action"` Key string `json:"key, omitempty"` @@ -28,17 +23,15 @@ type Event struct { KVPairs kvPairs `json:"kvs,omitempty"` Expiration *time.Time `json:"expiration,omitempty"` TTL int64 `json:"ttl,omitempty"` // Time to live in second - // The command index of the raft machine when the command is executed + // The index of the etcd state machine when the comment is executed Index uint64 `json:"index"` - Term uint64 `json:"term"` } -func newEvent(action string, key string, index uint64, term uint64) *Event { +func newEvent(action string, key string, index uint64) *Event { return &Event{ Action: action, Key: key, Index: index, - Term: term, } } diff --git a/store/event_history.go b/store/event_history.go index 4d11a8883..aaa93d44e 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -12,8 +12,6 @@ type EventHistory struct { Queue eventQueue StartIndex uint64 LastIndex uint64 - LastTerm uint64 - DupCnt uint64 // help to compute the watching point with duplicated indexes in the queue rwl sync.RWMutex } @@ -31,14 +29,9 @@ func (eh *EventHistory) addEvent(e *Event) *Event { eh.rwl.Lock() defer eh.rwl.Unlock() - if e.Index == eh.LastIndex { - eh.DupCnt += 1 - } - eh.Queue.insert(e) eh.LastIndex = e.Index - eh.LastTerm = e.Term eh.StartIndex = eh.Queue.Events[eh.Queue.Front].Index @@ -47,7 +40,7 @@ func (eh *EventHistory) addEvent(e *Event) *Event { // scan function is enumerating events from the index in history and // stops till the first point where the key has identified prefix -func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Error) { +func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Error) { eh.rwl.RLock() defer eh.rwl.RUnlock() @@ -56,7 +49,7 @@ func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Er return nil, etcdErr.NewError(etcdErr.EcodeEventIndexCleared, fmt.Sprintf("the requested history has been cleared [%v/%v]", - eh.StartIndex, index), UndefIndex, UndefTerm) + eh.StartIndex, index), 0) } // the index should locate before the size of the queue minus the duplicate count @@ -66,28 +59,17 @@ func (eh *EventHistory) scan(prefix string, index uint64) ([]*Event, *etcdErr.Er i := eh.Queue.Front - events := make([]*Event, 0) - var eventIndex uint64 - for { e := eh.Queue.Events[i] - if eventIndex != 0 && eventIndex != e.Index { - return events, nil - } - if strings.HasPrefix(e.Key, prefix) && index <= e.Index { // make sure we bypass the smaller one - eventIndex = e.Index - events = append(events, e) + return e, nil } i = (i + 1) % eh.Queue.Capacity if i > eh.Queue.back() { - if eventIndex == 0 { // find nothing, return and watch from current index - return nil, nil - } - return events, nil + return nil, nil } } } @@ -110,8 +92,6 @@ func (eh *EventHistory) clone() *EventHistory { StartIndex: eh.StartIndex, Queue: clonedQueue, LastIndex: eh.LastIndex, - LastTerm: eh.LastTerm, - DupCnt: eh.DupCnt, } } diff --git a/store/event_test.go b/store/event_test.go index aedf7f7da..8a70d09a2 100644 --- a/store/event_test.go +++ b/store/event_test.go @@ -13,7 +13,7 @@ func TestEventQueue(t *testing.T) { // Add for i := 0; i < 200; i++ { - e := newEvent(Create, "/foo", uint64(i), 1) + e := newEvent(Create, "/foo", uint64(i)) eh.addEvent(e) } @@ -35,27 +35,27 @@ func TestScanHistory(t *testing.T) { eh := newEventHistory(100) // Add - eh.addEvent(newEvent(Create, "/foo", 1, 1)) - eh.addEvent(newEvent(Create, "/foo/bar", 2, 1)) - eh.addEvent(newEvent(Create, "/foo/foo", 3, 1)) - eh.addEvent(newEvent(Create, "/foo/bar/bar", 4, 1)) - eh.addEvent(newEvent(Create, "/foo/foo/foo", 5, 1)) + eh.addEvent(newEvent(Create, "/foo", 1)) + eh.addEvent(newEvent(Create, "/foo/bar", 2)) + eh.addEvent(newEvent(Create, "/foo/foo", 3)) + eh.addEvent(newEvent(Create, "/foo/bar/bar", 4)) + eh.addEvent(newEvent(Create, "/foo/foo/foo", 5)) e, err := eh.scan("/foo", 1) - if err != nil || e[0].Index != 1 { - t.Fatalf("scan error [/foo] [1] %v", e[0].Index) + if err != nil || e.Index != 1 { + t.Fatalf("scan error [/foo] [1] %v", e.Index) } e, err = eh.scan("/foo/bar", 1) - if err != nil || e[0].Index != 2 { - t.Fatalf("scan error [/foo/bar] [2] %v", e[0].Index) + if err != nil || e.Index != 2 { + t.Fatalf("scan error [/foo/bar] [2] %v", e.Index) } e, err = eh.scan("/foo/bar", 3) - if err != nil || e[0].Index != 4 { - t.Fatalf("scan error [/foo/bar/bar] [4] %v", e[0].Index) + if err != nil || e.Index != 4 { + t.Fatalf("scan error [/foo/bar/bar] [4] %v", e.Index) } e, err = eh.scan("/foo/bar", 6) diff --git a/store/heap_test.go b/store/heap_test.go index aa0b9caf6..9175feef9 100644 --- a/store/heap_test.go +++ b/store/heap_test.go @@ -14,7 +14,7 @@ func TestHeapPushPop(t *testing.T) { for i := 0; i < 10; i++ { path := fmt.Sprintf("%v", 10-i) m := time.Duration(10 - i) - n := newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) + n := newKV(nil, path, path, 0, nil, "", time.Now().Add(time.Second*m)) h.push(n) } @@ -40,7 +40,7 @@ func TestHeapUpdate(t *testing.T) { for i, n := range kvs { path := fmt.Sprintf("%v", 10-i) m := time.Duration(10 - i) - n = newKV(nil, path, path, 0, 0, nil, "", time.Now().Add(time.Second*m)) + n = newKV(nil, path, path, 0, nil, "", time.Now().Add(time.Second*m)) kvs[i] = n h.push(n) } diff --git a/store/node.go b/store/node.go index 6b11727c8..a0916b81a 100644 --- a/store/node.go +++ b/store/node.go @@ -8,11 +8,6 @@ import ( etcdErr "github.com/coreos/etcd/error" ) -const ( - normal = iota - removed -) - var Permanent time.Time // Node is the basic element in the store system. @@ -26,7 +21,7 @@ type Node struct { ModifiedIndex uint64 ModifiedTerm uint64 - Parent *Node `json:"-"` // should not encode this field! avoid cyclical dependency. + Parent *Node `json:"-"` // should not encode this field! avoid circular dependency. ExpireTime time.Time ACL string @@ -39,14 +34,12 @@ type Node struct { // newKV creates a Key-Value pair func newKV(store *store, nodePath string, value string, createIndex uint64, - createTerm uint64, parent *Node, ACL string, expireTime time.Time) *Node { + parent *Node, ACL string, expireTime time.Time) *Node { return &Node{ Path: nodePath, CreateIndex: createIndex, - CreateTerm: createTerm, ModifiedIndex: createIndex, - ModifiedTerm: createTerm, Parent: parent, ACL: ACL, store: store, @@ -56,13 +49,12 @@ func newKV(store *store, nodePath string, value string, createIndex uint64, } // newDir creates a directory -func newDir(store *store, nodePath string, createIndex uint64, createTerm uint64, - parent *Node, ACL string, expireTime time.Time) *Node { +func newDir(store *store, nodePath string, createIndex uint64, parent *Node, + ACL string, expireTime time.Time) *Node { return &Node{ Path: nodePath, CreateIndex: createIndex, - CreateTerm: createTerm, Parent: parent, ACL: ACL, ExpireTime: expireTime, @@ -101,7 +93,7 @@ func (n *Node) IsDir() bool { // If the receiver node is not a key-value pair, a "Not A File" error will be returned. func (n *Node) Read() (string, *etcdErr.Error) { if n.IsDir() { - return "", etcdErr.NewError(etcdErr.EcodeNotFile, "", UndefIndex, UndefTerm) + return "", etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index()) } return n.Value, nil @@ -109,14 +101,13 @@ func (n *Node) Read() (string, *etcdErr.Error) { // Write function set the value of the node to the given value. // If the receiver node is a directory, a "Not A File" error will be returned. -func (n *Node) Write(value string, index uint64, term uint64) *etcdErr.Error { +func (n *Node) Write(value string, index uint64) *etcdErr.Error { if n.IsDir() { - return etcdErr.NewError(etcdErr.EcodeNotFile, "", UndefIndex, UndefTerm) + return etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index()) } n.Value = value n.ModifiedIndex = index - n.ModifiedTerm = term return nil } @@ -132,7 +123,7 @@ func (n *Node) ExpirationAndTTL() (*time.Time, int64) { // If the receiver node is not a directory, a "Not A Directory" error will be returned. func (n *Node) List() ([]*Node, *etcdErr.Error) { if !n.IsDir() { - return nil, etcdErr.NewError(etcdErr.EcodeNotDir, "", UndefIndex, UndefTerm) + return nil, etcdErr.NewError(etcdErr.EcodeNotDir, "", n.store.Index()) } nodes := make([]*Node, len(n.Children)) @@ -150,7 +141,7 @@ func (n *Node) List() ([]*Node, *etcdErr.Error) { // On success, it returns the file node func (n *Node) GetChild(name string) (*Node, *etcdErr.Error) { if !n.IsDir() { - return nil, etcdErr.NewError(etcdErr.EcodeNotDir, n.Path, UndefIndex, UndefTerm) + return nil, etcdErr.NewError(etcdErr.EcodeNotDir, n.Path, n.store.Index()) } child, ok := n.Children[name] @@ -168,7 +159,7 @@ func (n *Node) GetChild(name string) (*Node, *etcdErr.Error) { // error will be returned func (n *Node) Add(child *Node) *etcdErr.Error { if !n.IsDir() { - return etcdErr.NewError(etcdErr.EcodeNotDir, "", UndefIndex, UndefTerm) + return etcdErr.NewError(etcdErr.EcodeNotDir, "", n.store.Index()) } _, name := path.Split(child.Path) @@ -176,7 +167,7 @@ func (n *Node) Add(child *Node) *etcdErr.Error { _, ok := n.Children[name] if ok { - return etcdErr.NewError(etcdErr.EcodeNodeExist, "", UndefIndex, UndefTerm) + return etcdErr.NewError(etcdErr.EcodeNodeExist, "", n.store.Index()) } n.Children[name] = child @@ -189,7 +180,7 @@ func (n *Node) Remove(recursive bool, callback func(path string)) *etcdErr.Error if n.IsDir() && !recursive { // cannot delete a directory without set recursive to true - return etcdErr.NewError(etcdErr.EcodeNotFile, "", UndefIndex, UndefTerm) + return etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index()) } if !n.IsDir() { // key-value pair @@ -309,10 +300,10 @@ func (n *Node) UpdateTTL(expireTime time.Time) { // If the node is a key-value pair, it will clone the pair. func (n *Node) Clone() *Node { if !n.IsDir() { - return newKV(n.store, n.Path, n.Value, n.CreateIndex, n.CreateTerm, n.Parent, n.ACL, n.ExpireTime) + return newKV(n.store, n.Path, n.Value, n.CreateIndex, n.Parent, n.ACL, n.ExpireTime) } - clone := newDir(n.store, n.Path, n.CreateIndex, n.CreateTerm, n.Parent, n.ACL, n.ExpireTime) + clone := newDir(n.store, n.Path, n.CreateIndex, n.Parent, n.ACL, n.ExpireTime) for key, child := range n.Children { clone.Children[key] = child.Clone() diff --git a/store/stats_test.go b/store/stats_test.go index 9b3172ad8..ad16261b8 100644 --- a/store/stats_test.go +++ b/store/stats_test.go @@ -10,85 +10,85 @@ import ( // Ensure that a successful Get is recorded in the stats. func TestStoreStatsGetSuccess(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.Get("/foo", false, false, 3, 1) + s.Create("/foo", "bar", false, Permanent) + s.Get("/foo", false, false) assert.Equal(t, uint64(1), s.Stats.GetSuccess, "") } // Ensure that a failed Get is recorded in the stats. func TestStoreStatsGetFail(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.Get("/no_such_key", false, false, 3, 1) + s.Create("/foo", "bar", false, Permanent) + s.Get("/no_such_key", false, false) assert.Equal(t, uint64(1), s.Stats.GetFail, "") } // Ensure that a successful Create is recorded in the stats. func TestStoreStatsCreateSuccess(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) assert.Equal(t, uint64(1), s.Stats.CreateSuccess, "") } // Ensure that a failed Create is recorded in the stats. func TestStoreStatsCreateFail(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 3, 1) - s.Create("/foo", "bar", false, Permanent, 4, 1) + s.Create("/foo", "", false, Permanent) + s.Create("/foo", "bar", false, Permanent) assert.Equal(t, uint64(1), s.Stats.CreateFail, "") } // Ensure that a successful Update is recorded in the stats. func TestStoreStatsUpdateSuccess(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.Update("/foo", "baz", Permanent, 4, 1) + s.Create("/foo", "bar", false, Permanent) + s.Update("/foo", "baz", Permanent) assert.Equal(t, uint64(1), s.Stats.UpdateSuccess, "") } // Ensure that a failed Update is recorded in the stats. func TestStoreStatsUpdateFail(t *testing.T) { s := newStore() - s.Update("/foo", "bar", Permanent, 4, 1) + s.Update("/foo", "bar", Permanent) assert.Equal(t, uint64(1), s.Stats.UpdateFail, "") } // Ensure that a successful CAS is recorded in the stats. func TestStoreStatsCompareAndSwapSuccess(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent, 4, 1) + s.Create("/foo", "bar", false, Permanent) + s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent) assert.Equal(t, uint64(1), s.Stats.CompareAndSwapSuccess, "") } // Ensure that a failed CAS is recorded in the stats. func TestStoreStatsCompareAndSwapFail(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.CompareAndSwap("/foo", "wrong_value", 0, "baz", Permanent, 4, 1) + s.Create("/foo", "bar", false, Permanent) + s.CompareAndSwap("/foo", "wrong_value", 0, "baz", Permanent) assert.Equal(t, uint64(1), s.Stats.CompareAndSwapFail, "") } // Ensure that a successful Delete is recorded in the stats. func TestStoreStatsDeleteSuccess(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 3, 1) - s.Delete("/foo", false, 4, 1) + s.Create("/foo", "bar", false, Permanent) + s.Delete("/foo", false) assert.Equal(t, uint64(1), s.Stats.DeleteSuccess, "") } // Ensure that a failed Delete is recorded in the stats. func TestStoreStatsDeleteFail(t *testing.T) { s := newStore() - s.Delete("/foo", false, 4, 1) + s.Delete("/foo", false) assert.Equal(t, uint64(1), s.Stats.DeleteFail, "") } //Ensure that the number of expirations is recorded in the stats. func TestStoreStatsExpireCount(t *testing.T) { s := newStore() - go mockSyncService(s.deleteExpiredKeys) - s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond), 3, 1) + go mockSyncService(s.DeleteExpiredKeys) + s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond)) assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") time.Sleep(600 * time.Millisecond) assert.Equal(t, uint64(1), s.Stats.ExpireCount, "") diff --git a/store/store.go b/store/store.go index 5e374f1ff..f9ebab7ad 100644 --- a/store/store.go +++ b/store/store.go @@ -19,27 +19,27 @@ const defaultVersion = 2 type Store interface { Version() int CommandFactory() CommandFactory - Get(nodePath string, recursive, sorted bool, index uint64, term uint64) (*Event, error) - Set(nodePath string, value string, expireTime time.Time, index uint64, term uint64) (*Event, error) - Update(nodePath string, newValue string, expireTime time.Time, index uint64, term uint64) (*Event, error) - Create(nodePath string, value string, incrementalSuffix bool, expireTime time.Time, - index uint64, term uint64) (*Event, error) + Index() uint64 + Get(nodePath string, recursive, sorted bool) (*Event, error) + Set(nodePath string, value string, expireTime time.Time) (*Event, error) + Update(nodePath string, newValue string, expireTime time.Time) (*Event, error) + Create(nodePath string, value string, incrementalSuffix bool, + expireTime time.Time) (*Event, error) CompareAndSwap(nodePath string, prevValue string, prevIndex uint64, - value string, expireTime time.Time, index uint64, term uint64) (*Event, error) - Delete(nodePath string, recursive bool, index uint64, term uint64) (*Event, error) - Watch(prefix string, recursive bool, sinceIndex uint64, index uint64, term uint64) (<-chan *Event, error) + value string, expireTime time.Time) (*Event, error) + Delete(nodePath string, recursive bool) (*Event, error) + Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) Save() ([]byte, error) Recovery(state []byte) error TotalTransactions() uint64 JsonStats() []byte - DeleteExpiredKeys(cutoff time.Time, index uint64, term uint64) + DeleteExpiredKeys(cutoff time.Time) } type store struct { Root *Node WatcherHub *watcherHub - Index uint64 - Term uint64 + CurrentIndex uint64 Stats *Stats CurrentVersion int ttlKeyHeap *ttlKeyHeap // need to recovery manually @@ -53,7 +53,7 @@ func New() Store { func newStore() *store { s := new(store) s.CurrentVersion = defaultVersion - s.Root = newDir(s, "/", UndefIndex, UndefTerm, nil, "", Permanent) + s.Root = newDir(s, "/", s.CurrentIndex, nil, "", Permanent) s.Stats = newStats() s.WatcherHub = newWatchHub(1000) s.ttlKeyHeap = newTtlKeyHeap() @@ -65,6 +65,11 @@ func (s *store) Version() int { return s.CurrentVersion } +// Retrieves current of the store +func (s *store) Index() uint64 { + return s.CurrentIndex +} + // CommandFactory retrieves the command factory for the current version of the store. func (s *store) CommandFactory() CommandFactory { return GetCommandFactory(s.Version()) @@ -73,20 +78,20 @@ func (s *store) CommandFactory() CommandFactory { // Get function returns a get event. // If recursive is true, it will return all the content under the node path. // If sorted is true, it will sort the content by keys. -func (s *store) Get(nodePath string, recursive, sorted bool, index uint64, term uint64) (*Event, error) { +func (s *store) Get(nodePath string, recursive, sorted bool) (*Event, error) { s.worldLock.RLock() defer s.worldLock.RUnlock() nodePath = path.Clean(path.Join("/", nodePath)) - n, err := s.internalGet(nodePath, index, term) + n, err := s.internalGet(nodePath) if err != nil { s.Stats.Inc(GetFail) return nil, err } - e := newEvent(Get, nodePath, index, term) + e := newEvent(Get, nodePath, s.CurrentIndex) if n.IsDir() { // node is a directory e.Dir = true @@ -128,13 +133,12 @@ func (s *store) Get(nodePath string, recursive, sorted bool, index uint64, term // Create function creates the Node at nodePath. Create will help to create intermediate directories with no ttl. // If the node has already existed, create will fail. // If any node on the path is a file, create will fail. -func (s *store) Create(nodePath string, value string, unique bool, - expireTime time.Time, index uint64, term uint64) (*Event, error) { +func (s *store) Create(nodePath string, value string, unique bool, expireTime time.Time) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() defer s.worldLock.Unlock() - e, err := s.internalCreate(nodePath, value, unique, false, expireTime, index, term, Create) + e, err := s.internalCreate(nodePath, value, unique, false, expireTime, Create) if err == nil { s.Stats.Inc(CreateSuccess) @@ -146,12 +150,12 @@ func (s *store) Create(nodePath string, value string, unique bool, } // Set function creates or replace the Node at nodePath. -func (s *store) Set(nodePath string, value string, expireTime time.Time, index uint64, term uint64) (*Event, error) { +func (s *store) Set(nodePath string, value string, expireTime time.Time) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() defer s.worldLock.Unlock() - e, err := s.internalCreate(nodePath, value, false, true, expireTime, index, term, Set) + e, err := s.internalCreate(nodePath, value, false, true, expireTime, Set) if err == nil { s.Stats.Inc(SetSuccess) @@ -163,14 +167,14 @@ func (s *store) Set(nodePath string, value string, expireTime time.Time, index u } func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint64, - value string, expireTime time.Time, index uint64, term uint64) (*Event, error) { + value string, expireTime time.Time) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() defer s.worldLock.Unlock() - n, err := s.internalGet(nodePath, index, term) + n, err := s.internalGet(nodePath) if err != nil { s.Stats.Inc(CompareAndSwapFail) @@ -179,17 +183,20 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint if n.IsDir() { // can only test and set file s.Stats.Inc(CompareAndSwapFail) - return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, index, term) + return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, s.CurrentIndex) } // If both of the prevValue and prevIndex are given, we will test both of them. // Command will be executed, only if both of the tests are successful. if (prevValue == "" || n.Value == prevValue) && (prevIndex == 0 || n.ModifiedIndex == prevIndex) { - e := newEvent(CompareAndSwap, nodePath, index, term) + // update etcd index + s.CurrentIndex++ + + e := newEvent(CompareAndSwap, nodePath, s.CurrentIndex) e.PrevValue = n.Value // if test succeed, write the value - n.Write(value, index, term) + n.Write(value, s.CurrentIndex) n.UpdateTTL(expireTime) e.Value = value @@ -202,25 +209,25 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint cause := fmt.Sprintf("[%v != %v] [%v != %v]", prevValue, n.Value, prevIndex, n.ModifiedIndex) s.Stats.Inc(CompareAndSwapFail) - return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, index, term) + return nil, etcdErr.NewError(etcdErr.EcodeTestFailed, cause, s.CurrentIndex) } // Delete function deletes the node at the given path. // If the node is a directory, recursive must be true to delete it. -func (s *store) Delete(nodePath string, recursive bool, index uint64, term uint64) (*Event, error) { +func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) s.worldLock.Lock() defer s.worldLock.Unlock() - n, err := s.internalGet(nodePath, index, term) + n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error s.Stats.Inc(DeleteFail) return nil, err } - e := newEvent(Delete, nodePath, index, term) + e := newEvent(Delete, nodePath, s.CurrentIndex) if n.IsDir() { e.Dir = true @@ -240,33 +247,38 @@ func (s *store) Delete(nodePath string, recursive bool, index uint64, term uint6 return nil, err } + // update etcd index + s.CurrentIndex++ + e.Index = s.CurrentIndex + s.WatcherHub.notify(e) s.Stats.Inc(DeleteSuccess) return e, nil } -func (s *store) Watch(prefix string, recursive bool, sinceIndex uint64, index uint64, term uint64) (<-chan *Event, error) { +func (s *store) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) { prefix = path.Clean(path.Join("/", prefix)) + nextIndex := s.CurrentIndex + 1 + s.worldLock.RLock() defer s.worldLock.RUnlock() - s.Index, s.Term = index, term - var c <-chan *Event var err *etcdErr.Error if sinceIndex == 0 { - c, err = s.WatcherHub.watch(prefix, recursive, index+1) + c, err = s.WatcherHub.watch(prefix, recursive, nextIndex) } else { c, err = s.WatcherHub.watch(prefix, recursive, sinceIndex) } if err != nil { - err.Index = index - err.Term = term + // watchhub do not know the current Index + // we need to attach the currentIndex here + err.Index = s.CurrentIndex return nil, err } @@ -298,52 +310,59 @@ func (s *store) walk(nodePath string, walkFunc func(prev *Node, component string // Update function updates the value/ttl of the node. // If the node is a file, the value and the ttl can be updated. // If the node is a directory, only the ttl can be updated. -func (s *store) Update(nodePath string, newValue string, expireTime time.Time, index uint64, term uint64) (*Event, error) { +func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (*Event, error) { s.worldLock.Lock() defer s.worldLock.Unlock() + + currIndex, nextIndex := s.CurrentIndex, s.CurrentIndex+1 + nodePath = path.Clean(path.Join("/", nodePath)) - n, err := s.internalGet(nodePath, index, term) + n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error s.Stats.Inc(UpdateFail) return nil, err } - e := newEvent(Update, nodePath, s.Index, s.Term) + e := newEvent(Update, nodePath, nextIndex) if len(newValue) != 0 { if n.IsDir() { // if the node is a directory, we cannot update value s.Stats.Inc(UpdateFail) - return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, index, term) + return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex) } e.PrevValue = n.Value - n.Write(newValue, index, term) + n.Write(newValue, nextIndex) + e.Value = newValue + } else { + // do not update value + e.Value = n.Value } // update ttl n.UpdateTTL(expireTime) - e.Value = newValue - e.Expiration, e.TTL = n.ExpirationAndTTL() s.WatcherHub.notify(e) s.Stats.Inc(UpdateSuccess) + s.CurrentIndex = nextIndex + return e, nil } func (s *store) internalCreate(nodePath string, value string, unique bool, replace bool, - expireTime time.Time, index uint64, term uint64, action string) (*Event, error) { + expireTime time.Time, action string) (*Event, error) { - s.Index, s.Term = index, term + currIndex, nextIndex := s.CurrentIndex, s.CurrentIndex+1 if unique { // append unique item under the node path - nodePath += "/" + strconv.FormatUint(index, 10) + nodePath += "/" + strconv.FormatUint(nextIndex, 10) } nodePath = path.Clean(path.Join("/", nodePath)) @@ -355,11 +374,11 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla if err != nil { s.Stats.Inc(SetFail) - err.Index, err.Term = s.Index, s.Term + err.Index = currIndex return nil, err } - e := newEvent(action, nodePath, s.Index, s.Term) + e := newEvent(action, nodePath, nextIndex) n, _ := d.GetChild(newNodeName) @@ -367,25 +386,25 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla if n != nil { if replace { if n.IsDir() { - return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, index, term) + return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex) } e.PrevValue, _ = n.Read() n.Remove(false, nil) } else { - return nil, etcdErr.NewError(etcdErr.EcodeNodeExist, nodePath, index, term) + return nil, etcdErr.NewError(etcdErr.EcodeNodeExist, nodePath, currIndex) } } if len(value) != 0 { // create file e.Value = value - n = newKV(s, nodePath, value, index, term, d, "", expireTime) + n = newKV(s, nodePath, value, nextIndex, d, "", expireTime) } else { // create directory e.Dir = true - n = newDir(s, nodePath, index, term, d, "", expireTime) + n = newDir(s, nodePath, nextIndex, d, "", expireTime) } @@ -399,23 +418,20 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla e.Expiration, e.TTL = n.ExpirationAndTTL() } + s.CurrentIndex = nextIndex + s.WatcherHub.notify(e) return e, nil } // InternalGet function get the node of the given nodePath. -func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, *etcdErr.Error) { +func (s *store) internalGet(nodePath string) (*Node, *etcdErr.Error) { nodePath = path.Clean(path.Join("/", nodePath)) - // update file system known index and term - if index > s.Index { - s.Index, s.Term = index, term - } - walkFunc := func(parent *Node, name string) (*Node, *etcdErr.Error) { if !parent.IsDir() { - err := etcdErr.NewError(etcdErr.EcodeNotDir, parent.Path, index, term) + err := etcdErr.NewError(etcdErr.EcodeNotDir, parent.Path, s.CurrentIndex) return nil, err } @@ -424,7 +440,7 @@ func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, return child, nil } - return nil, etcdErr.NewError(etcdErr.EcodeKeyNotFound, path.Join(parent.Path, name), index, term) + return nil, etcdErr.NewError(etcdErr.EcodeKeyNotFound, path.Join(parent.Path, name), s.CurrentIndex) } f, err := s.walk(nodePath, walkFunc) @@ -436,12 +452,10 @@ func (s *store) internalGet(nodePath string, index uint64, term uint64) (*Node, } // deleteExpiredKyes will delete all -func (s *store) DeleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { +func (s *store) DeleteExpiredKeys(cutoff time.Time) { s.worldLock.Lock() defer s.worldLock.Unlock() - s.Index, s.Term = index, term - for { node := s.ttlKeyHeap.top() if node == nil || node.ExpireTime.After(cutoff) { @@ -451,10 +465,12 @@ func (s *store) DeleteExpiredKeys(cutoff time.Time, index uint64, term uint64) { s.ttlKeyHeap.pop() node.Remove(true, nil) + s.CurrentIndex++ + s.Stats.Inc(ExpireCount) - s.WatcherHub.notify(newEvent(Expire, node.Path, s.Index, s.Term)) + s.WatcherHub.notify(newEvent(Expire, node.Path, s.CurrentIndex)) } - s.WatcherHub.clearPendingWatchers() + } // checkDir function will check whether the component is a directory under parent node. @@ -469,10 +485,10 @@ func (s *store) checkDir(parent *Node, dirName string) (*Node, *etcdErr.Error) { return node, nil } - return nil, etcdErr.NewError(etcdErr.EcodeNotDir, parent.Path, UndefIndex, UndefTerm) + return nil, etcdErr.NewError(etcdErr.EcodeNotDir, parent.Path, s.CurrentIndex) } - n := newDir(s, path.Join(parent.Path, dirName), s.Index, s.Term, parent, parent.ACL, Permanent) + n := newDir(s, path.Join(parent.Path, dirName), s.CurrentIndex, parent, parent.ACL, Permanent) parent.Children[dirName] = n @@ -487,8 +503,7 @@ func (s *store) Save() ([]byte, error) { s.worldLock.Lock() clonedStore := newStore() - clonedStore.Index = s.Index - clonedStore.Term = s.Term + clonedStore.CurrentIndex = s.CurrentIndex clonedStore.Root = s.Root.Clone() clonedStore.WatcherHub = s.WatcherHub.clone() clonedStore.Stats = s.Stats.clone() diff --git a/store/store_test.go b/store/store_test.go index 1fc242d91..898d510ab 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -11,8 +11,8 @@ import ( // Ensure that the store can retrieve an existing value. func TestStoreGetValue(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, err := s.Get("/foo", false, false, 2, 1) + s.Create("/foo", "bar", false, Permanent) + e, err := s.Get("/foo", false, false) assert.Nil(t, err, "") assert.Equal(t, e.Action, "get", "") assert.Equal(t, e.Key, "/foo", "") @@ -23,14 +23,14 @@ func TestStoreGetValue(t *testing.T) { // Note that hidden files should not be returned. func TestStoreGetDirectory(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - s.Create("/foo/bar", "X", false, Permanent, 3, 1) - s.Create("/foo/_hidden", "*", false, Permanent, 4, 1) - s.Create("/foo/baz", "", false, Permanent, 5, 1) - s.Create("/foo/baz/bat", "Y", false, Permanent, 6, 1) - s.Create("/foo/baz/_hidden", "*", false, Permanent, 7, 1) - s.Create("/foo/baz/ttl", "Y", false, time.Now().Add(time.Second*3), 8, 1) - e, err := s.Get("/foo", true, false, 8, 1) + s.Create("/foo", "", false, Permanent) + s.Create("/foo/bar", "X", false, Permanent) + s.Create("/foo/_hidden", "*", false, Permanent) + s.Create("/foo/baz", "", false, Permanent) + s.Create("/foo/baz/bat", "Y", false, Permanent) + s.Create("/foo/baz/_hidden", "*", false, Permanent) + s.Create("/foo/baz/ttl", "Y", false, time.Now().Add(time.Second*3)) + e, err := s.Get("/foo", true, false) assert.Nil(t, err, "") assert.Equal(t, e.Action, "get", "") assert.Equal(t, e.Key, "/foo", "") @@ -53,13 +53,13 @@ func TestStoreGetDirectory(t *testing.T) { // Ensure that the store can retrieve a directory in sorted order. func TestStoreGetSorted(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - s.Create("/foo/x", "0", false, Permanent, 3, 1) - s.Create("/foo/z", "0", false, Permanent, 4, 1) - s.Create("/foo/y", "", false, Permanent, 5, 1) - s.Create("/foo/y/a", "0", false, Permanent, 6, 1) - s.Create("/foo/y/b", "0", false, Permanent, 7, 1) - e, err := s.Get("/foo", true, true, 8, 1) + s.Create("/foo", "", false, Permanent) + s.Create("/foo/x", "0", false, Permanent) + s.Create("/foo/z", "0", false, Permanent) + s.Create("/foo/y", "", false, Permanent) + s.Create("/foo/y/a", "0", false, Permanent) + s.Create("/foo/y/b", "0", false, Permanent) + e, err := s.Get("/foo", true, true) assert.Nil(t, err, "") assert.Equal(t, e.KVPairs[0].Key, "/foo/x", "") assert.Equal(t, e.KVPairs[1].Key, "/foo/y", "") @@ -71,7 +71,7 @@ func TestStoreGetSorted(t *testing.T) { // Ensure that the store can create a new key if it doesn't already exist. func TestStoreCreateValue(t *testing.T) { s := newStore() - e, err := s.Create("/foo", "bar", false, Permanent, 2, 1) + e, err := s.Create("/foo", "bar", false, Permanent) assert.Nil(t, err, "") assert.Equal(t, e.Action, "create", "") assert.Equal(t, e.Key, "/foo", "") @@ -81,14 +81,13 @@ func TestStoreCreateValue(t *testing.T) { assert.Nil(t, e.KVPairs, "") assert.Nil(t, e.Expiration, "") assert.Equal(t, e.TTL, 0, "") - assert.Equal(t, e.Index, uint64(2), "") - assert.Equal(t, e.Term, uint64(1), "") + assert.Equal(t, e.Index, uint64(1), "") } // Ensure that the store can create a new directory if it doesn't already exist. func TestStoreCreateDirectory(t *testing.T) { s := newStore() - e, err := s.Create("/foo", "", false, Permanent, 2, 1) + e, err := s.Create("/foo", "", false, Permanent) assert.Nil(t, err, "") assert.Equal(t, e.Action, "create", "") assert.Equal(t, e.Key, "/foo", "") @@ -98,22 +97,21 @@ func TestStoreCreateDirectory(t *testing.T) { // Ensure that the store fails to create a key if it already exists. func TestStoreCreateFailsIfExists(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - e, _err := s.Create("/foo", "", false, Permanent, 3, 1) + s.Create("/foo", "", false, Permanent) + e, _err := s.Create("/foo", "", false, Permanent) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeNodeExist, "") assert.Equal(t, err.Message, "Already exists", "") assert.Equal(t, err.Cause, "/foo", "") - assert.Equal(t, err.Index, uint64(3), "") - assert.Equal(t, err.Term, uint64(1), "") + assert.Equal(t, err.Index, uint64(1), "") assert.Nil(t, e, 0, "") } // Ensure that the store can update a key if it already exists. func TestStoreUpdateValue(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, err := s.Update("/foo", "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, err := s.Update("/foo", "baz", Permanent) assert.Nil(t, err, "") assert.Equal(t, e.Action, "update", "") assert.Equal(t, e.Key, "/foo", "") @@ -121,17 +119,16 @@ func TestStoreUpdateValue(t *testing.T) { assert.Equal(t, e.PrevValue, "bar", "") assert.Equal(t, e.Value, "baz", "") assert.Equal(t, e.TTL, 0, "") - assert.Equal(t, e.Index, uint64(3), "") - assert.Equal(t, e.Term, uint64(1), "") - e, _ = s.Get("/foo", false, false, 3, 1) + assert.Equal(t, e.Index, uint64(2), "") + e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "baz", "") } // Ensure that the store cannot update a directory. func TestStoreUpdateFailsIfDirectory(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - e, _err := s.Update("/foo", "baz", Permanent, 3, 1) + s.Create("/foo", "", false, Permanent) + e, _err := s.Update("/foo", "baz", Permanent) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeNotFile, "") assert.Equal(t, err.Message, "Not A File", "") @@ -142,14 +139,14 @@ func TestStoreUpdateFailsIfDirectory(t *testing.T) { // Ensure that the store can update the TTL on a value. func TestStoreUpdateValueTTL(t *testing.T) { s := newStore() - go mockSyncService(s.deleteExpiredKeys) - s.Create("/foo", "bar", false, Permanent, 2, 1) - _, err := s.Update("/foo", "baz", time.Now().Add(500*time.Millisecond), 3, 1) - e, _ := s.Get("/foo", false, false, 3, 1) + go mockSyncService(s.DeleteExpiredKeys) + s.Create("/foo", "bar", false, Permanent) + _, err := s.Update("/foo", "baz", time.Now().Add(500*time.Millisecond)) + e, _ := s.Get("/foo", false, false) assert.Equal(t, e.Value, "baz", "") time.Sleep(600 * time.Millisecond) - e, err = s.Get("/foo", false, false, 3, 1) + e, err = s.Get("/foo", false, false) assert.Nil(t, e, "") assert.Equal(t, err.(*etcdErr.Error).ErrorCode, etcdErr.EcodeKeyNotFound, "") } @@ -157,15 +154,15 @@ func TestStoreUpdateValueTTL(t *testing.T) { // Ensure that the store can update the TTL on a directory. func TestStoreUpdateDirTTL(t *testing.T) { s := newStore() - go mockSyncService(s.deleteExpiredKeys) - s.Create("/foo", "", false, Permanent, 2, 1) - s.Create("/foo/bar", "baz", false, Permanent, 3, 1) - _, err := s.Update("/foo", "", time.Now().Add(500*time.Millisecond), 3, 1) - e, _ := s.Get("/foo/bar", false, false, 3, 1) + go mockSyncService(s.DeleteExpiredKeys) + s.Create("/foo", "", false, Permanent) + s.Create("/foo/bar", "baz", false, Permanent) + _, err := s.Update("/foo", "", time.Now().Add(500*time.Millisecond)) + e, _ := s.Get("/foo/bar", false, false) assert.Equal(t, e.Value, "baz", "") time.Sleep(600 * time.Millisecond) - e, err = s.Get("/foo/bar", false, false, 3, 1) + e, err = s.Get("/foo/bar", false, false) assert.Nil(t, e, "") assert.Equal(t, err.(*etcdErr.Error).ErrorCode, etcdErr.EcodeKeyNotFound, "") } @@ -173,8 +170,8 @@ func TestStoreUpdateDirTTL(t *testing.T) { // Ensure that the store can delete a value. func TestStoreDeleteValue(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, err := s.Delete("/foo", false, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, err := s.Delete("/foo", false) assert.Nil(t, err, "") assert.Equal(t, e.Action, "delete", "") } @@ -182,8 +179,8 @@ func TestStoreDeleteValue(t *testing.T) { // Ensure that the store can delete a directory if recursive is specified. func TestStoreDeleteDiretory(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - e, err := s.Delete("/foo", true, 3, 1) + s.Create("/foo", "", false, Permanent) + e, err := s.Delete("/foo", true) assert.Nil(t, err, "") assert.Equal(t, e.Action, "delete", "") } @@ -191,8 +188,8 @@ func TestStoreDeleteDiretory(t *testing.T) { // Ensure that the store cannot delete a directory if recursive is not specified. func TestStoreDeleteDiretoryFailsIfNonRecursive(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - e, _err := s.Delete("/foo", false, 3, 1) + s.Create("/foo", "", false, Permanent) + e, _err := s.Delete("/foo", false) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeNotFile, "") assert.Equal(t, err.Message, "Not A File", "") @@ -202,60 +199,60 @@ func TestStoreDeleteDiretoryFailsIfNonRecursive(t *testing.T) { // Ensure that the store can conditionally update a key if it has a previous value. func TestStoreCompareAndSwapPrevValue(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, err := s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent) assert.Nil(t, err, "") assert.Equal(t, e.Action, "compareAndSwap", "") assert.Equal(t, e.PrevValue, "bar", "") assert.Equal(t, e.Value, "baz", "") - e, _ = s.Get("/foo", false, false, 3, 1) + e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "baz", "") } // Ensure that the store cannot conditionally update a key if it has the wrong previous value. func TestStoreCompareAndSwapPrevValueFailsIfNotMatch(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, _err := s.CompareAndSwap("/foo", "wrong_value", 0, "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndSwap("/foo", "wrong_value", 0, "baz", Permanent) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") assert.Equal(t, err.Message, "Test Failed", "") assert.Nil(t, e, "") - e, _ = s.Get("/foo", false, false, 3, 1) + e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "bar", "") } // Ensure that the store can conditionally update a key if it has a previous index. func TestStoreCompareAndSwapPrevIndex(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, err := s.CompareAndSwap("/foo", "", 2, "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, err := s.CompareAndSwap("/foo", "", 1, "baz", Permanent) assert.Nil(t, err, "") assert.Equal(t, e.Action, "compareAndSwap", "") assert.Equal(t, e.PrevValue, "bar", "") assert.Equal(t, e.Value, "baz", "") - e, _ = s.Get("/foo", false, false, 3, 1) + e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "baz", "") } // Ensure that the store cannot conditionally update a key if it has the wrong previous index. func TestStoreCompareAndSwapPrevIndexFailsIfNotMatch(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - e, _err := s.CompareAndSwap("/foo", "", 100, "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + e, _err := s.CompareAndSwap("/foo", "", 100, "baz", Permanent) err := _err.(*etcdErr.Error) assert.Equal(t, err.ErrorCode, etcdErr.EcodeTestFailed, "") assert.Equal(t, err.Message, "Test Failed", "") assert.Nil(t, e, "") - e, _ = s.Get("/foo", false, false, 3, 1) + e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "bar", "") } // Ensure that the store can watch for key creation. func TestStoreWatchCreate(t *testing.T) { s := newStore() - c, _ := s.Watch("/foo", false, 0, 0, 1) - s.Create("/foo", "bar", false, Permanent, 2, 1) + c, _ := s.Watch("/foo", false, 0) + s.Create("/foo", "bar", false, Permanent) e := nbselect(c) assert.Equal(t, e.Action, "create", "") assert.Equal(t, e.Key, "/foo", "") @@ -266,8 +263,8 @@ func TestStoreWatchCreate(t *testing.T) { // Ensure that the store can watch for recursive key creation. func TestStoreWatchRecursiveCreate(t *testing.T) { s := newStore() - c, _ := s.Watch("/foo", true, 0, 0, 1) - s.Create("/foo/bar", "baz", false, Permanent, 2, 1) + c, _ := s.Watch("/foo", true, 0) + s.Create("/foo/bar", "baz", false, Permanent) e := nbselect(c) assert.Equal(t, e.Action, "create", "") assert.Equal(t, e.Key, "/foo/bar", "") @@ -276,9 +273,9 @@ func TestStoreWatchRecursiveCreate(t *testing.T) { // Ensure that the store can watch for key updates. func TestStoreWatchUpdate(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", false, 0, 0, 1) - s.Update("/foo", "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + c, _ := s.Watch("/foo", false, 0) + s.Update("/foo", "baz", Permanent) e := nbselect(c) assert.Equal(t, e.Action, "update", "") assert.Equal(t, e.Key, "/foo", "") @@ -287,9 +284,9 @@ func TestStoreWatchUpdate(t *testing.T) { // Ensure that the store can watch for recursive key updates. func TestStoreWatchRecursiveUpdate(t *testing.T) { s := newStore() - s.Create("/foo/bar", "baz", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", true, 0, 0, 1) - s.Update("/foo/bar", "baz", Permanent, 3, 1) + s.Create("/foo/bar", "baz", false, Permanent) + c, _ := s.Watch("/foo", true, 0) + s.Update("/foo/bar", "baz", Permanent) e := nbselect(c) assert.Equal(t, e.Action, "update", "") assert.Equal(t, e.Key, "/foo/bar", "") @@ -298,9 +295,9 @@ func TestStoreWatchRecursiveUpdate(t *testing.T) { // Ensure that the store can watch for key deletions. func TestStoreWatchDelete(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", false, 0, 0, 1) - s.Delete("/foo", false, 3, 1) + s.Create("/foo", "bar", false, Permanent) + c, _ := s.Watch("/foo", false, 0) + s.Delete("/foo", false) e := nbselect(c) assert.Equal(t, e.Action, "delete", "") assert.Equal(t, e.Key, "/foo", "") @@ -309,9 +306,9 @@ func TestStoreWatchDelete(t *testing.T) { // Ensure that the store can watch for recursive key deletions. func TestStoreWatchRecursiveDelete(t *testing.T) { s := newStore() - s.Create("/foo/bar", "baz", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", true, 0, 0, 1) - s.Delete("/foo/bar", false, 3, 1) + s.Create("/foo/bar", "baz", false, Permanent) + c, _ := s.Watch("/foo", true, 0) + s.Delete("/foo/bar", false) e := nbselect(c) assert.Equal(t, e.Action, "delete", "") assert.Equal(t, e.Key, "/foo/bar", "") @@ -320,9 +317,9 @@ func TestStoreWatchRecursiveDelete(t *testing.T) { // Ensure that the store can watch for CAS updates. func TestStoreWatchCompareAndSwap(t *testing.T) { s := newStore() - s.Create("/foo", "bar", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", false, 0, 0, 1) - s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent, 3, 1) + s.Create("/foo", "bar", false, Permanent) + c, _ := s.Watch("/foo", false, 0) + s.CompareAndSwap("/foo", "bar", 0, "baz", Permanent) e := nbselect(c) assert.Equal(t, e.Action, "compareAndSwap", "") assert.Equal(t, e.Key, "/foo", "") @@ -331,9 +328,9 @@ func TestStoreWatchCompareAndSwap(t *testing.T) { // Ensure that the store can watch for recursive CAS updates. func TestStoreWatchRecursiveCompareAndSwap(t *testing.T) { s := newStore() - s.Create("/foo/bar", "baz", false, Permanent, 2, 1) - c, _ := s.Watch("/foo", true, 0, 0, 1) - s.CompareAndSwap("/foo/bar", "baz", 0, "bat", Permanent, 3, 1) + s.Create("/foo/bar", "baz", false, Permanent) + c, _ := s.Watch("/foo", true, 0) + s.CompareAndSwap("/foo/bar", "baz", 0, "bat", Permanent) e := nbselect(c) assert.Equal(t, e.Action, "compareAndSwap", "") assert.Equal(t, e.Key, "/foo/bar", "") @@ -342,17 +339,18 @@ func TestStoreWatchRecursiveCompareAndSwap(t *testing.T) { // Ensure that the store can watch for key expiration. func TestStoreWatchExpire(t *testing.T) { s := newStore() - go mockSyncService(s.deleteExpiredKeys) - s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond), 2, 1) - s.Create("/foofoo", "barbarbar", false, time.Now().Add(500*time.Millisecond), 2, 1) + go mockSyncService(s.DeleteExpiredKeys) + s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond)) + s.Create("/foofoo", "barbarbar", false, time.Now().Add(500*time.Millisecond)) - c, _ := s.Watch("/", true, 0, 0, 1) + c, _ := s.Watch("/", true, 0) e := nbselect(c) assert.Nil(t, e, "") time.Sleep(600 * time.Millisecond) e = nbselect(c) assert.Equal(t, e.Action, "expire", "") assert.Equal(t, e.Key, "/foo", "") + c, _ = s.Watch("/", true, 4) e = nbselect(c) assert.Equal(t, e.Action, "expire", "") assert.Equal(t, e.Key, "/foofoo", "") @@ -361,19 +359,19 @@ func TestStoreWatchExpire(t *testing.T) { // Ensure that the store can recover from a previously saved state. func TestStoreRecover(t *testing.T) { s := newStore() - s.Create("/foo", "", false, Permanent, 2, 1) - s.Create("/foo/x", "bar", false, Permanent, 3, 1) - s.Create("/foo/y", "baz", false, Permanent, 4, 1) + s.Create("/foo", "", false, Permanent) + s.Create("/foo/x", "bar", false, Permanent) + s.Create("/foo/y", "baz", false, Permanent) b, err := s.Save() s2 := newStore() s2.Recovery(b) - e, err := s.Get("/foo/x", false, false, 4, 1) + e, err := s.Get("/foo/x", false, false) assert.Nil(t, err, "") assert.Equal(t, e.Value, "bar", "") - e, err = s.Get("/foo/y", false, false, 4, 1) + e, err = s.Get("/foo/y", false, false) assert.Nil(t, err, "") assert.Equal(t, e.Value, "baz", "") } @@ -381,25 +379,25 @@ func TestStoreRecover(t *testing.T) { // Ensure that the store can recover from a previously saved state that includes an expiring key. func TestStoreRecoverWithExpiration(t *testing.T) { s := newStore() - go mockSyncService(s.deleteExpiredKeys) - s.Create("/foo", "", false, Permanent, 2, 1) - s.Create("/foo/x", "bar", false, Permanent, 3, 1) - s.Create("/foo/y", "baz", false, time.Now().Add(5*time.Millisecond), 4, 1) + go mockSyncService(s.DeleteExpiredKeys) + s.Create("/foo", "", false, Permanent) + s.Create("/foo/x", "bar", false, Permanent) + s.Create("/foo/y", "baz", false, time.Now().Add(5*time.Millisecond)) b, err := s.Save() time.Sleep(10 * time.Millisecond) s2 := newStore() - go mockSyncService(s2.deleteExpiredKeys) + go mockSyncService(s2.DeleteExpiredKeys) s2.Recovery(b) time.Sleep(600 * time.Millisecond) - e, err := s.Get("/foo/x", false, false, 4, 1) + e, err := s.Get("/foo/x", false, false) assert.Nil(t, err, "") assert.Equal(t, e.Value, "bar", "") - e, err = s.Get("/foo/y", false, false, 4, 1) + e, err = s.Get("/foo/y", false, false) assert.NotNil(t, err, "") assert.Nil(t, e, "") } @@ -414,9 +412,9 @@ func nbselect(c <-chan *Event) *Event { } } -func mockSyncService(f func(now time.Time, index uint64, term uint64)) { +func mockSyncService(f func(now time.Time)) { ticker := time.Tick(time.Millisecond * 500) for now := range ticker { - f(now, 2, 1) + f(now) } } diff --git a/store/v2/compare_and_swap_command.go b/store/v2/compare_and_swap_command.go index 1de79fb1f..fe7572bd7 100644 --- a/store/v2/compare_and_swap_command.go +++ b/store/v2/compare_and_swap_command.go @@ -30,8 +30,7 @@ func (c *CompareAndSwapCommand) CommandName() string { func (c *CompareAndSwapCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.CompareAndSwap(c.Key, c.PrevValue, c.PrevIndex, - c.Value, c.ExpireTime, server.CommitIndex(), server.Term()) + e, err := s.CompareAndSwap(c.Key, c.PrevValue, c.PrevIndex, c.Value, c.ExpireTime) if err != nil { log.Debug(err) diff --git a/store/v2/create_command.go b/store/v2/create_command.go index e187d99f7..a6863ce00 100644 --- a/store/v2/create_command.go +++ b/store/v2/create_command.go @@ -29,7 +29,7 @@ func (c *CreateCommand) CommandName() string { func (c *CreateCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.Create(c.Key, c.Value, c.Unique, c.ExpireTime, server.CommitIndex(), server.Term()) + e, err := s.Create(c.Key, c.Value, c.Unique, c.ExpireTime) if err != nil { log.Debug(err) diff --git a/store/v2/delete_command.go b/store/v2/delete_command.go index 3e8bac81c..1ac1d3987 100644 --- a/store/v2/delete_command.go +++ b/store/v2/delete_command.go @@ -25,7 +25,7 @@ func (c *DeleteCommand) CommandName() string { func (c *DeleteCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.Delete(c.Key, c.Recursive, server.CommitIndex(), server.Term()) + e, err := s.Delete(c.Key, c.Recursive) if err != nil { log.Debug(err) diff --git a/store/v2/set_command.go b/store/v2/set_command.go index 4f6ecf59f..c34071853 100644 --- a/store/v2/set_command.go +++ b/store/v2/set_command.go @@ -29,7 +29,7 @@ func (c *SetCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) // create a new node or replace the old node. - e, err := s.Set(c.Key, c.Value, c.ExpireTime, server.CommitIndex(), server.Term()) + e, err := s.Set(c.Key, c.Value, c.ExpireTime) if err != nil { log.Debug(err) diff --git a/store/v2/update_command.go b/store/v2/update_command.go index d080ecced..19eb68622 100644 --- a/store/v2/update_command.go +++ b/store/v2/update_command.go @@ -27,7 +27,7 @@ func (c *UpdateCommand) CommandName() string { func (c *UpdateCommand) Apply(server raft.Server) (interface{}, error) { s, _ := server.StateMachine().(store.Store) - e, err := s.Update(c.Key, c.Value, c.ExpireTime, server.CommitIndex(), server.Term()) + e, err := s.Update(c.Key, c.Value, c.ExpireTime) if err != nil { log.Debug(err) diff --git a/store/watcher_hub.go b/store/watcher_hub.go index 52af68c21..b952aec6d 100644 --- a/store/watcher_hub.go +++ b/store/watcher_hub.go @@ -16,11 +16,9 @@ import ( // event happens between the end of the first watch command and the start // of the second command. type watcherHub struct { - watchers map[string]*list.List - count int64 // current number of watchers. - EventHistory *EventHistory - pendingWatchers map[*list.Element]*list.List - pendingList map[*list.List]string + watchers map[string]*list.List + count int64 // current number of watchers. + EventHistory *EventHistory } // newWatchHub creates a watchHub. The capacity determines how many events we will @@ -29,10 +27,8 @@ type watcherHub struct { // Ideally, it should smaller than 20K/s[max throughput] * 2 * 50ms[RTT] = 2000 func newWatchHub(capacity int) *watcherHub { return &watcherHub{ - watchers: make(map[string]*list.List), - EventHistory: newEventHistory(capacity), - pendingWatchers: make(map[*list.Element]*list.List), - pendingList: make(map[*list.List]string), + watchers: make(map[string]*list.List), + EventHistory: newEventHistory(capacity), } } @@ -41,22 +37,16 @@ func newWatchHub(capacity int) *watcherHub { // If recursive is false, the first change after index at prefix will be sent to the event channel. // If index is zero, watch will start from the current index + 1. func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan *Event, *etcdErr.Error) { - events, err := wh.EventHistory.scan(prefix, index) + event, err := wh.EventHistory.scan(prefix, index) if err != nil { return nil, err } - eventChan := make(chan *Event, len(events)+5) // use a buffered channel + eventChan := make(chan *Event, 1) // use a buffered channel - if events != nil { - for _, e := range events { - eventChan <- e - } - - if events[0].Action == Expire { - eventChan <- nil - } + if event != nil { + eventChan <- event return eventChan, nil } @@ -123,16 +113,11 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { if w.notify(e, e.Key == path, deleted) { - if e.Action == Expire { - wh.pendingWatchers[curr] = l - wh.pendingList[l] = path - } else { - // if we successfully notify a watcher - // we need to remove the watcher from the list - // and decrease the counter - l.Remove(curr) - atomic.AddInt64(&wh.count, -1) - } + // if we successfully notify a watcher + // we need to remove the watcher from the list + // and decrease the counter + l.Remove(curr) + atomic.AddInt64(&wh.count, -1) } @@ -141,27 +126,6 @@ func (wh *watcherHub) notifyWatchers(e *Event, path string, deleted bool) { } } -func (wh *watcherHub) clearPendingWatchers() { - if len(wh.pendingWatchers) == 0 { // avoid making new maps - return - } - - for e, l := range wh.pendingWatchers { - l.Remove(e) - - if l.Len() == 0 { - path := wh.pendingList[l] - delete(wh.watchers, path) - } - - w, _ := e.Value.(*watcher) - w.eventChan <- nil - } - - wh.pendingWatchers = make(map[*list.Element]*list.List) - wh.pendingList = make(map[*list.List]string) -} - // clone function clones the watcherHub and return the cloned one. // only clone the static content. do not clone the current watchers. func (wh *watcherHub) clone() *watcherHub { diff --git a/store/watcher_test.go b/store/watcher_test.go index c3da475fc..d1b62b762 100644 --- a/store/watcher_test.go +++ b/store/watcher_test.go @@ -19,7 +19,7 @@ func TestWatcher(t *testing.T) { // do nothing } - e := newEvent(Create, "/foo/bar", 1, 1) + e := newEvent(Create, "/foo/bar", 1) wh.notify(e) @@ -31,7 +31,7 @@ func TestWatcher(t *testing.T) { c, _ = wh.watch("/foo", false, 2) - e = newEvent(Create, "/foo/bar", 2, 1) + e = newEvent(Create, "/foo/bar", 2) wh.notify(e) @@ -42,7 +42,7 @@ func TestWatcher(t *testing.T) { // do nothing } - e = newEvent(Create, "/foo", 3, 1) + e = newEvent(Create, "/foo", 3) wh.notify(e) From eca433cee5736152609b602bfdea7e5ec3f8f577 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 18:59:43 -0800 Subject: [PATCH 22/31] fix add sync_command.go --- store/v2/sync_command.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 store/v2/sync_command.go diff --git a/store/v2/sync_command.go b/store/v2/sync_command.go new file mode 100644 index 000000000..4b6459079 --- /dev/null +++ b/store/v2/sync_command.go @@ -0,0 +1,29 @@ +package v2 + +import ( + "time" + + "github.com/coreos/etcd/store" + "github.com/coreos/go-raft" +) + +func init() { + raft.RegisterCommand(&SyncCommand{}) +} + +type SyncCommand struct { + Time time.Time `json:"time"` +} + +// The name of the Sync command in the log +func (c SyncCommand) CommandName() string { + return "etcd:sync" +} + +func (c SyncCommand) Apply(server raft.Server) (interface{}, error) { + + s, _ := server.StateMachine().(store.Store) + s.DeleteExpiredKeys(c.Time) + + return nil, nil +} From d87e0e93d3b14b9e606fbac81448c8981703b4d4 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 19:05:38 -0800 Subject: [PATCH 23/31] fix get return the last modified index of the node --- store/node.go | 1 - store/store.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/store/node.go b/store/node.go index a0916b81a..7ed78303c 100644 --- a/store/node.go +++ b/store/node.go @@ -19,7 +19,6 @@ type Node struct { CreateIndex uint64 CreateTerm uint64 ModifiedIndex uint64 - ModifiedTerm uint64 Parent *Node `json:"-"` // should not encode this field! avoid circular dependency. diff --git a/store/store.go b/store/store.go index f9ebab7ad..04bacb711 100644 --- a/store/store.go +++ b/store/store.go @@ -91,7 +91,7 @@ func (s *store) Get(nodePath string, recursive, sorted bool) (*Event, error) { return nil, err } - e := newEvent(Get, nodePath, s.CurrentIndex) + e := newEvent(Get, nodePath, n.ModifiedIndex) if n.IsDir() { // node is a directory e.Dir = true From d8e5994c352ae8929b9f89bdcf65f98e90edb8e2 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 20:20:47 -0800 Subject: [PATCH 24/31] feat attach etcd-index,raft-index,raft-term to header --- server/server.go | 7 +++++++ server/v2/get_handler.go | 4 +++- store/node.go | 1 - 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/server/server.go b/server/server.go index e2b54f8b5..a435fbce0 100644 --- a/server/server.go +++ b/server/server.go @@ -225,6 +225,7 @@ func (s *Server) Close() { } } +// Dispatch command to the current leader func (s *Server) Dispatch(c raft.Command, w http.ResponseWriter, req *http.Request) error { ps := s.peerServer if ps.raftServer.State() == raft.Leader { @@ -252,6 +253,12 @@ func (s *Server) Dispatch(c raft.Command, w http.ResponseWriter, req *http.Reque e, _ := result.(*store.Event) b, _ = json.Marshal(e) + // etcd index should be the same as the event index + // which is also the last modified index of the node + w.Header().Add("X-Etcd-Index", fmt.Sprint(e.Index)) + w.Header().Add("X-Raft-Index", fmt.Sprint(s.CommitIndex())) + w.Header().Add("X-Raft-Term", fmt.Sprint(s.Term())) + if e.IsCreated() { w.WriteHeader(http.StatusCreated) } else { diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index f2d05c507..3e8ddee64 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -68,7 +68,9 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { } } - w.Header().Add("X-Etcd-Index", fmt.Sprint(event.Index)) + w.Header().Add("X-Etcd-Index", fmt.Sprint(s.Store().Index())) + w.Header().Add("X-Raft-Index", fmt.Sprint(s.CommitIndex())) + w.Header().Add("X-Raft-Term", fmt.Sprint(s.Term())) w.WriteHeader(http.StatusOK) b, _ := json.Marshal(event) diff --git a/store/node.go b/store/node.go index 7ed78303c..9a7196bd3 100644 --- a/store/node.go +++ b/store/node.go @@ -17,7 +17,6 @@ type Node struct { Path string CreateIndex uint64 - CreateTerm uint64 ModifiedIndex uint64 Parent *Node `json:"-"` // should not encode this field! avoid circular dependency. From 06f1b7f2e8f4ae203adf384a4e77b9dd425bc19e Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 20:49:19 -0800 Subject: [PATCH 25/31] refactor change Index to modifiedIndex --- server/v2/tests/delete_handler_test.go | 2 +- server/v2/tests/get_handler_test.go | 8 +++--- server/v2/tests/post_handler_test.go | 2 +- server/v2/tests/put_handler_test.go | 6 ++--- store/event.go | 34 ++++++++++++++------------ store/event_history.go | 6 ++--- store/event_test.go | 8 +++--- store/node.go | 15 ++++++------ store/store.go | 7 +++--- store/store_test.go | 4 +-- store/watcher.go | 2 +- 11 files changed, 50 insertions(+), 44 deletions(-) diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index 2905c68c4..997127a9e 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -24,6 +24,6 @@ func TestV2DeleteKey(t *testing.T) { resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), url.Values{}) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","index":2}`, "") + assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","modifiedIndex":2}`, "") }) } diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index 0f0054055..b15195873 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -27,7 +27,7 @@ func TestV2GetKey(t *testing.T) { assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["index"], 1, "") + assert.Equal(t, body["modifiedIndex"], 1, "") }) } @@ -54,7 +54,7 @@ func TestV2GetKeyRecursively(t *testing.T) { assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["index"], 2, "") + assert.Equal(t, body["modifiedIndex"], 1, "") assert.Equal(t, len(body["kvs"].([]interface{})), 2, "") kv0 := body["kvs"].([]interface{})[0].(map[string]interface{}) @@ -111,7 +111,7 @@ func TestV2WatchKey(t *testing.T) { assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["index"], 1, "") + assert.Equal(t, body["modifiedIndex"], 1, "") }) } @@ -164,6 +164,6 @@ func TestV2WatchKeyWithIndex(t *testing.T) { assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 2, "") + assert.Equal(t, body["modifiedIndex"], 2, "") }) } diff --git a/server/v2/tests/post_handler_test.go b/server/v2/tests/post_handler_test.go index 655278f22..856633ef0 100644 --- a/server/v2/tests/post_handler_test.go +++ b/server/v2/tests/post_handler_test.go @@ -23,7 +23,7 @@ func TestV2CreateUnique(t *testing.T) { assert.Equal(t, body["action"], "create", "") assert.Equal(t, body["key"], "/foo/bar/1", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["index"], 1, "") + assert.Equal(t, body["modifiedIndex"], 1, "") // Second POST should add next index to list. resp, _ = tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), nil) diff --git a/server/v2/tests/put_handler_test.go b/server/v2/tests/put_handler_test.go index 73f6be91c..3ee642604 100644 --- a/server/v2/tests/put_handler_test.go +++ b/server/v2/tests/put_handler_test.go @@ -22,7 +22,7 @@ func TestV2SetKey(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","index":1}`, "") + assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","modifiedIndex":1}`, "") }) } @@ -175,7 +175,7 @@ func TestV2SetKeyCASOnIndexSuccess(t *testing.T) { assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 2, "") + assert.Equal(t, body["modifiedIndex"], 2, "") }) } @@ -236,7 +236,7 @@ func TestV2SetKeyCASOnValueSuccess(t *testing.T) { assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["index"], 2, "") + assert.Equal(t, body["modifiedIndex"], 2, "") }) } diff --git a/store/event.go b/store/event.go index 50d2872da..126ceee4b 100644 --- a/store/event.go +++ b/store/event.go @@ -15,23 +15,23 @@ const ( ) type Event struct { - Action string `json:"action"` - Key string `json:"key, omitempty"` - Dir bool `json:"dir,omitempty"` - PrevValue string `json:"prevValue,omitempty"` - Value string `json:"value,omitempty"` - KVPairs kvPairs `json:"kvs,omitempty"` - Expiration *time.Time `json:"expiration,omitempty"` - TTL int64 `json:"ttl,omitempty"` // Time to live in second - // The index of the etcd state machine when the comment is executed - Index uint64 `json:"index"` + Action string `json:"action"` + + Key string `json:"key, omitempty"` + Dir bool `json:"dir,omitempty"` + PrevValue string `json:"prevValue,omitempty"` + Value string `json:"value,omitempty"` + KVPairs kvPairs `json:"kvs,omitempty"` + Expiration *time.Time `json:"expiration,omitempty"` + TTL int64 `json:"ttl,omitempty"` // Time to live in second + ModifiedIndex uint64 `json:"modifiedIndex"` } func newEvent(action string, key string, index uint64) *Event { return &Event{ - Action: action, - Key: key, - Index: index, + Action: action, + Key: key, + ModifiedIndex: index, } } @@ -47,6 +47,10 @@ func (e *Event) IsCreated() bool { return false } +func (e *Event) Index() uint64 { + return e.ModifiedIndex +} + // Converts an event object into a response object. func (event *Event) Response() interface{} { if !event.Dir { @@ -55,7 +59,7 @@ func (event *Event) Response() interface{} { Key: event.Key, Value: event.Value, PrevValue: event.PrevValue, - Index: event.Index, + Index: event.ModifiedIndex, TTL: event.TTL, Expiration: event.Expiration, } @@ -80,7 +84,7 @@ func (event *Event) Response() interface{} { Key: kv.Key, Value: kv.Value, Dir: kv.Dir, - Index: event.Index, + Index: event.ModifiedIndex, } } return responses diff --git a/store/event_history.go b/store/event_history.go index aaa93d44e..4fd077184 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -31,9 +31,9 @@ func (eh *EventHistory) addEvent(e *Event) *Event { eh.Queue.insert(e) - eh.LastIndex = e.Index + eh.LastIndex = e.Index() - eh.StartIndex = eh.Queue.Events[eh.Queue.Front].Index + eh.StartIndex = eh.Queue.Events[eh.Queue.Front].ModifiedIndex return e } @@ -62,7 +62,7 @@ func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Erro for { e := eh.Queue.Events[i] - if strings.HasPrefix(e.Key, prefix) && index <= e.Index { // make sure we bypass the smaller one + if strings.HasPrefix(e.Key, prefix) && index <= e.Index() { // make sure we bypass the smaller one return e, nil } diff --git a/store/event_test.go b/store/event_test.go index 8a70d09a2..dc30ce44d 100644 --- a/store/event_test.go +++ b/store/event_test.go @@ -23,7 +23,7 @@ func TestEventQueue(t *testing.T) { n := eh.Queue.Size for ; n > 0; n-- { e := eh.Queue.Events[i] - if e.Index != uint64(j) { + if e.Index() != uint64(j) { t.Fatalf("queue error!") } j++ @@ -42,19 +42,19 @@ func TestScanHistory(t *testing.T) { eh.addEvent(newEvent(Create, "/foo/foo/foo", 5)) e, err := eh.scan("/foo", 1) - if err != nil || e.Index != 1 { + if err != nil || e.Index() != 1 { t.Fatalf("scan error [/foo] [1] %v", e.Index) } e, err = eh.scan("/foo/bar", 1) - if err != nil || e.Index != 2 { + if err != nil || e.Index() != 2 { t.Fatalf("scan error [/foo/bar] [2] %v", e.Index) } e, err = eh.scan("/foo/bar", 3) - if err != nil || e.Index != 4 { + if err != nil || e.Index() != 4 { t.Fatalf("scan error [/foo/bar/bar] [4] %v", e.Index) } diff --git a/store/node.go b/store/node.go index 9a7196bd3..a7fd7853c 100644 --- a/store/node.go +++ b/store/node.go @@ -51,13 +51,14 @@ func newDir(store *store, nodePath string, createIndex uint64, parent *Node, ACL string, expireTime time.Time) *Node { return &Node{ - Path: nodePath, - CreateIndex: createIndex, - Parent: parent, - ACL: ACL, - ExpireTime: expireTime, - Children: make(map[string]*Node), - store: store, + Path: nodePath, + CreateIndex: createIndex, + ModifiedIndex: createIndex, + Parent: parent, + ACL: ACL, + ExpireTime: expireTime, + Children: make(map[string]*Node), + store: store, } } diff --git a/store/store.go b/store/store.go index 04bacb711..ff78320fe 100644 --- a/store/store.go +++ b/store/store.go @@ -220,6 +220,8 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { s.worldLock.Lock() defer s.worldLock.Unlock() + nextIndex := s.CurrentIndex + 1 + n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error @@ -227,7 +229,7 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { return nil, err } - e := newEvent(Delete, nodePath, s.CurrentIndex) + e := newEvent(Delete, nodePath, nextIndex) if n.IsDir() { e.Dir = true @@ -249,7 +251,6 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { // update etcd index s.CurrentIndex++ - e.Index = s.CurrentIndex s.WatcherHub.notify(e) s.Stats.Inc(DeleteSuccess) @@ -488,7 +489,7 @@ func (s *store) checkDir(parent *Node, dirName string) (*Node, *etcdErr.Error) { return nil, etcdErr.NewError(etcdErr.EcodeNotDir, parent.Path, s.CurrentIndex) } - n := newDir(s, path.Join(parent.Path, dirName), s.CurrentIndex, parent, parent.ACL, Permanent) + n := newDir(s, path.Join(parent.Path, dirName), s.CurrentIndex+1, parent, parent.ACL, Permanent) parent.Children[dirName] = n diff --git a/store/store_test.go b/store/store_test.go index 898d510ab..5ee58649d 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -81,7 +81,7 @@ func TestStoreCreateValue(t *testing.T) { assert.Nil(t, e.KVPairs, "") assert.Nil(t, e.Expiration, "") assert.Equal(t, e.TTL, 0, "") - assert.Equal(t, e.Index, uint64(1), "") + assert.Equal(t, e.ModifiedIndex, uint64(1), "") } // Ensure that the store can create a new directory if it doesn't already exist. @@ -119,7 +119,7 @@ func TestStoreUpdateValue(t *testing.T) { assert.Equal(t, e.PrevValue, "bar", "") assert.Equal(t, e.Value, "baz", "") assert.Equal(t, e.TTL, 0, "") - assert.Equal(t, e.Index, uint64(2), "") + assert.Equal(t, e.ModifiedIndex, uint64(2), "") e, _ = s.Get("/foo", false, false) assert.Equal(t, e.Value, "baz", "") } diff --git a/store/watcher.go b/store/watcher.go index 2015d0072..87b2e155d 100644 --- a/store/watcher.go +++ b/store/watcher.go @@ -24,7 +24,7 @@ func (w *watcher) notify(e *Event, originalPath bool, deleted bool) bool { // at the file we need to delete. // For example a watcher is watching at "/foo/bar". And we deletes "/foo". The watcher // should get notified even if "/foo" is not the path it is watching. - if (w.recursive || originalPath || deleted) && e.Index >= w.sinceIndex { + if (w.recursive || originalPath || deleted) && e.Index() >= w.sinceIndex { w.eventChan <- e return true } From cb4b6f1fe478b35b0bdfbb05d37344d6db137e46 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 20:52:06 -0800 Subject: [PATCH 26/31] feat add modifiedIndex in kvpair --- store/kv_pairs.go | 13 +++++++------ store/node.go | 10 ++++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/store/kv_pairs.go b/store/kv_pairs.go index f2496d363..be6c01fb4 100644 --- a/store/kv_pairs.go +++ b/store/kv_pairs.go @@ -6,12 +6,13 @@ import ( // When user list a directory, we add all the node into key-value pair slice type KeyValuePair struct { - Key string `json:"key, omitempty"` - Value string `json:"value,omitempty"` - Dir bool `json:"dir,omitempty"` - Expiration *time.Time `json:"expiration,omitempty"` - TTL int64 `json:"ttl,omitempty"` // Time to live in second - KVPairs kvPairs `json:"kvs,omitempty"` + Key string `json:"key, omitempty"` + Value string `json:"value,omitempty"` + Dir bool `json:"dir,omitempty"` + Expiration *time.Time `json:"expiration,omitempty"` + TTL int64 `json:"ttl,omitempty"` // Time to live in second + KVPairs kvPairs `json:"kvs,omitempty"` + ModifiedIndex uint64 `json:"modifiedIndex,omitempty"` } type kvPairs []KeyValuePair diff --git a/store/node.go b/store/node.go index a7fd7853c..a4968e1a0 100644 --- a/store/node.go +++ b/store/node.go @@ -226,8 +226,9 @@ func (n *Node) Remove(recursive bool, callback func(path string)) *etcdErr.Error func (n *Node) Pair(recurisive, sorted bool) KeyValuePair { if n.IsDir() { pair := KeyValuePair{ - Key: n.Path, - Dir: true, + Key: n.Path, + Dir: true, + ModifiedIndex: n.ModifiedIndex, } pair.Expiration, pair.TTL = n.ExpirationAndTTL() @@ -263,8 +264,9 @@ func (n *Node) Pair(recurisive, sorted bool) KeyValuePair { } pair := KeyValuePair{ - Key: n.Path, - Value: n.Value, + Key: n.Path, + Value: n.Value, + ModifiedIndex: n.ModifiedIndex, } pair.Expiration, pair.TTL = n.ExpirationAndTTL() return pair From 27157e5e784616197dd966e3e5aefa8e9b38f45b Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 21:17:32 -0800 Subject: [PATCH 27/31] fix tests --- .../multi_node_kill_all_and_recovery_test.go | 5 ++--- tests/functional/simple_snapshot_test.go | 13 +++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/functional/multi_node_kill_all_and_recovery_test.go b/tests/functional/multi_node_kill_all_and_recovery_test.go index 454b9648c..4f3c0e795 100644 --- a/tests/functional/multi_node_kill_all_and_recovery_test.go +++ b/tests/functional/multi_node_kill_all_and_recovery_test.go @@ -65,8 +65,7 @@ func TestMultiNodeKillAllAndRecovery(t *testing.T) { t.Fatalf("Recovery error: %s", err) } - if result.Index != 18 { - t.Fatalf("recovery failed! [%d/18]", result.Index) + if result.Index != 16 { + t.Fatalf("recovery failed! [%d/16]", result.Index) } } - diff --git a/tests/functional/simple_snapshot_test.go b/tests/functional/simple_snapshot_test.go index e7cce08cc..e6f8d7729 100644 --- a/tests/functional/simple_snapshot_test.go +++ b/tests/functional/simple_snapshot_test.go @@ -3,6 +3,7 @@ package test import ( "io/ioutil" "os" + "strconv" "testing" "time" @@ -52,8 +53,10 @@ func TestSimpleSnapshot(t *testing.T) { t.Fatal("wrong number of snapshot :[1/", len(snapshots), "]") } - if snapshots[0].Name() != "0_503.ss" { - t.Fatal("wrong name of snapshot :[0_503.ss/", snapshots[0].Name(), "]") + index, _ := strconv.Atoi(snapshots[0].Name()[2:5]) + + if index < 507 || index > 510 { + t.Fatal("wrong name of snapshot :", snapshots[0].Name()) } // issue second 501 commands @@ -82,7 +85,9 @@ func TestSimpleSnapshot(t *testing.T) { t.Fatal("wrong number of snapshot :[1/", len(snapshots), "]") } - if snapshots[0].Name() != "0_1004.ss" { - t.Fatal("wrong name of snapshot :[0_1004.ss/", snapshots[0].Name(), "]") + index, _ = strconv.Atoi(snapshots[0].Name()[2:6]) + + if index < 1015 || index > 1018 { + t.Fatal("wrong name of snapshot :", snapshots[0].Name()) } } From e427c85f030cbd9054bc5a3c687fc4d5fd0dc64e Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sat, 9 Nov 2013 21:31:17 -0800 Subject: [PATCH 28/31] refactor add debug info to remove_node test --- tests/functional/remove_node_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/functional/remove_node_test.go b/tests/functional/remove_node_test.go index 09fa747aa..a9490cdcc 100644 --- a/tests/functional/remove_node_test.go +++ b/tests/functional/remove_node_test.go @@ -1,6 +1,7 @@ package test import ( + "fmt" "net/http" "os" "testing" @@ -31,6 +32,7 @@ func TestRemoveNode(t *testing.T) { for i := 0; i < 2; i++ { client.Do(rmReq) + fmt.Println("send remove to node3 and wait for its exiting") etcds[2].Wait() resp, err := c.Get("_etcd/machines") @@ -71,6 +73,7 @@ func TestRemoveNode(t *testing.T) { // first kill the node, then remove it, then add it back for i := 0; i < 2; i++ { etcds[2].Kill() + fmt.Println("kill node3 and wait for its exiting") etcds[2].Wait() client.Do(rmReq) From 8b2e1025efecadd2bebbe1c9d8949a5a2411652f Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 11 Nov 2013 21:19:30 -0800 Subject: [PATCH 29/31] style remove the extra space --- store/event.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/store/event.go b/store/event.go index 126ceee4b..f3d607e0b 100644 --- a/store/event.go +++ b/store/event.go @@ -15,8 +15,7 @@ const ( ) type Event struct { - Action string `json:"action"` - + Action string `json:"action"` Key string `json:"key, omitempty"` Dir bool `json:"dir,omitempty"` PrevValue string `json:"prevValue,omitempty"` From fe5fb6cfbabf5ab2a082d00769be9de9331ee2e0 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 11 Nov 2013 21:21:50 -0800 Subject: [PATCH 30/31] style naming the initialization fields for sync command --- store/v2/command_factory.go | 4 +++- store/v2/sync_command.go | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/store/v2/command_factory.go b/store/v2/command_factory.go index 8332891d6..77263b8d7 100644 --- a/store/v2/command_factory.go +++ b/store/v2/command_factory.go @@ -73,5 +73,7 @@ func (f *CommandFactory) CreateCompareAndSwapCommand(key string, value string, p } func (f *CommandFactory) CreateSyncCommand(now time.Time) raft.Command { - return &SyncCommand{time.Now()} + return &SyncCommand{ + Time: time.Now(), + } } diff --git a/store/v2/sync_command.go b/store/v2/sync_command.go index 4b6459079..caf2c37cc 100644 --- a/store/v2/sync_command.go +++ b/store/v2/sync_command.go @@ -21,7 +21,6 @@ func (c SyncCommand) CommandName() string { } func (c SyncCommand) Apply(server raft.Server) (interface{}, error) { - s, _ := server.StateMachine().(store.Store) s.DeleteExpiredKeys(c.Time) From 811c577fe8a117d0f7c9390edc86de785c76d128 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 11 Nov 2013 21:31:12 -0800 Subject: [PATCH 31/31] test stop mockSync goroutines --- store/stats_test.go | 8 ++++++- store/store_test.go | 51 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/store/stats_test.go b/store/stats_test.go index ad16261b8..cc5a4afb5 100644 --- a/store/stats_test.go +++ b/store/stats_test.go @@ -87,7 +87,13 @@ func TestStoreStatsDeleteFail(t *testing.T) { //Ensure that the number of expirations is recorded in the stats. func TestStoreStatsExpireCount(t *testing.T) { s := newStore() - go mockSyncService(s.DeleteExpiredKeys) + + c := make(chan bool) + defer func() { + c <- true + }() + + go mockSyncService(s.DeleteExpiredKeys, c) s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond)) assert.Equal(t, uint64(0), s.Stats.ExpireCount, "") time.Sleep(600 * time.Millisecond) diff --git a/store/store_test.go b/store/store_test.go index 5ee58649d..01b8bb7bb 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -139,7 +139,13 @@ func TestStoreUpdateFailsIfDirectory(t *testing.T) { // Ensure that the store can update the TTL on a value. func TestStoreUpdateValueTTL(t *testing.T) { s := newStore() - go mockSyncService(s.DeleteExpiredKeys) + + c := make(chan bool) + defer func() { + c <- true + }() + go mockSyncService(s.DeleteExpiredKeys, c) + s.Create("/foo", "bar", false, Permanent) _, err := s.Update("/foo", "baz", time.Now().Add(500*time.Millisecond)) e, _ := s.Get("/foo", false, false) @@ -154,7 +160,13 @@ func TestStoreUpdateValueTTL(t *testing.T) { // Ensure that the store can update the TTL on a directory. func TestStoreUpdateDirTTL(t *testing.T) { s := newStore() - go mockSyncService(s.DeleteExpiredKeys) + + c := make(chan bool) + defer func() { + c <- true + }() + go mockSyncService(s.DeleteExpiredKeys, c) + s.Create("/foo", "", false, Permanent) s.Create("/foo/bar", "baz", false, Permanent) _, err := s.Update("/foo", "", time.Now().Add(500*time.Millisecond)) @@ -339,7 +351,13 @@ func TestStoreWatchRecursiveCompareAndSwap(t *testing.T) { // Ensure that the store can watch for key expiration. func TestStoreWatchExpire(t *testing.T) { s := newStore() - go mockSyncService(s.DeleteExpiredKeys) + + stopChan := make(chan bool) + defer func() { + stopChan <- true + }() + go mockSyncService(s.DeleteExpiredKeys, stopChan) + s.Create("/foo", "bar", false, time.Now().Add(500*time.Millisecond)) s.Create("/foofoo", "barbarbar", false, time.Now().Add(500*time.Millisecond)) @@ -379,7 +397,13 @@ func TestStoreRecover(t *testing.T) { // Ensure that the store can recover from a previously saved state that includes an expiring key. func TestStoreRecoverWithExpiration(t *testing.T) { s := newStore() - go mockSyncService(s.DeleteExpiredKeys) + + c := make(chan bool) + defer func() { + c <- true + }() + go mockSyncService(s.DeleteExpiredKeys, c) + s.Create("/foo", "", false, Permanent) s.Create("/foo/x", "bar", false, Permanent) s.Create("/foo/y", "baz", false, time.Now().Add(5*time.Millisecond)) @@ -388,7 +412,13 @@ func TestStoreRecoverWithExpiration(t *testing.T) { time.Sleep(10 * time.Millisecond) s2 := newStore() - go mockSyncService(s2.DeleteExpiredKeys) + + c2 := make(chan bool) + defer func() { + c2 <- true + }() + go mockSyncService(s2.DeleteExpiredKeys, c2) + s2.Recovery(b) time.Sleep(600 * time.Millisecond) @@ -412,9 +442,14 @@ func nbselect(c <-chan *Event) *Event { } } -func mockSyncService(f func(now time.Time)) { +func mockSyncService(f func(now time.Time), c chan bool) { ticker := time.Tick(time.Millisecond * 500) - for now := range ticker { - f(now) + for { + select { + case <-c: + return + case now := <-ticker: + f(now) + } } }