Repository navigation
compute: make the correction buffer's metrics O(1) - #38895
Merged
antiguru merged 1 commit intoSep 30, 2026
Merged
Conversation
This was referenced Sep 16, 2026
antiguru
marked this pull request as ready for review
September 16, 2026 18:00
antiguru
force-pushed
the
moritz/correction-metrics-o1
branch
from
September 17, 2026 13:54
3347975 to
001e6cb
Compare
antiguru
force-pushed
the
moritz/correction-metrics-o1
branch
from
September 23, 2026 13:23
001e6cb to
a97c70e
Compare
antiguru
commented
Sep 30, 2026
antiguru
left a comment
Member
Author
There was a problem hiding this comment.
Self-review.
- Bug:
Stage::dropnow goes throughstage_diff(-len, ..). That logschain_created(0)followed bychain_dropped(len), and whenlen == 0it logs nothing. Either way thechain_created(0)fromstage_createdis never balanced, sonet_batchesstays one high for every dropped stage.CorrectionLogger::dropretracts the leftover at teardown, which is why nothing shows. Previously the drop loggedchain_dropped(len). It needs astage_droppedthat restores that event and subtracts the totals. - Nit:
chain_droppeduses an uncheckedfetch_sub, whileadd_signedasserts on underflow. Both should use the same checked path.
🤖 Posted by Claude Code
antiguru
force-pushed
the
moritz/correction-metrics-o1
branch
from
September 30, 2026 13:12
a97c70e to
4c1f1be
Compare
Member
Author
|
Addressed. 🤖 Posted by Claude Code |
antiguru
force-pushed
the
moritz/correction-metrics-o1
branch
from
September 30, 2026 15:44
4c1f1be to
6332981
Compare
`CorrectionV2::update_metrics` walked every chain and every chunk on every operation, from four call sites including insert, so its cost grew with the buffer: 17% of insert at 16.8M updates and rising. An `Accounting` handle, shared by the buffer and by every structure that holds its chains, now carries running record, size, and allocation totals, so a metrics update reads three counters. The totals hook the discipline the introspection logging already kept: a chain is announced once when the buffer takes it and retired once before it is dropped or consumed, so the totals and the logging cannot drift apart. Each chunk caches its serialized size at construction, and each chain maintains the sum as chunks are pushed, so no accounting step pages a chunk in. The reported size is the logical serialized size of the bodies, resident or not. The previous walk reported resident bytes only, which depended on what the pager had done with each chunk since. Residency belongs to the pager, which changes it without telling the buffer. Capacity is reported equal to size, since chunk bodies are exactly sized when minted, and allocations count one per chunk plus the staging vector while it holds one. `metrics_totals_match_walk` checks the totals against a walk after inserts, a partial drain, a since advance, and a consolidation. `logged_chains_balance_on_drop` checks that every chain announced to introspection, the stage's included, is retired when the buffer drops. Co-Authored-By: Claude Fable 5.1 <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
antiguru
force-pushed
the
moritz/correction-metrics-o1
branch
from
September 30, 2026 18:07
6332981 to
b4d3de1
Compare
antiguru
added a commit
that referenced
this pull request
Sep 30, 2026
`compute_correction_v2_chunk_size` sizes `CorrectionV2`'s staging area, not its chunks: chunk bodies are fixed at the columnar ship size. The staging size sets how many updates accumulate before a chain is minted, and every chain minted is a chain some later read has to merge. Raising the default from 8 KiB to 2 MiB, one chunk body's worth, takes insert on a 4.19M-update append workload from 3503 ms to 1897 ms, with drain and peak RSS unchanged. These are medians of three `correction_mem` runs with the rest of the stack applied and no pool installed. The curve is monotone across the range (the new `bench_chunk_size` sweep in the `correction` bench shows it), so the default is bounded only by the per-sink, per-worker cost of the staging vector. The stage counted its fill in tuples of `size_of::<(D, Timestamp, Diff)>()`, which is 40 bytes for a `Row` however wide the row is, so a byte-sized flag bounded neither the staging memory nor the reported size once rows spill out of line. A new `DataBytes` trait on the buffer's data types reports each datum's bytes with the heap included (`Row` via `byte_len`, `DataflowErrorSer` via its serialized length), the stage ships when the staged bytes reach the flag, and its size metrics carry the same figure. `stage_ships_by_bytes` checks that rows of two widths ship after the number of updates their bytes predict. That vector was allocated eagerly at the configured capacity and re-allocated at that capacity on every flush. At 2 MiB per sink and worker that is no longer free, so it now grows on demand and gives its allocation back on flush. The mzcompose and parallel-workload flag tables follow the new default, since CI runs the value listed there rather than the compiled default. The flag's description now says what it sizes. First of four, split out of #38645: this, the O(1) metrics (#38895), the per-chunk borrow hoist (#38896), and the pool substrate (#38897) land in that order. Supersedes the fork PR #38647. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <[email protected]>
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.
CorrectionV2::update_metricswalked every chain and every chunk on every operation, from four call sites including insert, so its cost grew with the buffer: 17% of insert at 16.8M updates and rising. AnAccountinghandle, shared by the buffer and by every structure that holds its chains, now carries running record, size, and allocation totals, so a metrics update reads three counters.The totals hook the discipline the introspection logging already kept: a chain is announced once when the buffer takes it and retired once before it is dropped or consumed, so the totals and the logging cannot drift apart. Each chunk caches its serialized size at construction and each chain maintains the sum, so no accounting step pages a chunk in.
The reported size is the logical serialized size of the bodies, resident or not. The previous walk reported resident bytes only, which depended on what the pager had done with each chunk since; residency belongs to the pager. Capacity is reported equal to size, since chunk bodies are exactly sized when minted, and allocations count one per chunk plus the staging vector while it holds one, so
mz_arrangement_sizeskeeps itscapacityandallocationscolumns for correction buffers.metrics_totals_match_walkchecks the totals against a walk after inserts, a partial drain, a since advance, and a consolidation.logged_chains_balance_on_dropchecks that every chain announced to introspection, the stage's included, is retired when the buffer drops.Supersedes the fork PR #38648.
🤖 Generated with Claude Code