lianetm commented on code in PR #18702:
URL: https://github.com/apache/kafka/pull/18702#discussion_r1939721414
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1326,7 +1331,7 @@ private void close(Duration timeout, boolean
swallowException) {
// We are already closing with a timeout, don't allow wake-ups from
here on.
wakeupTrigger.disableWakeups();
- final Timer closeTimer = time.timer(timeout);
+ final Timer closeTimer = createTimer(timeout);
Review Comment:
not introduced here but affected with this change. I notice that
`runRebalanceCallbacksOnClose` consumes time from the close timeout, right?
(receives the timer just to update it). But that behaviour is not the same in
the Classic Consumer.
In the classic, the close timeout only applies to requests really. The
callbacks run when closing the Abstract coordinator, without time boundaries,
and most importantly, without consuming time from the close timeout. We
`runRebalanceCallbacksOnClose` without time boundaries too, but it does consume
the time from the timeout param, right? Wouldn't that potentially leave less
time for the following requests? I'm concerned about existing apps, running
callbacks, calling close with a timeout that used to be "enough", but now it
may not be. Should we simply remove the timer from the
runRebalanceCallbacksOnClose?
--
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]
