lianetm commented on code in PR #17440:
URL: https://github.com/apache/kafka/pull/17440#discussion_r1850512580
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/CoordinatorRequestManager.java:
##########
@@ -188,6 +190,8 @@ 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);
+ metadataError.completeExceptionally(groupAuthorizationException);
+ metadataError = metadataError.newIncompleteFuture();
Review Comment:
hey, you're right about the differences in how errors are propagated, but
they are in different situations, some with different threading model actually,
so makes sense we won't have the same approach right? (errors that need to move
around the same thread vs different threads). To recap, the requirements are:
1. propagate errors from background thread to app thread (here we could have
a triggering event we can use, or not, so we use `ErrorEvent` consumed from
poll) - this is already in place
2. propagate errors within the background thread to fail pending requests
(here we have the case of coordinator errors, metadata errors, ...). - this is
a gap this PR is aiming to fix, split in 2 for coord and metadata
Do the use cases makes sense? And do we agree that what we're trying to fix
with this PR is requirement 2? If so then it makes sense that we won't be
mixing events/ErrorEvent here, simply because we just need to pass errors
around in the same thread (requirement 2). Please let me know if I'm missing
something about the problem we're trying to fix. Thanks!
--
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]
