dajac commented on code in PR #16949:
URL: https://github.com/apache/kafka/pull/16949#discussion_r1727171724


##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntime.java:
##########
@@ -769,6 +772,7 @@ private void flushCurrentBatch() {
                     for (DeferredEvent event : currentBatch.deferredEvents) {
                         deferredEventQueue.add(offset, event);
                     }
+                    runtimeMetrics.recordFlushTime(time.milliseconds() - 
flushStartMs);

Review Comment:
   nit: Should we record this just before `updateLastWrittenOffset`? This is 
actually what you did on the transaction completion path.



##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/metrics/GroupCoordinatorRuntimeMetricsTest.java:
##########
@@ -48,15 +54,38 @@ public void testMetricNames() {
             metrics.metricName("event-queue-size", METRICS_GROUP),
             metrics.metricName("partition-load-time-max", METRICS_GROUP),
             metrics.metricName("partition-load-time-avg", METRICS_GROUP),
-            metrics.metricName("thread-idle-ratio-avg", METRICS_GROUP)
+            metrics.metricName("thread-idle-ratio-avg", METRICS_GROUP),
+            metrics.metricName("event-queue-time-ms-max", METRICS_GROUP),
+            metrics.metricName("event-queue-time-ms-p50", METRICS_GROUP),
+            metrics.metricName("event-queue-time-ms-p95", METRICS_GROUP),
+            metrics.metricName("event-queue-time-ms-p99", METRICS_GROUP),
+            metrics.metricName("event-queue-time-ms-p999", METRICS_GROUP),
+            metrics.metricName("event-processing-time-ms-max", METRICS_GROUP),
+            metrics.metricName("event-processing-time-ms-p50", METRICS_GROUP),
+            metrics.metricName("event-processing-time-ms-p95", METRICS_GROUP),
+            metrics.metricName("event-processing-time-ms-p99", METRICS_GROUP),
+            metrics.metricName("event-processing-time-ms-p999", METRICS_GROUP),
+            metrics.metricName("event-purgatory-time-ms-max", METRICS_GROUP),
+            metrics.metricName("event-purgatory-time-ms-p50", METRICS_GROUP),
+            metrics.metricName("event-purgatory-time-ms-p95", METRICS_GROUP),
+            metrics.metricName("event-purgatory-time-ms-p99", METRICS_GROUP),
+            metrics.metricName("event-purgatory-time-ms-p999", METRICS_GROUP),
+            metrics.metricName("batch-flush-time-ms-max", METRICS_GROUP),
+            metrics.metricName("batch-flush-time-ms-p50", METRICS_GROUP),
+            metrics.metricName("batch-flush-time-ms-p95", METRICS_GROUP),
+            metrics.metricName("batch-flush-time-ms-p99", METRICS_GROUP),
+            metrics.metricName("batch-flush-time-ms-p999", METRICS_GROUP)

Review Comment:
   Could you please add those metrics to the description and to the KIP?



-- 
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

Reply via email to