Skip to content

KAFKA-19711: Add commit-rate metric to metered state stores Add commit metric#21853

Open
bbejeck wants to merge 2 commits intoapache:trunkfrom
bbejeck:KAFKA-19711_add_commit_metric
Open

KAFKA-19711: Add commit-rate metric to metered state stores Add commit metric#21853
bbejeck wants to merge 2 commits intoapache:trunkfrom
bbejeck:KAFKA-19711_add_commit_metric

Conversation

@bbejeck
Copy link
Member

@bbejeck bbejeck commented Mar 23, 2026

Add commit-rate, commit-latency-avg, and commit-latency-max metrics to replace the flush-* metrics which are now deprecated. Both
sets of metrics are recorded during the deprecation period for backward
compatibility. The flush-* metrics will be removed in the next major
release. Also adds a varargs overload of
StreamsMetricsImpl.maybeMeasureLatency to support recording the same
latency on multiple sensors cleanly.

Copy link
Contributor

@eduwercamacaro eduwercamacaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 329 to 330
final KafkaMetric metric = metric("flush-rate");
assertTrue((Double) metric.metricValue() > 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The KIP mentions that while the flush metrics are only deprecated, they will no longer record any data under normal use, as Kafka Streams will no longer call StateStore#flush(), but this test seems to assert on a non-zero value. Is not recording flush metrics a next step?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, no my intent was to do it in one step, that detail of the KIP slipped my mind. I'll update it.

@bbejeck
Copy link
Member Author

bbejeck commented Mar 23, 2026

@clolov I've updated the PR to only record for the commit-X metrics. We need to keep the deprecated flush-X around until the next major release, so I opted to use a dummy value in the init method as workaround for the Spotbugs unused field violation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants