lianetm commented on code in PR #22035:
URL: https://github.com/apache/kafka/pull/22035#discussion_r3529969210
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventHandler.java:
##########
@@ -155,4 +158,25 @@ public void close(final Duration timeout) {
() -> log.warn("The application event handler was already
closed")
);
}
+
+ /**
+ * Best-effort check that the consumer network thread is still alive. If
the thread has
+ * already terminated (due to a failure or shutdown), it will never
process any events from
+ * the queue. Rather than blocking indefinitely or timing out with a
misleading error, this
+ * fails fast with a clear error message.
+ *
+ * <p>Note: this is inherently racy — the thread could die between this
check and the
+ * subsequent {@code applicationEventQueue.add()}. That narrow window is
acceptable because
+ * any subsequent call to {@code add()} will detect the dead thread
immediately, and any
+ * orphaned events will be expired by the {@link CompletableEventReaper}
during consumer
+ * {@link #close() close}.
Review Comment:
uhm I'm not sure we can guarantee that "orphaned events will be expired
during consumer close" ? I agree that if the thread closes gracefully (runOnce
stops), app events are reaped on the finally, but that wouldn't be the case if
the thread was already terminated when we hit the add, after this check, right?
If so, we should probably just remove the claim about reaping on close in the
race case.
--
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]
