kevin-wu24 commented on code in PR #19589:
URL: https://github.com/apache/kafka/pull/19589#discussion_r2270508416
##########
raft/src/main/java/org/apache/kafka/raft/RaftUtil.java:
##########
@@ -524,14 +526,16 @@ public static AddRaftVoterRequestData addVoterRequest(
String clusterId,
int timeoutMs,
ReplicaKey voter,
- Endpoints listeners
+ Endpoints listeners,
+ boolean ackWhenCommitted
) {
return new AddRaftVoterRequestData()
.setClusterId(clusterId)
.setTimeoutMs(timeoutMs)
Review Comment:
I think I see what you mean @chia7712.
The leader does have timeout logic to expire the pending `AddVoterRequest`
when it sends the `ApiVersionsRequest`. But it doesn't check the client-side
timeout sent as part of the request has expired. In practice, this doesn't
affect too much because the leader will either time out the request during the
APIVersions handling, or if unable to commit the new voters record, will
resign. The important thing is that the leader does not get stuck with a
pending voters operation. For the purposes of auto-join PR, adding handling for
the client-side timeout is out of scope.
Additionally, for the default timeout of an AdminClient request to
add/remove a voter (1 min IIRC), the fetch timeout is significantly smaller, so
it won't matter. If the timeout is sufficiently small enough, then we are
missing handling on the server-side.
Filed https://issues.apache.org/jira/browse/KAFKA-19600 as a follow-up.
--
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
