Skip to content

Use Batcher to implement all of Arrange - #845

Merged
frankmcsherry merged 3 commits into
master-nextfrom
batcher-seals-batches
Aug 26, 2026
Merged

Use Batcher to implement all of Arrange#845
frankmcsherry merged 3 commits into
master-nextfrom
batcher-seals-batches

Conversation

@frankmcsherry

@frankmcsherry frankmcsherry commented Aug 26, 2026

Copy link
Copy Markdown
Member

arrange_core named three stages of one pipeline: a chunker that melds wire containers into sorted chunks, a batcher that merges chunks and carves by frontier, and a builder that seals a carved chain into a batch.
All three are a sorting batcher's internal strategy.
This PR hides the outer two, leaving Batcher as the only trait arrange knows about.

Chunker and Builder do not go away; they stop being public vocabulary.

The batcher seals its own batchesMergeBatcher carries the builder and seals internally, so extract returns a batch instead of a chain of chunks.
Sealing still happens at extraction with the whole chain in hand, so Builder::with_capacity still pre-sizes from the full key, value, and update counts.

\Batcher`'s time and output become associated types` — the input a batcher accepts stays a parameter; what it produces, and the timestamps it carves by, are facts about the batcher.

-pub trait Batcher<T, C0, C1> {
+pub trait Batcher<C0> {
+    type Time;
+    type Output;

The batcher chunks its own inputMergeBatcher holds the chunker, so insert takes the container from the wire and extract flushes before merging.
That removes the pre-seal drain loop from the operator, along with the comment explaining a seam arrange had no business knowing about.

Net effect on the signature:

-pub fn arrange_core<'scope, P, C, Chu, Ba, Bu, Tr>(
+pub fn arrange_core<'scope, P, C, Ba, Tr>(
-    Chu: ContainerBuilder + for<'a> PushInto<&'a mut C> + 'static,
-    Ba: Batcher<Tr::Time, Chu::Container, Vec<Bu::Input>> + 'static,
-    Bu: Builder<Time=Tr::Time, Output: Into<Tr::Batch>>,
+    Ba: Batcher<C, Time = Tr::Time, Output: Into<Tr::Batch>> + 'static,

One consequence worth flagging: MergeBatcher now implements Batcher<C> for every C its chunker accepts, and extract does not mention C, so a caller holding a concrete batcher must say which implementation it means.
Generic callers pin C in their bounds and are unaffected; the three sites needing a turbofish are tests driving a batcher by hand.
Stating that extraction is independent of the input type would mean putting extract on an un-parameterized supertrait, which seems worth revisiting only if a caller wants per-input extraction.

Builds warning-free, workspace tests pass.

frankmcsherry and others added 2 commits August 26, 2026 10:46
`arrange_core` used to take both a batcher and a builder, pull a chain of
chunks out of the batcher, and call `Bu::seal` on it to get a batch. The
batcher named the batch type in its output, but did not produce one.

Now `MergeBatcher` carries the builder and seals internally, so `extract`
returns a batch. `arrange_core` drops its `Bu` parameter: the bounds go from

    Ba: Batcher<Tr::Time, Chu::Container, Vec<Bu::Input>>,
    Bu: Builder<Time=Tr::Time, Output: Into<Tr::Batch>>,

to

    Ba: Batcher<Tr::Time, Chu::Container, B>,
    B: Into<Tr::Batch>,

Arrange no longer knows that builders exist; it asks for updates and gets
something it can turn into a batch. Call sites stop naming a builder, which
is the bulk of the diff.

The builder is `PhantomData` on `MergeBatcher` because `seal` is an
associated function. Sealing still happens at extraction with the whole
chain in hand, so `Builder::with_capacity` still pre-sizes from the full
key, value, and update counts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The input a batcher accepts stays a parameter, because a batcher may
reasonably accept several shapes of container. What it produces, and the
timestamps it carves by, are facts about the batcher, so they become
associated types. Neither impl in the tree implements `Batcher` twice for one
type, so the parameters were buying no polymorphism.

    -pub trait Batcher<T, C0, C1> {
    +pub trait Batcher<C0> {
    +    type Time;
    +    type Output;

Callers that want to pin the output still can, as half_join does with
`Output = Vec<CMid>`. Callers that only need it convertible say so directly:

    -    Ba: Batcher<Tr::Time, Chu::Container, B> + 'static,
    -    B: Into<Tr::Batch>,
    +    Ba: Batcher<Chu::Container, Time = Tr::Time, Output: Into<Tr::Batch>> + 'static,

That drops a type parameter from `arrange_core`, `Collection::arrange`,
`arrange_named`, and `consolidate_named`, and with it the anonymous `_` that
every call site had to write in its turbofish.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@frankmcsherry
frankmcsherry force-pushed the batcher-seals-batches branch from b5acf83 to f74efcf Compare August 26, 2026 15:03
`arrange_core` used to hold a chunker, push each incoming container into it,
drain the chunks it emitted into the batcher, and drain it again before
sealing so a partial final chunk would not be stranded. That last part needed
a comment explaining a seam the operator had no business knowing about.

`MergeBatcher` now holds the chunker, so `insert` takes the container from the
wire and `extract` flushes before merging. `arrange_core` drops its `Chu`
parameter and ingest becomes `batcher.insert(data)`.

    -pub fn arrange_core<'scope, P, C, Chu, Ba, Tr>(
    +pub fn arrange_core<'scope, P, C, Ba, Tr>(
    -    Chu: ContainerBuilder + for<'a> PushInto<&'a mut C> + 'static,
    -    Ba: Batcher<Chu::Container, Time = Tr::Time, Output: Into<Tr::Batch>> + 'static,
    +    Ba: Batcher<C, Time = Tr::Time, Output: Into<Tr::Batch>> + 'static,

The chunker is not a separate axis from the batcher: it adapts whatever
container is on the wire to the merger's preferred representation, so it is
part of what selects a batcher rather than a tax on everyone downstream. The
columnar batcher pairs with `TrieChunker` and with `ContainerChunker<ColChunk>`
depending on its input, and those are now two batchers rather than one batcher
and two chunkers. Corgi keeps its column-native ingest by naming its chunker
where it names its batcher.

One consequence: `MergeBatcher` implements `Batcher<C>` for every `C` its
chunker accepts, and `extract` does not mention `C`, so a caller holding a
concrete batcher must say which implementation it means. Generic callers pin
`C` in their bounds and are unaffected; the three sites that needed a turbofish
are tests driving a batcher by hand.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@frankmcsherry frankmcsherry changed the title The batcher seals its own batches Use Batcher to implement all of Arrange Aug 26, 2026
@frankmcsherry
frankmcsherry merged commit fd7d13b into master-next Aug 26, 2026
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