Skip to content

Relocate batch merging traits into the fueled spine - #841

Merged
frankmcsherry merged 2 commits into
master-nextfrom
merger-into-spine
Aug 24, 2026
Merged

Relocate batch merging traits into the fueled spine#841
frankmcsherry merged 2 commits into
master-nextfrom
merger-into-spine

Conversation

@frankmcsherry

@frankmcsherry frankmcsherry commented Aug 21, 2026

Copy link
Copy Markdown
Member

What

Moves the Merger trait and the Batch::Merger associated type out of trace/mod.rs and into spine_fueled.rs, as a spine-local trait:

  • SpineBatch: Batch (in spine_fueled.rs) associates a batch type with its Merger. The Rc forwarding (RcMerger, impl SpineBatch for Rc<B>) moves alongside it.
  • Batch (in trace/mod.rs) retains only empty, which has a consumer outside the spine: TraceWriter::seal mints empty batches to pad otherwise empty intervals of time, so empty-minting is a property of writable traces generally, not a spine opinion.
  • Implementors (OrdValBatch, OrdKeyBatch, ChunkBatch) split into a one-method impl Batch and a separate impl SpineBatch.

Batch::begin_merge does not survive the move. It and Merger::new were two entry points to the same construction, and both implementors overrode begin_merge with the trait's own default body. Only Merger::new remains; the documentation of what beginning a merge means moves onto Merger. The spine's own MergeState::begin_merge, which handles the structurally-empty cases, is unaffected.

No behavior changes; all other edits are path and name updates (including three test modules).

Why

An audit showed the merge machinery had exactly one consumer: the fueled spine. Different trace maintenance strategies want differently shaped merge contracts (compare merge_batcher's own Merger, which owns its input chains), so the progressive-merge signature is an opinion of this spine rather than a property of batches in general. Relocating it makes later changes to that opinion — e.g. merge state that owns its inputs and releases them incrementally, to avoid the 2x memory spike during merges of chunk batches — local to the spine instead of revisions to the common batch contract.

Verification

cargo build --workspace (and --examples), cargo test --workspace --lib, and cargo test -p differential-dataflow --tests all pass; cargo doc introduces no new warnings.

🤖 Generated with Claude Code

The Merger trait and the Batch::Merger associated type were consumed only
by spine_fueled.rs, but lived on the common Batch trait, obliging every
batch type to carry this one spine's merge opinion. Move them into the
spine as a spine-local trait, SpineBatch: Batch, along with the Rc
forwarding (RcMerger).

The common Batch trait retains only `empty`, which has a consumer outside
the spine: TraceWriter::seal mints empty batches to pad otherwise empty
intervals of time. Implementors (OrdValBatch, OrdKeyBatch, ChunkBatch)
split their impls accordingly.

No behavior changes; call sites are path and name updates only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`SpineBatch::begin_merge` and `Merger::new` were two entry points to the same
construction, and the two implementors overrode `begin_merge` with the trait's
own default body. Keep `Merger::new` and drop `begin_merge`; `SpineBatch` is now
just the association of a batch type with its merger, and the documentation of
what starting a merge means moves to `Merger` itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@frankmcsherry
frankmcsherry merged commit b22182a into master-next Aug 24, 2026
6 checks passed
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