Merge pull request #6866 from absolute8511/master

raft: add node should reset the pendingConf state

raft/node_test.go

@@ -291,6 +291,71 @@ func TestNodeProposeConfig(t *testing.T) {
 	}
 }
 
+// TestNodeProposeAddDuplicateNode ensures that two proposes to add the same node should
+// not affect the later propose to add new node.
+func TestNodeProposeAddDuplicateNode(t *testing.T) {
+	n := newNode()
+	s := NewMemoryStorage()
+	r := newTestRaft(1, []uint64{1}, 10, 1, s)
+	go n.run(r)
+	n.Campaign(context.TODO())
+	rdyEntries := make([]raftpb.Entry, 0)
+	ticker := time.NewTicker(time.Millisecond * 100)
+	done := make(chan struct{})
+	stop := make(chan struct{})
+	applyChan := make(chan struct{})
+	go func() {
+		defer close(done)
+		for {
+			select {
+			case <-stop:
+				return
+			case <-ticker.C:
+				n.Tick()
+			case rd := <-n.Ready():
+				s.Append(rd.Entries)
+				for _, e := range rd.Entries {
+					rdyEntries = append(rdyEntries, e)
+					switch e.Type {
+					case raftpb.EntryNormal:
+					case raftpb.EntryConfChange:
+						var cc raftpb.ConfChange
+						cc.Unmarshal(e.Data)
+						n.ApplyConfChange(cc)
+					}
+				}
+				n.Advance()
+				applyChan <- struct{}{}
+			}
+		}
+	}()
+	cc1 := raftpb.ConfChange{Type: raftpb.ConfChangeAddNode, NodeID: 1}
+	ccdata1, _ := cc1.Marshal()
+	n.ProposeConfChange(context.TODO(), cc1)
+	<-applyChan
+	// try add the same node again
+	n.ProposeConfChange(context.TODO(), cc1)
+	<-applyChan
+	// the new node join should be ok
+	cc2 := raftpb.ConfChange{Type: raftpb.ConfChangeAddNode, NodeID: 2}
+	ccdata2, _ := cc2.Marshal()
+	n.ProposeConfChange(context.TODO(), cc2)
+	<-applyChan
+	close(stop)
+	<-done
+
+	if len(rdyEntries) != 4 {
+		t.Errorf("len(entry) = %d, want %d, %v\n", len(rdyEntries), 3, rdyEntries)
+	}
+	if !bytes.Equal(rdyEntries[1].Data, ccdata1) {
+		t.Errorf("data = %v, want %v", rdyEntries[1].Data, ccdata1)
+	}
+	if !bytes.Equal(rdyEntries[3].Data, ccdata2) {
+		t.Errorf("data = %v, want %v", rdyEntries[3].Data, ccdata2)
+	}
+	n.Stop()
+}
+
 // TestBlockProposal ensures that node will block proposal when it does not
 // know who is the current leader; node will accept proposal when it knows
 // who is the current leader.

raft/raft.go

@@ -813,6 +813,7 @@ func stepLeader(r *raft, m pb.Message) {
 		for i, e := range m.Entries {
 			if e.Type == pb.EntryConfChange {
 				if r.pendingConf {
+					r.logger.Infof("propose conf %s ignored since pending unapplied configuration", e.String())
 					m.Entries[i] = pb.Entry{Type: pb.EntryNormal}
 				}
 				r.pendingConf = true
@@ -1145,6 +1146,7 @@ func (r *raft) promotable() bool {
 }
 
 func (r *raft) addNode(id uint64) {
+	r.pendingConf = false
 	if _, ok := r.prs[id]; ok {
 		// Ignore any redundant addNode calls (which can happen because the
 		// initial bootstrapping entries are applied twice).
@@ -1152,7 +1154,6 @@ func (r *raft) addNode(id uint64) {
 	}
 
 	r.setProgress(id, 0, r.raftLog.lastIndex()+1)
-	r.pendingConf = false
 }
 
 func (r *raft) removeNode(id uint64) {