squah-confluent commented on code in PR #20847:
URL: https://github.com/apache/kafka/pull/20847#discussion_r2594677110
##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeMetricsImpl.java:
##########
@@ -69,6 +69,16 @@ public class CoordinatorRuntimeMetricsImpl implements
CoordinatorRuntimeMetrics
*/
public static final String BATCH_FLUSH_TIME_METRIC_NAME =
"batch-flush-time-ms";
+ /**
+ * The cached buffer size metric name.
Review Comment:
nit:
```suggestion
* The buffer cache size metric name.
```
##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeMetricsImpl.java:
##########
@@ -92,6 +102,17 @@ public class CoordinatorRuntimeMetricsImpl implements
CoordinatorRuntimeMetrics
*/
private final MetricName eventQueueSize;
+ /**
+ * Metric to count the size of the cached buffer.
Review Comment:
nit:
```suggestion
* Metric to count the size of the cached buffers.
```
##########
coordinator-common/src/test/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeMetricsImplTest.java:
##########
@@ -59,6 +61,8 @@ private static Set<MetricName> expectedMetricNames(Metrics
metrics) {
kafkaMetricName(metrics, NUM_PARTITIONS_METRIC_NAME, "state",
"loading"),
kafkaMetricName(metrics, NUM_PARTITIONS_METRIC_NAME, "state",
"active"),
kafkaMetricName(metrics, NUM_PARTITIONS_METRIC_NAME, "state",
"failed"),
+ kafkaMetricName(metrics, BATCH_BUFFER_CACHE_SIZE_METRIC_NAME),
+ kafkaMetricName(metrics,
BATCH_BUFFER_CACHE_DISCARD_COUNT_METRIC_NAME),
Review Comment:
nit: Can we move these down after the `batch-flush-rate`? so that they're
together with the batch- metrics.
##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeMetricsImpl.java:
##########
@@ -156,6 +177,17 @@ public CoordinatorRuntimeMetricsImpl(Metrics metrics,
String metricsGroup) {
this.eventQueueSize = kafkaMetricName("event-queue-size", "The event
accumulator queue size.");
+ this.bufferCacheSize = kafkaMetricName(
+ BATCH_BUFFER_CACHE_SIZE_METRIC_NAME,
+ "The current total size in bytes of the append buffers being held
in the coordinator's cache."
+ );
+
+ this.bufferCacheDiscardCount = kafkaMetricName(
+ BATCH_BUFFER_CACHE_DISCARD_COUNT_METRIC_NAME,
+ "The count of over-sized append buffers that were discarded
instead of being cached upon release."
+ );
+
+ metrics.addMetric(bufferCacheDiscardCount, (Gauge<Long>) (config, now)
-> bufferCacheDiscardCounter.get());
Review Comment:
nit: Could we move this after `metrics.addMetric(numPartitionsFailed` to
match the order of the field definitions?
##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeMetricsImpl.java:
##########
@@ -69,6 +69,16 @@ public class CoordinatorRuntimeMetricsImpl implements
CoordinatorRuntimeMetrics
*/
public static final String BATCH_FLUSH_TIME_METRIC_NAME =
"batch-flush-time-ms";
+ /**
+ * The cached buffer size metric name.
+ */
+ public static final String BATCH_BUFFER_CACHE_SIZE_METRIC_NAME =
"batch-buffer-cache-size-bytes";
+
+ /**
+ * The buffer skip cache count metric name.
Review Comment:
nit:
```suggestion
* The buffer cache discard count metric name.
```
##########
docs/upgrade.html:
##########
@@ -19,16 +19,26 @@
<script id="upgrade-template" type="text/x-handlebars-template">
+<h4><a id="upgrade_4_3_0" href="#upgrade_4_3_0">Upgrading to 4.3.0</a></h4>
Review Comment:
Awesome, thank you! Since there's no documentation for the share coordinator
metrics yet, let's wait until #21046 is merged.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
