Skip to content

Relocate batch merging traits into the fueled spine - #841

Draft
frankmcsherry wants to merge 1 commit into
master-nextfrom
merger-into-spine
Draft

Relocate batch merging traits into the fueled spine#841
frankmcsherry wants to merge 1 commit into
master-nextfrom
merger-into-spine

Conversation

@frankmcsherry

Copy link
Copy Markdown
Member

What

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

  • SpineBatch: Batch (in spine_fueled.rs) carries type Merger and begin_merge. 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.

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>
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