kirktrue commented on code in PR #14359:
URL: https://github.com/apache/kafka/pull/14359#discussion_r1323783693
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/Fetcher.java:
##########
@@ -50,7 +55,8 @@
public class Fetcher<K, V> extends AbstractFetch<K, V> {
private final Logger log;
- private final AtomicBoolean isClosed = new AtomicBoolean(false);
+ private final FetchCollector<K, V> fetchCollector;
+ private final ConsumerNetworkClient client;
Review Comment:
I made a change here that I'm still not crazy about.
Instead of having a `NodeStatusDetector` instance variable in
`AbstractFetch`, I introduced a type parameter in `AbstractFetch` that allows a
subclass to provide a more specific type. Now `Fetcher` can provide the type as
`ConsumerNetworkClient` and thus it can call the specialized methods on the
instance variable from `AbstractFetch`.
The part I'm not crazy about is that the instance variable is still called
`nodeStatusDetector`. I think I would be a bit confused when I see calls like
`nodeStatusDetector.disableWakeups()`. But maybe that's just me. I thought of
changing the name of the instance variable to `client` or `networkInfo` or
other equally vague names.
If you have any input/suggestions/preferences, do LMK. 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
