kirktrue commented on code in PR #17199:
URL: https://github.com/apache/kafka/pull/17199#discussion_r1868541417
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -380,7 +387,9 @@ public void onGroupAssignmentUpdated(Set<TopicPartition>
partitions) {
new CompletableEventReaper(logContext),
applicationEventProcessorSupplier,
networkClientDelegateSupplier,
- requestManagersSupplier);
+ requestManagersSupplier,
+ kafkaConsumerMetrics
Review Comment:
Nit: alignment.
```suggestion
requestManagersSupplier,
kafkaConsumerMetrics
```
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -553,7 +570,8 @@ public void onGroupAssignmentUpdated(Set<TopicPartition>
partitions) {
new CompletableEventReaper(logContext),
applicationEventProcessorSupplier,
networkClientDelegateSupplier,
- requestManagersSupplier);
+ requestManagersSupplier,
+ kafkaConsumerMetrics);
Review Comment:
Nit: alignment.
```suggestion
requestManagersSupplier,
kafkaConsumerMetrics);
```
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##########
@@ -60,21 +61,24 @@ public class ConsumerNetworkThread extends KafkaThread
implements Closeable {
private final Supplier<ApplicationEventProcessor>
applicationEventProcessorSupplier;
private final Supplier<NetworkClientDelegate>
networkClientDelegateSupplier;
private final Supplier<RequestManagers> requestManagersSupplier;
+ private final Optional<KafkaConsumerMetrics> kafkaConsumerMetrics;
private ApplicationEventProcessor applicationEventProcessor;
private NetworkClientDelegate networkClientDelegate;
private RequestManagers requestManagers;
private volatile boolean running;
private final IdempotentCloser closer = new IdempotentCloser();
private volatile Duration closeTimeout =
Duration.ofMillis(DEFAULT_CLOSE_TIMEOUT_MS);
private volatile long cachedMaximumTimeToWait = MAX_POLL_TIMEOUT_MS;
+ private long lastPollTimeMs = 0L;
Review Comment:
Pinging on this. I believe it's only written on the background thread, but
just want to be sure.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -350,7 +355,9 @@ public void onGroupAssignmentUpdated(Set<TopicPartition>
partitions) {
metrics,
fetchMetricsManager.throttleTimeSensor(),
clientTelemetryReporter.map(ClientTelemetryReporter::telemetrySender).orElse(null),
- backgroundEventHandler);
+ backgroundEventHandler,
+ kafkaConsumerMetrics
Review Comment:
Nit: alignment.
```suggestion
backgroundEventHandler,
kafkaConsumerMetrics
```
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -458,10 +465,15 @@ public void onGroupAssignmentUpdated(Set<TopicPartition>
partitions) {
this.defaultApiTimeoutMs = defaultApiTimeoutMs;
this.deserializers = deserializers;
this.applicationEventHandler = applicationEventHandler;
- this.kafkaConsumerMetrics = new KafkaConsumerMetrics(metrics,
"consumer");
+ this.kafkaConsumerMetrics = new AsyncConsumerMetrics(metrics,
CONSUMER_METRIC_GROUP_PREFIX);
Review Comment:
👍
--
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
