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