philipnee commented on code in PR #16685:
URL: https://github.com/apache/kafka/pull/16685#discussion_r1700543658
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventHandler.java:
##########
@@ -54,13 +57,37 @@ public ApplicationEventHandler(final LogContext logContext,
final Supplier<RequestManagers>
requestManagersSupplier) {
this.log = logContext.logger(ApplicationEventHandler.class);
this.applicationEventQueue = applicationEventQueue;
+ this.networkThreadMetricsManager = null;
this.networkThread = new ConsumerNetworkThread(logContext,
time,
applicationEventQueue,
applicationEventReaper,
applicationEventProcessorSupplier,
networkClientDelegateSupplier,
- requestManagersSupplier);
+ requestManagersSupplier,
+ null);
+ this.networkThread.start();
+ }
+
+ public ApplicationEventHandler(final LogContext logContext,
Review Comment:
This is Method Reference. so you don't need to add an extra ctor
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventHandler.java:
##########
@@ -54,13 +57,37 @@ public ApplicationEventHandler(final LogContext logContext,
final Supplier<RequestManagers>
requestManagersSupplier) {
this.log = logContext.logger(ApplicationEventHandler.class);
this.applicationEventQueue = applicationEventQueue;
+ this.networkThreadMetricsManager = null;
this.networkThread = new ConsumerNetworkThread(logContext,
time,
applicationEventQueue,
applicationEventReaper,
applicationEventProcessorSupplier,
networkClientDelegateSupplier,
- requestManagersSupplier);
+ requestManagersSupplier,
+ null);
+ this.networkThread.start();
+ }
+
+ public ApplicationEventHandler(final LogContext logContext,
Review Comment:
just modify the original constructor and the factor class should work
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/events/BackgroundEventHandler.java:
##########
@@ -30,9 +32,24 @@
public class BackgroundEventHandler {
private final Queue<BackgroundEvent> backgroundEventQueue;
+ private final KafkaConsumerMetrics kafkaConsumerMetrics;
+ private final KafkaShareConsumerMetrics kafkaShareConsumerMetrics;
+ private boolean isShareConsumer;
- public BackgroundEventHandler(final Queue<BackgroundEvent>
backgroundEventQueue) {
+ public BackgroundEventHandler(final Queue<BackgroundEvent>
backgroundEventQueue, KafkaConsumerMetrics kafkaConsumerMetrics) {
this.backgroundEventQueue = backgroundEventQueue;
+ this.kafkaConsumerMetrics = kafkaConsumerMetrics;
+ this.kafkaShareConsumerMetrics = null;
+ isShareConsumer = false;
Review Comment:
you don't really need this - option one: have kafkaConsumerMetrics and
shareConsumerMetrics to impelment a common interface and do if(metrics
instanceOf [one of the metrics class])
option two: either do null check or use optional. Optional is better imo.
--
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
