m1a2st commented on code in PR #18050:
URL: https://github.com/apache/kafka/pull/18050#discussion_r1879153506
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/CoordinatorRequestManager.java:
##########
@@ -188,12 +184,12 @@ private void onFailedResponse(final long currentTimeMs,
final Throwable exceptio
if (exception == Errors.GROUP_AUTHORIZATION_FAILED.exception()) {
log.debug("FindCoordinator request failed due to authorization
error {}", exception.getMessage());
KafkaException groupAuthorizationException =
GroupAuthorizationException.forGroupId(this.groupId);
- backgroundEventHandler.add(new
ErrorEvent(groupAuthorizationException));
+ fatalError = Optional.of(groupAuthorizationException);
return;
}
log.warn("FindCoordinator request failed due to fatal exception",
exception);
- backgroundEventHandler.add(new ErrorEvent(exception));
Review Comment:
> we keep the coordinator mgr propagating its errors via the event queue,
just because we do need to know about them in the app thread (from the
consumer.poll)
In my opinion, if we remove the event queue propagation method in this PR,
it might lead to unexpected behavior in SharedConsumer. If this concern exists,
should we consider keeping the event queue propagation method?
> the coord mgr does not propagate via error event anymore, just exposes the
coordinator found or fatal error. Other managers that require a coordinator
know what to do about it
I'm not entirely clear on this approach, so I'm still thinking it through.
--
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
