AndrewJSchofield commented on code in PR #19192:
URL: https://github.com/apache/kafka/pull/19192#discussion_r1993541564
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManager.java:
##########
@@ -527,8 +533,12 @@ public CompletableFuture<Map<TopicIdPartition,
Acknowledgements>> commitSync(
* Enqueue an AcknowledgeRequestState to be picked up on the next poll.
*
* @param acknowledgementsMap The acknowledgements to commit
+ * @param defaultTimeoutMs Timeout which would be used when the request
is retried, i.e. if it fails with
Review Comment:
I'm a bit confused by the difference between `deadlineMs` for `commitSync()`
and `defaultTimeoutMs` for `commitAsync()`. It seems to me they are the same
thing, especially since in both cases the value becomes the third argument to
the constructor for `AcknowledgeRequestState`. Probably worth aligning these
things (and I don't see what was wrong with `deadlineMs` for the async case).
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManager.java:
##########
@@ -1188,6 +1206,14 @@ boolean isEmpty() {
inFlightAcknowledgements.isEmpty();
}
+ /**
+ * Resets the timer with the new deadline and resets the RequestState.
Review Comment:
nit: Probably "new timeout" rather than "new deadline" given that you've
replaced the concept of deadline with a timeout.
--
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
