row-spine: back production spines with Arc for cross-thread arrangement sharing#37881
Open
antiguru wants to merge 2 commits into
Open
row-spine: back production spines with Arc for cross-thread arrangement sharing#37881antiguru wants to merge 2 commits into
antiguru wants to merge 2 commits into
Conversation
Introduce `mz_row_spine::ArcBatch`, a local newtype around `Arc<B>` carrying differential's batch traits (the orphan rule forbids the blanket impl on a bare `Arc<B>`), and switch the production spines and their builders — `RowRowSpine`, `RowValSpine`, `RowSpine`, `ValRowSpine`, `ColValSpine`, `ColKeySpine` — from `Rc`/`RcBuilder` to `ArcBatch`/`ArcBuilder`. An `Arc`-backed batch whose contents are `Send + Sync` can be read from a thread other than the one maintaining the trace, which `Rc` cannot do. Only the batch handle becomes atomic; the batch contents are unchanged. Also add generic `ArcOrdVal`/`ArcOrdKeySpine` aliases for callers outside `mz_compute`, adapt batch-size logging (`log_arrangement_size_inner`) to reach through the newtype to the inner `Arc`, and switch the storage sink trace to the `Arc`-backed spine. This is the foundational primitive the two-runtime read-isolation work (MaterializeInc#37770) builds on, extracted here for standalone review. It builds against released differential-dataflow with no fork or `[patch.crates-io]`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CY7GHdrTBfAG4tgvSgPGJ9
The introspection golden prints the Rust type name of each arrangement's batch allocation. Switching the production spines to `ArcBatch` changes that name from `alloc::rc::Rc<OrdValBatch<..>>` to `mz_row_spine::arc_batch::ArcBatch<OrdValBatch<..>>`. Churn only, 42 symmetric type-name substitutions, no operator-graph change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CY7GHdrTBfAG4tgvSgPGJ9
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
1 participant
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.
Motivation
Cross-runtime arrangement sharing (the two-runtime read-isolation work, #37770)
needs batches readable from a thread other than the one maintaining the trace.
Differential's default spines reference-count batches with
Rc, which isworker-local.
Description
Introduce
mz_row_spine::ArcBatch, a local newtype aroundArc<B>that carriesdifferential's batch traits (the orphan rule forbids the blanket impl on a bare
Arc<B>), and switch the production spines and their builders —RowRowSpine,RowValSpine,RowSpine,ValRowSpine,ColValSpine,ColKeySpine— fromRc/RcBuildertoArcBatch/ArcBuilder. AnArc-backed batch whose contentsare
Send + Synccan be read across threads, whichRccannot do. Only thebatch handle becomes atomic; the batch contents are unchanged, so the cost is a
marginally more expensive refcount.
Also adds generic
ArcOrdVal/ArcOrdKeySpinealiases for callers outsidemz_compute, adapts batch-size logging (log_arrangement_size_inner) to reachthrough the newtype to the inner
Arc, and switches the storage sink trace tothe
Arc-backed spine.Builds against released differential-dataflow 0.25 with no fork or
[patch.crates-io].Context
Extracted from #37770 for standalone review; #37770 builds on this.
Verification
cargo check --workspaceon top ofmainpasses (6 files changed, noCargo.lockchurn, no other crate affected by theRc→Arcswitch).🤖 Generated with Claude Code