lianetm commented on code in PR #18702:
URL: https://github.com/apache/kafka/pull/18702#discussion_r1941349655
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1368,6 +1373,10 @@ private void close(Duration timeout, boolean
swallowException) {
}
}
+ private Timer createTimer(Duration timeout) {
Review Comment:
nit: should we show this is for the close op? `createTimerForCloseRequests`
or similar maybe?
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1368,6 +1373,10 @@ private void close(Duration timeout, boolean
swallowException) {
}
}
+ private Timer createTimer(Duration timeout) {
+ return time.timer(Duration.ofMillis(Math.min(timeout.toMillis(),
requestTimeoutMs)));
Review Comment:
```suggestion
return time.timer(Math.min(timeout.toMillis(), requestTimeoutMs));
```
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1368,6 +1373,10 @@ private void close(Duration timeout, boolean
swallowException) {
}
}
+ private Timer createTimer(Duration timeout) {
+ return time.timer(Duration.ofMillis(Math.min(timeout.toMillis(),
requestTimeoutMs)));
Review Comment:
Should we add a null check to the time obj here? Similar to what the classic
does, since close can be called from the constructor at any point (finally) if
something fails.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -1782,6 +1782,13 @@ public void close() {
* timeout. If the consumer is unable to complete offset commits and
gracefully leave the group
* before the timeout expires, the consumer is force closed. Note that
{@link #wakeup()} cannot be
* used to interrupt close.
+ * <p>
+ * The actual maximum wait time is bounded by the {@link
ConsumerConfig#REQUEST_TIMEOUT_MS_CONFIG} setting, which
+ * only applies to operations performed with the coordinator
(coordinator-related requests and
Review Comment:
Should we say this instead? (could be to the coordinator or the leader)
```suggestion
* only applies to operations performed with the broker
(coordinator-related requests and
```
--
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
