lucasbru commented on code in PR #15693:
URL: https://github.com/apache/kafka/pull/15693#discussion_r1601287604
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java:
##########
@@ -984,6 +984,8 @@ public void close(final Timer timer) {
}
} finally {
super.close(timer);
+ // Super-class close may wait for more commit callbacks to
complete.
+ invokeCompletedOffsetCommitCallbacks();
Review Comment:
There may async commits that are completed during coordinator close. This is
done to provide the guarantee that we want to complete async commits in
`Consumer.close` (as long as we don't timeout). We also provide the guarantee
that if an async commit completes, the corresponding callback is executed. So
then we need to execute callbacks here.
This change could cause issues if we somehow interact with the consumer
within the commit callback, but the consumer is already partially closed. Note
that callback is already called during close (further above) where the
coordinator isn't called yet. So essentially, this change will just make it
"more commonplace" to call the callbacks during close.
But it would be an option to just leave the legacy consumer to have somewhat
inconsistent behavior here, and just change it for the new consumer. WDYT?
--
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]
