Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions testdata/confchange_v2_add_single_explicit.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down