kevin-wu24 commented on code in PR #19589:
URL: https://github.com/apache/kafka/pull/19589#discussion_r2067411018
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -2280,6 +2280,25 @@ private boolean handleApiVersionsResponse(
);
}
+ private boolean handleAddVoterResponse(
+ RaftResponse.Inbound responseMetadata,
+ long currentTimeMs
+ ) {
+ RemoveRaftVoterResponseData data = (RemoveRaftVoterResponseData)
responseMetadata.data();
+
+ Errors error = Errors.forCode(data.errorCode());
+
+ if (error == Errors.NONE || error == Errors.UNSUPPORTED_VERSION ||
error == Errors.DUPLICATE_VOTER) {
+ return true;
+ } else if (error == Errors.REQUEST_TIMED_OUT) {
+ FollowerState follower = quorum.followerStateOrThrow();
+ follower.resetAddRemoveVoterPeriod(currentTimeMs);
+ return true;
Review Comment:
Does the new controller that receives an `AddVoterResponse` here need to do
anything special here? I don't think we can ever be in the
`UNSUPPORTED_VERSION` case actually, since we cannot downgrade `kraft.version`
AFAIK. If the new controller gets the `DUPLICATE_VOTER` error, it should mean a
previous request succeeded, and before the new controller has fetched the
updated voter set with itself from the leader, it sent another
`AddVoterRequest` because the timer expired.
My rationale for the L2293 `else if` block is that when we get
`REQUEST_TIMED_OUT`, we should back off to give the active controller a change
to finish its current add/remove RPC, or commit its epoch.
This comment applies to `handleRemoveVoterResponse` below as well.
--
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
