From 4f20e01f60e4364748d88edfd5cf5bd474c7fa48 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sat, 29 Aug 2015 11:09:56 +0200 Subject: [PATCH] raft: Ignore proposals if not a current member. Fixes another panic in MultiNode.Propose. --- raft/multinode_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ raft/raft.go | 6 ++++++ 2 files changed, 54 insertions(+) diff --git a/raft/multinode_test.go b/raft/multinode_test.go index e53a9fc0e..dd343d170 100644 --- a/raft/multinode_test.go +++ b/raft/multinode_test.go @@ -227,6 +227,54 @@ func TestProposeUnknownGroup(t *testing.T) { } } +// TestProposeAfterRemoveLeader ensures that we gracefully handle +// proposals that are attempted after a leader has been removed from +// the active configuration, but before that leader has called +// MultiNode.RemoveGroup. +func TestProposeAfterRemoveLeader(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mn := newMultiNode(1) + go mn.run() + defer mn.Stop() + + storage := NewMemoryStorage() + if err := mn.CreateGroup(1, newTestConfig(1, nil, 10, 1, storage), + []Peer{{ID: 1}}); err != nil { + t.Fatal(err) + } + if err := mn.Campaign(ctx, 1); err != nil { + t.Fatal(err) + } + + if err := mn.ProposeConfChange(ctx, 1, raftpb.ConfChange{ + Type: raftpb.ConfChangeRemoveNode, + NodeID: 1, + }); err != nil { + t.Fatal(err) + } + gs := <-mn.Ready() + g := gs[1] + if err := storage.Append(g.Entries); err != nil { + t.Fatal(err) + } + for _, e := range g.CommittedEntries { + if e.Type == raftpb.EntryConfChange { + var cc raftpb.ConfChange + if err := cc.Unmarshal(e.Data); err != nil { + t.Fatal(err) + } + mn.ApplyConfChange(1, cc) + } + } + mn.Advance(gs) + + if err := mn.Propose(ctx, 1, []byte("somedata")); err != nil { + t.Errorf("err = %v, want nil", err) + } +} + // TestNodeTick from node_test.go has no equivalent in multiNode because // it reaches into the raft object which is not exposed. diff --git a/raft/raft.go b/raft/raft.go index 1108604b9..281ea0108 100644 --- a/raft/raft.go +++ b/raft/raft.go @@ -522,6 +522,12 @@ func stepLeader(r *raft, m pb.Message) { if len(m.Entries) == 0 { r.logger.Panicf("%x stepped empty MsgProp", r.id) } + if _, ok := r.prs[r.id]; !ok { + // If we are not currently a member of the range (i.e. this node + // was removed from the configuration while serving as leader), + // drop any new proposals. + return + } for i, e := range m.Entries { if e.Type == pb.EntryConfChange { if r.pendingConf {