lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2135478481
Hey @appchemist , thanks for the updates! High level comment with the goal
of trying to simplify if possible. I don't quite get the need for a new way of
updating metadata (`ErrorPropagateMetadataUpdater`) when it seems to me that
the way metadata is updated is not different for the new consumer vs the legacy
one. It's how the errors are propagated what's different here (and what we need
to sort out). What about the option of just keeping the metadata update logic
as it was, and only changing the propagation logic:
1. call a new `maybeThrowMetadataErrors()` in the NetworkClientDelegate
poll()
2. define maybeThrowMetadataErrors simply as :
```
private void maybeThrowMetadataErrors() {
try {
metadata.maybeThrowAnyException();
} catch (Exception e) {
backgroundEventHandler.add(new ErrorEvent(e));
}
}
```
With this, we know that metadata updates will be applied in the same way for
both consumer impl (ConsumerNetworkClient and NetworkClientDelegate), and the
only difference is how the error is propagated (directly thrown in the former
vs propagated via events in the latter). Does this make sense?
Regardless of the impl, I suggest we also add a test in the
`NetworkClientDelegate`, mocking metadata errors, and seeing how polling the
network client should generate the `ErrorEvent`.
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
