No leader using prevote feature

[flike]

We use Etcd raft lib in our system, and we not change the etcd raft lib code.
Today I test a scenario with prevote,the cluster will no leader.

The follow test case will be failed

func TestPreVoteWithLeaderNetworkPartition(t *testing.T) {
	n1 := newTestRaft(1, []uint64{1, 2, 3}, 10, 1, NewMemoryStorage())
	n2 := newTestRaft(2, []uint64{1, 2, 3}, 10, 1, NewMemoryStorage())
	n3 := newTestRaft(3, []uint64{1, 2, 3}, 10, 1, NewMemoryStorage())

	n1.becomeFollower(1, None)
	n2.becomeFollower(1, None)
	n3.becomeFollower(1, None)

	n1.preVote = true
	n2.preVote = true
	n3.preVote = true

	n1.checkQuorum = true
	n2.checkQuorum = true
	n3.checkQuorum = true

	nt := newNetwork(n1, n2, n3)
	nt.send(pb.Message{From: 1, To: 1, Type: pb.MsgHup})

	sm := nt.peers[1].(*raft)
	if sm.state != StateLeader {
		t.Errorf("peer 2 state: %s, want %s", sm.state, StateFollower)
	}

	// simulate leader down. followers start split vote.
	nt.isolate(1)
	sm = nt.peers[2].(*raft)
	if sm.state != StateFollower {
		t.Errorf("peer 2 state: %s, want %s", sm.state, StateFollower)
	}

	nt.send(pb.Message{From: 2, To: 2, Type: pb.MsgHup})
	nt.send(pb.Message{From: 3, To: 3, Type: pb.MsgHup})
	sm = nt.peers[3].(*raft)
	if sm.state != StateLeader {
		t.Errorf("peer 2 state: %s, want %s", sm.state, StateLeader)
	}
}

I think in becomePreCandidate function should set r.lead to None

func (r *raft) becomePreCandidate() {
	// TODO(xiangli) remove the panic when the raft implementation is stable
	if r.state == StateLeader {
		panic("invalid transition [leader -> pre-candidate]")
	}
	// Becoming a pre-candidate changes our step functions and state,
	// but doesn't change anything else. In particular it does not increase
	// r.Term or change r.Vote.
	r.step = stepCandidate
	r.votes = make(map[uint64]bool)
+       r.lead = None
	r.tick = r.tickElection
	r.state = StatePreCandidate
	r.logger.Infof("%x became pre-candidate at term %d", r.id, r.Term)
}

and then in Step() function, r.lead is set by becomePreCandidate function to None, inLease will be false. Otherwise, inLease will be true high possibility, the node will ignore MsgPreVote, and result in no node will get the quorum grant votes.

func (r *raft) Step(m pb.Message) error {
	// Handle the message term, which may result in our stepping down to a follower.
	switch {
	case m.Term == 0:
		// local message
	case m.Term > r.Term:
		if m.Type == pb.MsgVote || m.Type == pb.MsgPreVote {
			force := bytes.Equal(m.Context, []byte(campaignTransfer))
			//!!! r.lead is None, inLease will be false
			//othersize, inLease will be true high possibility
			inLease := r.checkQuorum && r.lead != None && r.electionElapsed < r.electionTimeout
			if !force && inLease {
				// If a server receives a RequestVote request within the minimum election timeout
				// of hearing from a current leader, it does not update its term or grant its vote
				r.logger.Infof("%x [logterm: %d, index: %d, vote: %x] ignored %s from %x [logterm: %d, index: %d] at term %d: lease is not expired (remaining ticks: %d)",
					r.id, r.raftLog.lastTerm(), r.raftLog.lastIndex(), r.Vote, m.Type, m.From, m.LogTerm, m.Index, r.Term, r.electionTimeout-r.electionElapsed)
				return nil
			}
		}

[gyuho]

I think this makes sense, especially when there are two pre-candidates from leader isolation. And leader steps down anyway when becomePreCandidate gets called. Defer to @bdarnell @xiang90

[bdarnell]

In the real world, unless you're doing something tricky to manually trigger campaigns, time will pass before the two followers become pre-candidates. If you call tick() before sending MsgHup to the two followers, then r.electionElapsed will increase and then inLease will be false.

It might make sense to set r.lead = None in becomePreCandidate, but I don't think it's the right solution to this problem. The two followers will become pre-candidates at slightly different times. This means that the first node's MsgPreVote will still get dropped, and the second one will win. This seems odd. Whether one node grants a vote should be independent of whether it has transitioned to pre-candidate state itself.

[bdarnell]

There is an open PR with this change: #8334.

In #8334 (review) I suggested that part of CheckQuorum is redundant when PreVote is enabled and we may want to disable the inLease check when both are enabled. (In CockroachDB, we use PreVote without CheckQuorum)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.