[GitHub] [kafka] hachikuji commented on a change in pull request #9539: KAFKA-10634: Adding LeaderId to Voters list in LeaderChangeMessage

[GitBox <gi...@apache.org>]

hachikuji commented on a change in pull request #9539:
URL: https://github.com/apache/kafka/pull/9539#discussion_r533593868



##########
File path: raft/src/main/java/org/apache/kafka/raft/LeaderState.java
##########
@@ -72,11 +73,20 @@ public int epoch() {
         return voterReplicaStates.keySet().stream().filter(id -> id != localId).collect(Collectors.toSet());
     }
 
+    public Set<Integer> endorsingVoters() {

Review comment:
       I think the terminology may be causing some confusing here. The "granting" voters are those who have voted for the leader and which the leader is using for the basis of its election. The "endorsing" voters are those who have acknowledged the leader. The set of endorsing voters changes over time as the voters convert to followers and begin fetching. We use `nonEndorsingVoters` to know which voters still need to be sent `BeginQuorumEpoch`. Note that it is possible for a voter to have granted a vote, but not yet endorsed the election (because it does not know the vote succeeded). 
   
   We should not conflate these sets, but maybe we can come up with better terms to avoid the confusion. I would stick with "granting" voters as the set of voters who granted the leader's candidacy. However, maybe we can change "endorsing" to "acknowledged"? These are the voters who have acknowledged the election result. What do you think?

##########
File path: clients/src/main/resources/common/message/LeaderChangeMessage.json
##########
@@ -22,7 +22,11 @@
     {"name": "LeaderId", "type": "int32", "versions": "0+",
       "about": "The ID of the newly elected leader"},
     {"name": "Voters", "type": "[]Voter", "versions": "0+",
-      "about": "The voters who voted for the current leader"}
+      "about": "The set of voters in the quorum for this epoch"},
+    {"name": "GrantingVoters", "type": "[]Voter", "versions": "0+",
+      "about": "The voters who voted for the leader at the time of election"},
+    {"name": "LeaderEpoch", "type": "int32", "versions": "0+",

Review comment:
       The reason we didn't have this is because the batch header already has a field for the leader epoch.




----------------------------------------------------------------
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:
users@infra.apache.org