ahuang98 commented on code in PR #17807: URL: https://github.com/apache/kafka/pull/17807#discussion_r1879049273
########## raft/src/main/java/org/apache/kafka/raft/EpochState.java: ########## @@ -26,16 +26,18 @@ default Optional<LogOffsetMetadata> highWatermark() { } /** - * Decide whether to grant a vote to a candidate. + * Decide whether to grant a vote to a replica. * * It is the responsibility of the caller to invoke * {@link QuorumState#transitionToUnattachedVotedState(int, ReplicaKey)} if vote is granted. * - * @param candidateKey the id and directory of the candidate - * @param isLogUpToDate whether the candidate’s log is at least as up-to-date as receiver’s log + * @param replicaKey the id and directory of the replica requesting the vote + * @param isLogUpToDate whether the replica's log is at least as up-to-date as receiver’s log * @return true if it can grant the vote, false otherwise */ - boolean canGrantVote(ReplicaKey candidateKey, boolean isLogUpToDate); + boolean canGrantVote(ReplicaKey replicaKey, boolean isLogUpToDate); + + boolean canGrantPreVote(ReplicaKey replicaKey, boolean isLogUpToDate); Review Comment: yes, originally it was just the one method since the logic for granting preVotes used to be (incorrectly) very similar to granting standard votes. I decided to split it out since I was seeing the cyclomatic complexity of certain states getting worse (e.g. Follower, Unattached state). I also considered having just one overridden EpochState method `canGrantVote()` and then having that call a private helper `canGrantPreVote`, but that just seemed like an extra hoop for the sake of not creating an extra method for the EpochState interface -- 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. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org