From 3d91faf85a81b369f219ca9694491bc44dde7627 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Fri, 5 Dec 2014 15:35:39 -0500 Subject: [PATCH 1/2] Pre-apply the bootstrapping ConfChange entries. This eliminates the need to fake an ApplyConfChange call before Campaign in tests. Fixes #1856. --- raft/node.go | 12 ++++++++++++ raft/node_test.go | 2 -- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/raft/node.go b/raft/node.go index 83f788a44..b0d0ab6a5 100644 --- a/raft/node.go +++ b/raft/node.go @@ -150,6 +150,18 @@ func StartNode(id uint64, peers []Peer, election, heartbeat int, storage Storage // TODO(bdarnell): These entries are still unstable; do we need to preserve // the invariant that committed < unstable? r.raftLog.committed = r.raftLog.lastIndex() + // Now apply them, mainly so that the application can call Campaign + // immediately after StartNode in tests. Note that these nodes will + // be added to raft twice: here and when the application's Ready + // loop calls ApplyConfChange. The calls to addNode must come after + // all calls to raftLog.append so progress.next is set after these + // bootstrapping entries (it is an error if we try to append these + // entries since they have already been committed). + // We do not set raftLog.applied so the application will be able + // to observe all conf changes via Ready.CommittedEntries. + for _, peer := range peers { + r.addNode(peer.ID) + } go n.run(r) return &n diff --git a/raft/node_test.go b/raft/node_test.go index 52fcfa15e..1617dc110 100644 --- a/raft/node_test.go +++ b/raft/node_test.go @@ -324,7 +324,6 @@ func TestNodeStart(t *testing.T) { } storage := NewMemoryStorage() n := StartNode(1, []Peer{{ID: 1}}, 10, 1, storage) - n.ApplyConfChange(cc) n.Campaign(ctx) g := <-n.Ready() if !reflect.DeepEqual(g, wants[0]) { @@ -421,7 +420,6 @@ func TestNodeAdvance(t *testing.T) { storage := NewMemoryStorage() n := StartNode(1, []Peer{{ID: 1}}, 10, 1, storage) - n.ApplyConfChange(raftpb.ConfChange{Type: raftpb.ConfChangeAddNode, NodeID: 1}) n.Campaign(ctx) <-n.Ready() n.Propose(ctx, []byte("foo")) From ea4d645a83b70979f1892199be1b4a5d6dd1f7fa Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Fri, 5 Dec 2014 17:15:50 -0500 Subject: [PATCH 2/2] raft: Ignore redundant addNode calls. This avoids clobbering any state when bootstrapping entries are applied twice. --- raft/raft.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/raft/raft.go b/raft/raft.go index a6e93ece8..8531df47b 100644 --- a/raft/raft.go +++ b/raft/raft.go @@ -472,6 +472,12 @@ func (r *raft) handleSnapshot(m pb.Message) { func (r *raft) resetPendingConf() { r.pendingConf = false } func (r *raft) addNode(id uint64) { + if _, ok := r.prs[id]; ok { + // Ignore any redundant addNode calls (which can happen because the + // initial bootstrapping entries are applied twice). + return + } + r.setProgress(id, 0, r.raftLog.lastIndex()+1) r.pendingConf = false }