Skip to content

Commit

Permalink
raft: Check promotable() in MsgTimeoutNow handling
Browse files Browse the repository at this point in the history
If MsgTimeoutNow arrived after a node was removed, the node could start
and win an election, then panic in becomeLeader (see
cockroachdb/cockroach#8535)
  • Loading branch information
bdarnell committed Nov 7, 2016
1 parent ecd4803 commit 2f34547
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
14 changes: 9 additions & 5 deletions raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -1050,11 +1050,15 @@ func stepFollower(r *raft, m pb.Message) {
m.To = r.lead
r.send(m)
case pb.MsgTimeoutNow:
r.logger.Infof("%x [term %d] received MsgTimeoutNow from %x and starts an election to get leadership.", r.id, r.Term, m.From)
// Leadership transfers never use pre-vote even if r.preVote is true; we
// know we are not recovering from a partition so there is no need for the
// extra round trip.
r.campaign(campaignTransfer)
if r.promotable() {
r.logger.Infof("%x [term %d] received MsgTimeoutNow from %x and starts an election to get leadership.", r.id, r.Term, m.From)
// Leadership transfers never use pre-vote even if r.preVote is true; we
// know we are not recovering from a partition so there is no need for the
// extra round trip.
r.campaign(campaignTransfer)
} else {
r.logger.Infof("%x received MsgTimeoutNow from %x but is not promotable", r.id, m.From)
}
case pb.MsgReadIndex:
if r.lead == None {
r.logger.Infof("%x no leader at term %d; dropping index reading msg", r.id, r.Term)
Expand Down
15 changes: 15 additions & 0 deletions raft/raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2835,6 +2835,21 @@ func checkLeaderTransferState(t *testing.T, r *raft, state StateType, lead uint6
}
}

// TestTransferNonMember verifies that when a MsgTimeoutNow arrives at
// a node that has been removed from the group, nothing happens.
// (previously, if the node also got votes, it would panic as it
// transitioned to StateLeader)
func TestTransferNonMember(t *testing.T) {
r := newTestRaft(1, []uint64{2, 3, 4}, 5, 1, NewMemoryStorage())
r.Step(pb.Message{From: 2, To: 1, Type: pb.MsgTimeoutNow})

r.Step(pb.Message{From: 2, To: 1, Type: pb.MsgVoteResp})
r.Step(pb.Message{From: 3, To: 1, Type: pb.MsgVoteResp})
if r.state != StateFollower {
t.Fatalf("state is %s, want StateFollower", r.state)
}
}

func ents(terms ...uint64) *raft {
storage := NewMemoryStorage()
for i, term := range terms {
Expand Down

0 comments on commit 2f34547

Please sign in to comment.