lucasbru commented on code in PR #14680:
URL: https://github.com/apache/kafka/pull/14680#discussion_r1392314116


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/PrototypeAsyncConsumer.java:
##########
@@ -712,6 +729,10 @@ private void close(Duration timeout, boolean 
swallowException) {
         if (applicationEventHandler != null)
             closeQuietly(() -> applicationEventHandler.close(timeout), "Failed 
to close application event handler with a timeout(ms)=" + timeout, 
firstException);
 
+        // Invoke all callbacks after the background thread exists in case if 
there are unsent async
+        // commits
+        maybeInvokeCallbacks();

Review Comment:
   The fenced exception would indicate here that the last commit did not 
complete, correct? I think you are right, throwing the exception is not 
strictly required for correctness here; at the same time, when investigating 
what was going on the client during a fencing situation, it would be valuable 
information that the commit failed due to fencing, even if we are shutting 
down. If we can tell that from non-DEBUG logs, I'd say it's fine though, I 
haven't checked.



-- 
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

Reply via email to