diff --git a/raft.go b/raft.go index 94c2363d..56f53139 100644 --- a/raft.go +++ b/raft.go @@ -1298,6 +1298,10 @@ func stepLeader(r *raft, m pb.Message) error { return ErrProposalDropped } + // Track if any ConfChange was rejected so we can return an error + // while still appending the noop to maintain log consistency. + var confChangeRejected bool + for i := range m.Entries { e := &m.Entries[i] var cc pb.ConfChangeI @@ -1330,7 +1334,10 @@ func stepLeader(r *raft, m pb.Message) error { if failedCheck != "" && !r.disableConfChangeValidation { r.logger.Infof("%x ignoring conf change %v at config %s: %s", r.id, cc, r.trk.Config, failedCheck) + // Convert to noop to maintain log consistency, but mark as rejected + // so we can return an error to notify the caller. m.Entries[i] = pb.Entry{Type: pb.EntryNormal} + confChangeRejected = true } else { r.pendingConfIndex = r.raftLog.lastIndex() + uint64(i) + 1 traceChangeConfEvent(cc, r) @@ -1342,6 +1349,11 @@ func stepLeader(r *raft, m pb.Message) error { return ErrProposalDropped } r.bcastAppend() + // Return error if any ConfChange was rejected, so the caller knows + // the config change didn't take effect (even though a noop was appended). + if confChangeRejected { + return ErrProposalDropped + } return nil case pb.MsgReadIndex: // only one voting member (the leader) in the cluster diff --git a/raft_test.go b/raft_test.go index 85bbcce7..675e3c36 100644 --- a/raft_test.go +++ b/raft_test.go @@ -2687,7 +2687,7 @@ func TestStepConfig(t *testing.T) { // TestStepIgnoreConfig tests that if raft step the second msgProp in // EntryConfChange type when the first one is uncommitted, the node will set -// the proposal to noop and keep its original state. +// the proposal to noop, keep its original state, and return an error. func TestStepIgnoreConfig(t *testing.T) { // a raft that cannot make progress r := newTestRaft(1, 10, 1, newTestMemoryStorage(withPeers(1, 2))) @@ -2696,7 +2696,10 @@ func TestStepIgnoreConfig(t *testing.T) { r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Type: pb.EntryConfChange}}}) index := r.raftLog.lastIndex() pendingConfIndex := r.pendingConfIndex - r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Type: pb.EntryConfChange}}}) + // Second ConfChange should be converted to noop but return an error + err := r.Step(pb.Message{From: 1, To: 1, Type: pb.MsgProp, Entries: []pb.Entry{{Type: pb.EntryConfChange}}}) + assert.Equal(t, ErrProposalDropped, err) + // A noop entry should still be appended (for log consistency) wents := []pb.Entry{{Type: pb.EntryNormal, Term: 1, Index: 3, Data: nil}} ents, err := r.raftLog.entries(index+1, noLimit) require.NoError(t, err) diff --git a/testdata/confchange_v2_add_single_explicit.txt b/testdata/confchange_v2_add_single_explicit.txt index 1d390e55..01a11ba1 100644 --- a/testdata/confchange_v2_add_single_explicit.txt +++ b/testdata/confchange_v2_add_single_explicit.txt @@ -103,6 +103,7 @@ propose-conf-change 1 v3 v4 v5 ---- INFO 1 ignoring conf change {ConfChangeTransitionAuto [{ConfChangeAddNode 3} {ConfChangeAddNode 4} {ConfChangeAddNode 5}] []} at config voters=(1 2)&&(1): must transition out of joint config first +raft proposal dropped # Propose a transition out of the joint config. We'll see this at index 6 below. propose-conf-change 1 @@ -165,6 +166,7 @@ stabilize propose-conf-change 1 ---- INFO 1 ignoring conf change {ConfChangeTransitionAuto [] []} at config voters=(1 2): not in joint state; refusing empty conf change +raft proposal dropped # Finishes work for the empty entry we just proposed. stabilize