KAFKA-10397: do not expose statistics-based RocksDB metrics when the user provides a Statistics object#22719
Open
Aangbaeck wants to merge 1 commit into
Conversation
…user provides a Statistics object When a user supplies their own Statistics object to a RocksDB state store through RocksDBConfigSetter, RocksDBStore passes null to the metrics recorder, because Kafka Streams must not read the user's Statistics -- doing so via getAndResetTickerCount() would reset the user's counters. As a result record() short-circuits and the statistics-based metrics (bytes-written, bytes-read, memtable-*, block-cache-* hit ratios, compaction-*, write-stall-duration, number-open-files, number-file-errors) are never recorded. They were, however, still registered in init() and therefore exposed as perpetually-empty metrics. Fix: register the statistics-based sensors lazily in addValueProviders(), only once a value provider with a non-null Statistics is added, instead of unconditionally in init(). The property / block-cache gauges read the RocksDB instance directly and remain valid without a Statistics object, so they are still registered in init() as before. record() additionally guards on whether the sensors were registered. This changes only which metrics are registered in the user-provided-Statistics case; the default (Streams-owned Statistics at RecordingLevel.DEBUG) is unchanged, and the property-based gauges are unchanged in all cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KAFKA-10397
When a user supplies their own
Statisticsobject to a RocksDB state store throughRocksDBConfigSetter,RocksDBStorepassesnullto the metrics recorder, becauseKafka Streams must not read the user's
Statistics— reading it viagetAndResetTickerCount()would reset the user's counters. As a resultRocksDBMetricsRecorder.record()short-circuits and the statistics-based metrics arenever recorded. They were, however, still registered in
init()and therefore exposedas perpetually-empty metrics.
Affected (statistics-based,
RecordingLevel.DEBUG) metrics:bytes-written,bytes-read,memtable-bytes-flushed,memtable-hit-ratio,memtable-flush-time-{avg,min,max},write-stall-duration,block-cache-{data,index,filter}-hit-ratio,bytes-{read,written}-compaction,compaction-time-{avg,min,max},number-open-files,number-file-errors.Fix
Register the statistics-based sensors lazily in
addValueProviders(), only once a valueprovider with a non-null
Statisticsis added, instead of unconditionally ininit().The property / block-cache gauges read the RocksDB instance directly and remain valid
without a
Statisticsobject, so they are still registered ininit()as before.record()additionally guards on whether the sensors were registered.Only the user-provided-
Statisticscase changes; the default (Streams-ownedStatisticsat
RecordingLevel.DEBUG) and the property-based gauges are unchanged in all cases.Testing
RocksDBMetricsRecorderTest— new/updated unit tests: gauges (not sensors) areregistered on
init(); statistics-based sensors are registered when value providerswith a
Statisticsare added, not registered whenStatisticsis absent, andregistered only once across multiple segments.
RocksDBStoreTest.shouldNotRegisterStatisticsBasedMetricsWhenUserProvidesStatistics—end-to-end: with a user-provided
StatisticsatDEBUG, a statistics-based metric(
bytes-written-total) is not registered while a property gauge(
num-entries-active-mem-table) still is.checkstyle,spotlessandspotbugspass.Note on compatibility / KIP
This changes which metrics are registered in the specific case of a user-provided
Statisticsobject (they were always empty in that case); it does not change thedocumented metric set for the default configuration. The JIRA (filed by a committer) is
not tagged
needs-kip— flagging here so reviewers can confirm whether any KIP orcompatibility note is warranted.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com