GlenGeng commented on a change in pull request #144:
URL: https://github.com/apache/incubator-ratis/pull/144#discussion_r466803280
##########
File path:
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -785,7 +785,7 @@ public RaftClientReply
setConfiguration(SetConfigurationRequest request) throws
}
private boolean shouldWithholdVotes(long candidateTerm) {
- if (state.getCurrentTerm() < candidateTerm) {
+ if (state.getCurrentTerm() >= candidateTerm) {
Review comment:
It does obey the paper.
> 4.2.3 Disruptive servers
> Without additional mechanism, servers not in Cnew can disrupt the cluster.
Once the cluster leader has created the Cnew entry, a server that is not in
Cnew will no longer receive heartbeats, so it will time out and start new
elections. Furthermore, it will not receive the Cnew entry or learn of that
entry’s commitment, so it will not know that it has been removed from the
cluster. The server will send RequestVote RPCs with new term numbers, and this
will cause the current leader to revert to follower state. A new leader from
Cnew will eventually be elected, but the disruptive server will time out again
and the process will repeat, resulting in poor availability. If multiple
servers have been removed from the cluster, the situation could degrade further.
> Because of this scenario, we now believe that no solution based on
comparing logs alone (such as the Pre-Vote check) will be sufficient to tell if
an election will be disruptive.
> Raft’s solution uses heartbeats to determine when a valid leader exists.
In Raft, a leader is
considered active if it is able to maintain heartbeats to its followers
(otherwise, another server will start an election). Thus, servers should not be
able to disrupt a leader whose cluster is receiving heartbeats. We modify the
RequestVote RPC to achieve this: 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. It can either drop the request, reply
with a vote denial, or delay the request; the result is essentially the same.
This does not affect normal elections, where each server waits at least a
minimum election timeout before starting an election. However, it helps avoid
disruptions from servers not in Cnew: while a leader is able to get heartbeats
to its cluster, it will not be deposed by larger term numbers.
BTW, `shouldWithholdVotes` is redundant now, the cases handled by
`shouldWithholdVotes()` can also handled by `recognizeCandidate()`. if
currentTerm is larger than candidateTerm, just reject the RequestVote request,
no further handling is needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
