Aangbaeck opened a new pull request, #22719:
URL: https://github.com/apache/kafka/pull/22719

   ## Summary
   
   [KAFKA-10397](https://issues.apache.org/jira/browse/KAFKA-10397)
   
   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` — reading it via
   `getAndResetTickerCount()` would reset the user's counters. As a result
   `RocksDBMetricsRecorder.record()` short-circuits and the statistics-based 
metrics are
   never recorded. They were, however, still registered in `init()` and 
therefore exposed
   as 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 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.
   
   Only the user-provided-`Statistics` case changes; the default (Streams-owned 
`Statistics`
   at `RecordingLevel.DEBUG`) and the property-based gauges are unchanged in 
all cases.
   
   ## Testing
   
   - `RocksDBMetricsRecorderTest` — new/updated unit tests: gauges (not 
sensors) are
     registered on `init()`; statistics-based sensors are registered when value 
providers
     with a `Statistics` are added, not registered when `Statistics` is absent, 
and
     registered only once across multiple segments.
   - 
`RocksDBStoreTest.shouldNotRegisterStatisticsBasedMetricsWhenUserProvidesStatistics`
 —
     end-to-end: with a user-provided `Statistics` at `DEBUG`, a 
statistics-based metric
     (`bytes-written-total`) is not registered while a property gauge
     (`num-entries-active-mem-table`) still is.
   - `checkstyle`, `spotless` and `spotbugs` pass.
   
   ## Note on compatibility / KIP
   
   This changes which metrics are *registered* in the specific case of a 
user-provided
   `Statistics` object (they were always empty in that case); it does not 
change the
   documented 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 or
   compatibility note is warranted.
   
   Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
   


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

Reply via email to