kevin-wu24 commented on code in PR #20859:
URL: https://github.com/apache/kafka/pull/20859#discussion_r2561465070
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -2335,8 +2344,11 @@ private boolean handleAddVoterResponse(
/* These error codes indicate the replica was successfully added or
the leader is unable to
* process the request. In either case, reset the update voter set
timer to back off.
*/
- if (error == Errors.NONE || error == Errors.REQUEST_TIMED_OUT ||
- error == Errors.DUPLICATE_VOTER) {
+ if (error == Errors.NONE) {
+
quorum.followerStateOrThrow().resetUpdateVoterSetPeriod(currentTimeMs);
+ hasAutoJoined = true;
Review Comment:
Take a look at this code from `AddVoterHandler#handleApiVersionsResponse`:
```
current.setLastOffset(leaderState.appendVotersRecord(newVoters,
currentTimeMs));
if (!current.ackWhenCommitted()) { ***we execute the below when
auto-join is enabled***
// complete the future to send response, but do not reset the
state,
// since the new voter set is not yet committed
current.future().complete(RaftUtil.addVoterResponse(Errors.NONE,
null));
}
```
When auto-join is enabled, we send the RPC response BEFORE the new voters
record is committed. The motivation behind this is detailed in:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1186%3A+Update+AddRaftVoterRequest+RPC+to+support+auto-join,
and is necessary for a fully available auto-join feature.
What this means here is that our local node may not actually have
auto-joined, but we set `hasAutoJoined = true`. For example, what if the remote
leader fails over before the VotersRecord adding the local node is committed?
The local node has not added itself via auto-join, and will not until it
restarts, despite the user expecting it too.
The "more correct" place to set `hasAutoJoined = true` is in `pollFollower`
if we are in the `quorum.isVoter()` case AND `quorumConfig.autoJoin() == true`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]