Skip to content

row-spine: back production spines with Arc for cross-thread arrangement sharing - #37881

Closed
antiguru wants to merge 2 commits into
MaterializeInc:mainfrom
antiguru:claude/spines-differential-arc-j93mho
Closed

row-spine: back production spines with Arc for cross-thread arrangement sharing#37881
antiguru wants to merge 2 commits into
MaterializeInc:mainfrom
antiguru:claude/spines-differential-arc-j93mho

Conversation

@antiguru

@antiguru antiguru commented Jul 24, 2026

Copy link
Copy Markdown
Member

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 is
worker-local.

Description

Introduce mz_row_spine::ArcBatch, a local newtype around Arc<B> that carries
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 across threads, which Rc cannot do. Only the
batch handle becomes atomic; the batch contents are unchanged, so the cost is a
marginally more expensive refcount.

Also adds generic ArcOrdVal/ArcOrdKeySpine aliases for callers outside
mz_compute, adapts batch-size logging (log_arrangement_size_inner) to reach
through the newtype to the inner Arc, and switches the storage sink trace to
the 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 --workspace on top of main passes (6 files changed, no
Cargo.lock churn, no other crate affected by the RcArc switch).

🤖 Generated with Claude Code

Comment thread src/compute/src/extensions/arrange.rs
Comment thread src/row-spine/src/lib.rs Outdated
@antiguru
antiguru force-pushed the claude/spines-differential-arc-j93mho branch 2 times, most recently from 5f6acf4 to baf3e14 Compare August 17, 2026 08:56
antiguru added a commit that referenced this pull request Aug 19, 2026
### Motivation

The two-runtime read-isolation work (#37770) runs two compute runtimes
in a
single clusterd process, sharing one metrics registry. Without
per-runtime
labels the two runtimes register the same metric series and collide.
This PR
adds the role concept and the metric labeling on its own, so that the
two-runtime PR is left with only the runtime wiring.

### Description

Introduce `ComputeRuntimeRole` (`Solo`, `Maintenance`, `Interactive`) in
`mz_compute::server` and thread a role through `serve` into
`ComputeMetrics::register_with`. Each named role stamps a distinct
`role`
const label on its metrics, so two runtimes sharing one process registry
register distinct series rather than colliding. The whole-registry
`workload_class` postprocessor is gated on the globals-owning role so it
is
installed exactly once.

`Solo` is the single-runtime default and is behaviorally identical to
compute
before a second runtime existed: it emits no `role` label, so
exact-match
dashboards and alerts are byte-unchanged, and it owns the process
globals as
the sole runtime always has. Every call site passes `Solo` here.

Two unit tests assert the two halves of the contract: `Solo` metrics
carry no
`role` label, and the two named roles carry distinct labels so they
coexist on
one registry.

### Context

Prerequisite for #37770, which flips the call sites to `Maintenance` and
`Interactive` and adds the runtimes that use them. Independent of the
other
#37770 prerequisites (#37880, #37881).

### Verification

`cargo check -p mz-compute -p mz-clusterd` passes with no new warnings.
The two
new `metrics::tests` pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CY7GHdrTBfAG4tgvSgPGJ9

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
antiguru and others added 2 commits August 21, 2026 09:18
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.

Two consumers of the sink trace follow from that switch. The iceberg sink stashes
input batches while it waits for their batch description, so its `VecDeque` and
the `with_ready_batches` helper now hold `ArcBatch` rather than `Rc`. And the
arrangement-size operator's cache comment named the `RcBox` allocation its `Weak`
keeps reserved, which is an `ArcInner` once the batch handle is atomic. The
invariant it documents is unchanged.

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 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019G29DBfgE8LXpE5jamm2Zi
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 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019G29DBfgE8LXpE5jamm2Zi
@antiguru

Copy link
Copy Markdown
Member Author

Superseded by #38396, which carries the same commits from an upstream branch (mh/interactive-00-arc-spines, at the same sha 87c4c306a as this PR's head).

The reason is mechanical: a stacked PR's base has to be a branch in this repository, so #38386 already bases on that upstream mirror. GitHub allows changing a PR's base but never its head, so this PR could not become the stack's root. Closing here and continuing in #38396, which is the root of #38386 through #38393.

Both review comments on this PR were addressed before the move.

(Posted by Claude Code.)

@antiguru antiguru closed this Aug 21, 2026
@antiguru
antiguru deleted the claude/spines-differential-arc-j93mho branch August 21, 2026 13:40
pull Bot pushed a commit to Arstman/materialize that referenced this pull request Aug 21, 2026
…nt sharing (MaterializeInc#38396)

Replaces MaterializeInc#37881, whose head branch lives on a fork and so cannot be the
base of a stacked PR in this repository. Same commits, same tree, on an
upstream branch instead. This is the root of the stack MaterializeInc#38386 through
MaterializeInc#38393, which splits MaterializeInc#37770.

### Motivation

Cross-runtime arrangement sharing (the two-runtime read-isolation work,
MaterializeInc#37770) needs batches readable from a thread other than the one
maintaining the trace. Differential's default spines reference-count
batches with `Rc`, which is worker-local.

### Description

Introduce `mz_row_spine::ArcBatch`, a local newtype around `Arc<B>` that
carries 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 across threads, which `Rc` cannot do. Only the batch
handle becomes atomic; the batch contents are unchanged, so the cost is
a marginally more expensive refcount.

Also adds generic `ArcOrdVal`/`ArcOrdKeySpine` aliases for callers
outside `mz_compute`, adapts batch-size logging
(`log_arrangement_size_inner`) to reach through the newtype to the inner
`Arc`, and switches the storage sink trace to the `Arc`-backed spine.

Builds against released differential-dataflow 0.25 with no fork or
`[patch.crates-io]`.

### Verification

`cargo check --workspace` passes with no `Cargo.lock` churn.
`relations.slt`'s golden is rewritten because the spine type name
appears in operator names.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

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