From 5533c3058a1967232b6a71e6581e67300af9ba9a Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Tue, 2 May 2017 12:07:49 -0700 Subject: [PATCH 1/2] etcdserver: apply() sets consistIndex for any entry type previously, apply() doesn't set consistIndex for EntryConfChange type. this causes a misalignment between consistIndex and applied index where EntryConfChange entry results setting applied index but not consistIndex. suppose that addMember() is called and leader reflects that change. 1. applied index and consistIndex is now misaligned. 2. a new follower node joined. 3. leader sends the snapshot to follower where the applied index is the snapshot metadata index. 4. follower node saves the snapshot and database(includes consistIndex) from leader. 5. restarting follower loads snapshot and database. 6. follower checks snapshot metadata index(same as applied index) and database consistIndex, finds them don't match, and then panic. FIXES #7834 --- etcdserver/server.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/etcdserver/server.go b/etcdserver/server.go index f5fc36952fa..33430276bff 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -1272,9 +1272,14 @@ func (s *EtcdServer) apply(es []raftpb.Entry, confState *raftpb.ConfState) (appl case raftpb.EntryNormal: s.applyEntryNormal(&e) case raftpb.EntryConfChange: + // set the consistent index of current executing entry + if e.Index > s.consistIndex.ConsistentIndex() { + s.consistIndex.setConsistentIndex(e.Index) + } var cc raftpb.ConfChange pbutil.MustUnmarshal(&cc, e.Data) removedSelf, err := s.applyConfChange(cc, confState) + s.setAppliedIndex(e.Index) shouldStop = shouldStop || removedSelf s.w.Trigger(cc.ID, &confChangeResponse{s.cluster.Members(), err}) default: From e33b10a666bf77545d800bd7d8f365e5069b4543 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Tue, 2 May 2017 12:25:29 -0700 Subject: [PATCH 2/2] etcdserver: add a test to ensure config change also update ConsistIndex --- etcdserver/server_test.go | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index f8bbb0dc449..55f32d5229c 100644 --- a/etcdserver/server_test.go +++ b/etcdserver/server_test.go @@ -585,6 +585,49 @@ func TestApplyConfChangeShouldStop(t *testing.T) { } } +// TestApplyConfigChangeUpdatesConsistIndex ensures a config change also updates the consistIndex +// where consistIndex equals to applied index. +func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) { + cl := membership.NewCluster("") + cl.SetStore(store.New()) + cl.AddMember(&membership.Member{ID: types.ID(1)}) + r := newRaftNode(raftNodeConfig{ + Node: newNodeNop(), + transport: rafthttp.NewNopTransporter(), + }) + srv := &EtcdServer{ + id: 1, + r: *r, + cluster: cl, + w: wait.New(), + } + + // create EntryConfChange entry + now := time.Now() + urls, err := types.NewURLs([]string{"http://whatever:123"}) + if err != nil { + t.Fatal(err) + } + m := membership.NewMember("", urls, "", &now) + m.ID = types.ID(2) + b, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + cc := &raftpb.ConfChange{Type: raftpb.ConfChangeAddNode, NodeID: 2, Context: b} + ents := []raftpb.Entry{{ + Index: 2, + Type: raftpb.EntryConfChange, + Data: pbutil.MustMarshal(cc), + }} + + _, appliedi, _ := srv.apply(ents, &raftpb.ConfState{}) + consistIndex := srv.consistIndex.ConsistentIndex() + if consistIndex != appliedi { + t.Fatalf("consistIndex = %v, want %v", consistIndex, appliedi) + } +} + // TestApplyMultiConfChangeShouldStop ensures that apply will return shouldStop // if the local member is removed along with other conf updates. func TestApplyMultiConfChangeShouldStop(t *testing.T) {