Browse Source

raft: Ignore proposals if not a current member.

Fixes another panic in MultiNode.Propose.
Ben Darnell 10 years ago
parent
commit
4f20e01f60
2 changed files with 54 additions and 0 deletions
  1. 48 0
      raft/multinode_test.go
  2. 6 0
      raft/raft.go

+ 48 - 0
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.
 

+ 6 - 0
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 {