Skip to content

spine: expose in-progress merges for memory accounting - #852

Draft
antiguru wants to merge 1 commit into
TimelyDataflow:masterfrom
antiguru:spine-merger-visibility
Draft

spine: expose in-progress merges for memory accounting#852
antiguru wants to merge 1 commit into
TimelyDataflow:masterfrom
antiguru:spine-merger-visibility

Conversation

@antiguru

@antiguru antiguru commented Sep 1, 2026

Copy link
Copy Markdown
Member

A consumer that accounts for a spine's memory footprint by walking TraceReader::map_batches undercounts it. For a layer mid-merge, that method presents the merge's two input batches and stops, but the layer holds a third allocation: the merger's partially assembled output. Merger::new sizes that output's containers with BatchContainer::merge_capacity, so the allocation appears in full the moment the merge begins, and stays invisible for the merge's whole duration.

This adds three accessors so a caller can reach that storage.

  • Spine::map_mergers applies a closure to the merger of each layer currently mid-merge. It is an inherent method rather than a TraceReader one because TraceReader has no Merger associated type, and its Batch is bounded by BatchReader rather than Batch, so the merger type is not nameable at that level.
  • OrdValMerger::result and OrdKeyMerger::result borrow the storage being assembled. OrdValBuilder::result is already public, so this exposes nothing new in kind.
  • RcMerger::inner borrows the wrapped merger. Without it, map_mergers on the default Rc-backed spines yields an opaque wrapper and the new method is unusable.

No existing signature, trait, or behavior changes. The whole diff is 44 added lines.

Motivation

Materialize reports per-arrangement heap size, capacity, and allocation counts as introspection relations and Prometheus metrics, computed by summing over map_batches. Because in-progress merges are invisible, those numbers understate a merging arrangement.

Measured on a one-million-row index across 87 observations, comparing what map_batches alone reports against map_batches plus map_mergers:

true/reported size (bytes written) capacity (bytes reserved)
median 1.54x 1.83x
p90 1.95x 1.97x
max 2.53x 2.55x

The undercount grows with merge progress, from a median 1.07x while the merge output is under a quarter full to 1.91x once it is over three quarters full. That is the wrong shape: the accounting is accurate while a merge is barely started and wrong by nearly a factor of two once the output is nearly built, which is also when the input batches are still resident and the arrangement's real footprint peaks. At the peak observation, 30.3 MB were reported against 59.0 MB actually written.

Ratios can exceed 2x because a container sizes its merged reservation from the inputs' record counts rather than from the inputs' own allocations.

Notes for review

  • A merger's contents change as the merge proceeds, so a caller must re-read it on every observation rather than cache the result the way it can for a sealed batch. The map_mergers doc comment says so, since it is the one way this API is easy to misuse.
  • OrdValMerger::staging (an UpdsBuilder) remains inaccessible. It holds one key/value pair's worth of times and diffs, bounded by the maximum multiplicity of a single pair, which is negligible against the output. Exposing it would need further accessors on UpdsBuilder, so it is deliberately left out.
  • An alternative shape would have been for the spine to report sizes itself, extending the existing BatcherEvent (which already carries size, capacity, and allocation deltas) with a spine-level equivalent. That needs a heap_size notion on BatchContainer and an implementation for every container, and it duplicates caching that consumers already have. These accessors are the smaller change.

Draft because it is paired with an unmerged Materialize change that consumes it.

🤖 Generated with Claude Code

A consumer that accounts for a spine's memory footprint by walking
`TraceReader::map_batches` undercounts it. For a layer mid-merge, that method
presents the merge's two input batches and stops, but the layer holds a third
allocation: the merger's partially assembled output. `Merger::new` sizes that
output's containers with `BatchContainer::merge_capacity`, so the allocation
appears in full the moment the merge begins and is invisible for the merge's
whole duration.

Add `Spine::map_mergers`, which applies a closure to the merger of each layer
that is currently mid-merge. It is an inherent method rather than a `TraceReader`
one because `TraceReader` has no `Merger` associated type, and its `Batch` is
bounded by `BatchReader` rather than `Batch`, so the merger type is not nameable
at that level.

Reaching a merger is only useful if its contents can be inspected, so also add
`OrdValMerger::result` and `OrdKeyMerger::result`, which borrow the storage being
assembled, and `RcMerger::inner`, without which `map_mergers` on the default
`Rc`-backed spines yields an opaque wrapper.

All three additions are accessors. No existing signature, trait, or behavior
changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant