vamossagar12 commented on a change in pull request #9539:
URL: https://github.com/apache/kafka/pull/9539#discussion_r516438813
##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -317,6 +317,9 @@ private void appendLeaderChangeMessage(LeaderState state,
long currentTimeMs) {
.map(follower -> new Voter().setVoterId(follower))
.collect(Collectors.toList());
+ // Adding the leader to the voters as the protocol ensures that leader
always votes for itself.
+ voters.add(new Voter().setVoterId(state.election().leaderId()));
Review comment:
Thanks @hachikuji . So, from the context of this PR, do you suggest to
continue using the definition of Voters in the LeaderChange message as all the
voters ? Maybe we can create separate issues for:
1) changing LeaderChange message to include both all voters and endorsing
voters for that Leader.
2) Make relevant changes to be able to pass the grantingVoters information
from CandidateState to LeaderState. These include the things you mentioned like
removing `onBecomeLeader` from `initialize()` or throw exception.
Just curious on the last part. As per KAFKA-10527, a node can't be
initialized as a leader. So, this block of code is effectively unused then:
`if (quorum.isLeader()) {
onBecomeLeader(currentTimeMs);
} else if (quorum.isCandidate()) {
onBecomeCandidate(currentTimeMs);
} `
Let me know how should I proceed here...
##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -317,6 +317,9 @@ private void appendLeaderChangeMessage(LeaderState state,
long currentTimeMs) {
.map(follower -> new Voter().setVoterId(follower))
.collect(Collectors.toList());
+ // Adding the leader to the voters as the protocol ensures that leader
always votes for itself.
+ voters.add(new Voter().setVoterId(state.election().leaderId()));
Review comment:
Thanks @hachikuji . So, from the context of this PR, do you suggest to
continue using the definition of Voters in the LeaderChange message as all the
voters ? Maybe we can create separate issues for:
1) changing LeaderChange message to include both all voters and endorsing
voters for that Leader.
2) Make relevant changes to be able to pass the grantingVoters information
from CandidateState to LeaderState. These include the things you mentioned like
removing `onBecomeLeader` from `initialize()` or throw exception.
Just curious on the last part. As per KAFKA-10527, a node can't be
initialized as a leader. So, this block of code is effectively unused then:
`if (quorum.isLeader()) {
onBecomeLeader(currentTimeMs);
} else if (quorum.isCandidate()) {
onBecomeCandidate(currentTimeMs);
} `
Or should we chase all the above as part of this PR itself? Plz let me know.
----------------------------------------------------------------
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]
