feat(inkless:consume): add diskless reader throughput metrics - #727
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds byte-throughput meters for Inkless diskless fetches (per-reader) and for the “consolidation supplement” paths in the broker, so operators can observe consumer-served bytes, cache-hit bytes, and remote-storage download rates for both hot and lagging consumer paths.
Changes:
- Add new per-reader diskless throughput meters to
InklessFetchMetricsand record them insideFetchPlannerhot/cold fetch paths. - Add consolidation supplement/local byte meters on
ReplicaManager, recording in both synchronous fetch handling andDelayedFetch(without double-counting). - Update metrics documentation and add/extend unit tests to validate the new meters’ behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| storage/inkless/src/test/java/io/aiven/inkless/consume/FetchPlannerTest.java | Adds assertions verifying the new diskless throughput metric recording on cache hit/miss and lagging paths. |
| storage/inkless/src/main/java/io/aiven/inkless/consume/InklessFetchMetrics.java | Defines/registers/removes new meters and exposes new record*Bytes* methods. |
| storage/inkless/src/main/java/io/aiven/inkless/consume/FetchPlanner.java | Records new throughput meters at cache/storage read sites for hot and lagging fetch paths. |
| docs/inkless/metrics.rst | Documents the newly added per-reader throughput metrics. |
| core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala | Adds tests validating consolidation metric recording and non-double-counting across sync vs delayed fetch. |
| core/src/main/scala/kafka/server/ReplicaManager.scala | Introduces consolidation supplement/local byte meters and records them on synchronous response paths only. |
| core/src/main/scala/kafka/server/DelayedFetch.scala | Records consolidation local bytes and supplement meters during the delayed-fetch re-read/re-supplement path. |
Comments suppressed due to low confidence (2)
core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala:2492
- This test asserts the Yammer meter is exactly 0, but Yammer meters are registered in a global registry and can have non-zero counts from earlier tests in the same JVM. Capture the baseline before calling
fetchMessagesand assert that it does not change while the request is parked.
assertEquals(1, fetchPurgatory.watched(), "Fetch must park (supplement error, minBytes unmet)")
assertEquals(0L, yammerMeterCount("ConsolidationLocalBytesPerSec"),
"Sync read must not record local bytes when parking; DelayedFetch records them on completion")
core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala:2545
- This assertion expects the Yammer meter count to equal
localBytes, but Yammer meters live in a global registry and may already have a non-zero count from previous tests. Capture the baseline before the fetch and assert on the delta instead.
waitForFetchResponse(responseData)
assertEquals(localBytes, yammerMeterCount("ConsolidationLocalBytesPerSec"),
"Immediate response must record the local-log bytes exactly once")
jeqo
force-pushed
the
jeqo/kc-275-reader-throughput-metrics
branch
from
July 29, 2026 08:22
943e8f4 to
edd135c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
storage/inkless/src/main/java/io/aiven/inkless/consume/FetchPlanner.java:336
- In the cold-path fetch, DisklessBytesOut/StorageLaggingBytesIn are recorded inside the primary supplier. If a hedge completes
primarysuccessfully and the primary supplier later fails (throws), the request is still served from the hedge but these throughput metrics will never be recorded because the supplier exits before therecord*calls run. Consider recording these metrics from the future completion (the value that actually wins/completesprimary), similar to the hot-pathprimary.thenAccept(...), and removing the in-supplier recording to avoid double-counting.
final FileExtent fileExtent = fetchFileExtent(laggingObjectFetcher, request, firstByteReceived);
metrics.recordStorageLaggingBytesIn(fileExtent.data().length);
metrics.recordDisklessBytesOut(fileExtent.data().length);
return fileExtent;
docs/inkless/metrics.rst:148
- This doc update covers the new InklessFetch throughput meters, but the PR also introduces new broker-side consolidation throughput meters on ReplicaManager (ConsolidationSupplementRate/ConsolidationSupplementBytesPerSec/ConsolidationLocalBytesPerSec). Those new JMX metrics are not documented anywhere in this generated metrics reference; please add documentation for them (either by extending the generator or adding a dedicated section for these Yammer metrics) so operators can discover and interpret them.
RecentDataRequestRate Rate of requests served via the hot path (recent data with cache) per second. Under the consolidation metrics group (ConsolidationFetchMetrics) this counts cache-hit peeks that reuse consumer-cached data.
StorageBytesInPerSec Bytes downloaded from remote storage on the hot path (cache misses) per second. Counts the primary (cache-populating) download only; a hedged retry's second download is not counted here.
StorageLaggingBytesInPerSec Bytes downloaded from remote storage on the cold path (lagging consumers) per second
jeqo
marked this pull request as ready for review
July 29, 2026 08:49
jeqo
force-pushed
the
jeqo/kc-275-reader-throughput-metrics
branch
from
July 29, 2026 13:50
edd135c to
5d13dcf
Compare
Adds byte-level throughput metrics for the diskless fetch path: - DisklessBytesOutPerSec, CacheHitBytesPerSec, StorageBytesInPerSec, StorageLaggingBytesInPerSec on InklessFetchMetrics (per-Reader JMX group), recorded at the FetchPlanner cache/storage read sites. - ConsolidationSupplementRate, ConsolidationSupplementBytesPerSec, ConsolidationLocalBytesPerSec on ReplicaManager, recorded across the synchronous fetchMessages path and DelayedFetch. The three consolidation meters are recorded only on the paths that respond from the synchronous local-log read; the delayedResponse path discards that read and re-reads (and re-issues the supplement) inside DelayedFetch, which records there, so marking in both would double-count. CacheHitBytesPerSec is attributed from the future's completion state at lookup (a true hit returns an already-complete future), so a caller that coalesces onto an in-flight cache miss is not miscounted as a cache hit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeqo
force-pushed
the
jeqo/kc-275-reader-throughput-metrics
branch
from
July 29, 2026 14:14
5d13dcf to
b726b4b
Compare
giuseppelillo
approved these changes
Jul 29, 2026
giuseppelillo
pushed a commit
that referenced
this pull request
Jul 29, 2026
Adds byte-level throughput metrics for the diskless fetch path: - DisklessBytesOutPerSec, CacheHitBytesPerSec, StorageBytesInPerSec, StorageLaggingBytesInPerSec on InklessFetchMetrics (per-Reader JMX group), recorded at the FetchPlanner cache/storage read sites. - ConsolidationSupplementRate, ConsolidationSupplementBytesPerSec, ConsolidationLocalBytesPerSec on ReplicaManager, recorded across the synchronous fetchMessages path and DelayedFetch. The three consolidation meters are recorded only on the paths that respond from the synchronous local-log read; the delayedResponse path discards that read and re-reads (and re-issues the supplement) inside DelayedFetch, which records there, so marking in both would double-count. CacheHitBytesPerSec is attributed from the future's completion state at lookup (a true hit returns an already-complete future), so a caller that coalesces onto an in-flight cache miss is not miscounted as a cache hit. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
giuseppelillo
pushed a commit
that referenced
this pull request
Jul 30, 2026
Adds byte-level throughput metrics for the diskless fetch path: - DisklessBytesOutPerSec, CacheHitBytesPerSec, StorageBytesInPerSec, StorageLaggingBytesInPerSec on InklessFetchMetrics (per-Reader JMX group), recorded at the FetchPlanner cache/storage read sites. - ConsolidationSupplementRate, ConsolidationSupplementBytesPerSec, ConsolidationLocalBytesPerSec on ReplicaManager, recorded across the synchronous fetchMessages path and DelayedFetch. The three consolidation meters are recorded only on the paths that respond from the synchronous local-log read; the delayedResponse path discards that read and re-reads (and re-issues the supplement) inside DelayedFetch, which records there, so marking in both would double-count. CacheHitBytesPerSec is attributed from the future's completion state at lookup (a true hit returns an already-complete future), so a caller that coalesces onto an in-flight cache miss is not miscounted as a cache hit. Co-authored-by: Claude Opus 4.8 <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.
Adds byte-level throughput metrics for the diskless fetch path:
The three consolidation meters are recorded only on the paths that respond from the synchronous local-log read; the delayedResponse path discards that read and re-reads (and re-issues the supplement) inside DelayedFetch, which records there, so marking in both would double-count.
CacheHitBytesPerSec is attributed from the future's completion state at lookup (a true hit returns an already-complete future), so a caller that coalesces onto an in-flight cache miss is not miscounted as a cache hit.