Repository navigation
compute: consolidate through the buffer pool - #38900
Merged
Merged
Conversation
antiguru
force-pushed
the
consolidate-pool
branch
3 times, most recently
from
September 17, 2026 16:30
692ecd4 to
52eedeb
Compare
`columnar_consolidate` batched through `ColumnMergeBatcher`, the columnar merge batcher that routes its chains through `mz_timely_util::column_pager` and the pager underneath it. It now batches through the chunk seam instead: `ChunkChunker` feeds `AccountedChunkBatcher`, whose chains are `ColumnChunk`s that the process buffer pool spills while the chunk spill gate is set. The flatten step that follows loads a chunk's body on its way into the output session, so a spilled chain costs one read per chunk and nothing is visited per record. Unlike the arrange sites, this path carries no selector. It has two call sites, and dataflows reach both without asking: `UnionConsolidation`, which lowering picks for a union with a negated input, and `LetRecConsolidation`, which every recursive binding runs to detect its fixed point. Spilling stays gated, so the change is a resident batcher swap until `enable_column_paged_batcher_spill` is set, and the batcher's memory reaches the `mz_arrangement_batcher_*_raw` introspection tables either way, which the pager-backed batcher also did. Tests: `columnar_consolidate_spills_and_round_trips` drives a payload past the pool's 64 KiB spill floor, asserts from the pool's own counters that bodies got there, and checks every row back out. Release notes: No user-facing changes. Co-Authored-By: Claude Opus 5 <[email protected]>
antiguru
force-pushed
the
consolidate-pool
branch
from
September 17, 2026 21:08
52eedeb to
9f174a0
Compare
antiguru
marked this pull request as ready for review
September 25, 2026 13:24
DAlperin
approved these changes
Sep 25, 2026
DAlperin
left a comment
Member
There was a problem hiding this comment.
Seems fine since it defaults off, excited to test it though
Member
Author
|
Thanks for the review! |
antiguru
added a commit
that referenced
this pull request
Oct 1, 2026
`Column` served two roles. On a dataflow edge it is built by pushing, crosses an exchange as channel bytes, and is read on arrival, which is what its `Typed`, `Bytes`, and `Align` variants are for. Behind an edge, in a merge batcher's chains, a spill-backed `ColumnChunk`, and a batch builder's seal, the same type held sorted, consolidated bodies that were only ever typed or, when read back from a store, serialized words. That code never constructed `Bytes`, and it asserted the variant it expected at seven sites with `unreachable!`. `ColumnBody<C>` is the type at rest: `Typed(C::Container)` while a body is written, `Words(Vec<u64>)` once a store handed it back, and nothing else. Writing to a serialized body materializes it by a bulk per-leaf copy, so the merge, extract, advance, and settle bodies address the typed containers without matching on a variant. `ColumnChunk`, `ColumnChunker`, `ColumnMerger`, `UnchunkBuilder`, `RowRowColPagedBuilder`, and the correction buffer's chunks and builder now hold bodies. The edge container keeps its name and every operator that names it is untouched. The boundaries are moves. A chunker takes whatever the edge delivered and emits bodies, and a body goes back onto an edge through `From`, typed staying typed and words becoming `Align`. The column pager's batcher and `ValRowColPagedBuilder` keep `Column` chains until the pager is retired, so the merge and extract bodies are generic over a `MergeChunk` trait both types implement, and `PagedChunker` wraps the chunker for that batcher. Fifth in the stack, on top of #38897. The chunked consolidation from #38900 hands its bodies back onto the edge through `From`. #38334 touches the same files and will need rebasing onto this, since edge paging lands on `Column` alone, which is the point of the split. Part of [CPU-279](https://linear.app/materializeinc/issue/CPU-279). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <[email protected]>
DAlperin
added a commit
that referenced
this pull request
Oct 7, 2026
### Motivation `ColumnChunk` read one process-wide spill gate: the OR of a compute leg (`enable_column_paged_batcher_spill`) and a storage leg (`enable_upsert_paged_spill`). Compute and storage dataflows share a `clusterd` process and its worker threads, so either flag spilled every `ColumnChunk` in the process, whichever subsystem owned it. That became visible when columnar consolidation (`UnionConsolidation`, `LetRecConsolidation`) moved onto `ColumnChunk` (#38900). In production, `enable_upsert_paged_spill` is on by default, so union and recursive consolidation started spilling through the buffer pool even though compute's spill flag is off. The freed slots stay on the pool's warm list, and that shows up as `mz_column_pool_warm_bytes` growth on the v26.45.0 canaries ([CPU-280](https://linear.app/materializeinc/issue/CPU-280)). It also means compute and storage spilling cannot be turned on or tested independently. ### Change The gate is now a zero-sized type parameter: `ColumnChunk<D, T, R, G>` with `G: SpillGate`. * `ComputeSpill` reads compute's flag, and `StorageSpill` reads storage's. * Every consumer names its gate explicitly. There is no default, so a new consumer has to pick one. Compute's arrange, consolidate, and temporal-bucket batchers use `ComputeSpill`. The upsert-v2 chunked stash and feedback arrangement use `StorageSpill`. * The gate type lives on `SpilledBody`, so the `Resident` variant and its match arms are unchanged. * `G::enabled()` is resolved at compile time, so a commit reads one relaxed atomic where it used to read two. * The MV sink correction buffer's gate (`try_spill_ref`) is unchanged. * No flag changes. `enable_upsert_paged_spill` still installs the buffer pool, but it now only spills upsert's chunks. ### Tests `spill_gates_compose` in `mz_timely_util::columnar::chunk` is replaced by `spill_gates_are_independent`, which checks that each of the compute, storage, and sink gates spills only its own chunks. It fails if `ComputeSpill` reads the storage gate. ### Release notes None. No user-visible change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
columnar_consolidatebatched throughColumnMergeBatcher, the columnar merge batcher that routes its chains throughmz_timely_util::column_pagerand the pager underneath it. It now batches through the chunk seam instead:ChunkChunkerfeedsAccountedChunkBatcher, whose chains areColumnChunks that the process buffer pool spills while the chunk spill gate is set. The flatten step that follows loads a chunk's body on its way into the output session, so a spilled chain costs one read per chunk and nothing is visited per record.Unlike the arrange sites, this path carries no selector. It has two call sites, and dataflows reach both without asking:
UnionConsolidation, which lowering picks for a union with a negated input, andLetRecConsolidation, which every recursive binding runs to detect its fixed point.Spilling stays gated, so the change is a resident batcher swap until
enable_column_paged_batcher_spillis set, and the batcher's memory reaches themz_arrangement_batcher_*_rawintrospection tables either way, which the pager-backed batcher also did.This removes one of the three remaining
mz_timely_util::column_pagerconsumers. The two left are the MV sink's correction buffer and storage's paged upsert stash flavor; the pager and its tiered budget stay for them.columnar_consolidate_spills_and_round_tripsdrives a payload past the pool's 64 KiB spill floor, asserts from the pool's own counters that bodies got there, and checks every row back out.columnar_consolidate_accumulates_and_cancelscovers the flatten path without spilling.Release notes: No user-facing changes.
🤖 Generated with Claude Code