Repository navigation
Conversation
7cc7c1e to
edb0ed3
Compare
edb0ed3 to
33f6773
Compare
33f6773 to
2f0fd74
Compare
2f0fd74 to
69cfb34
Compare
69cfb34 to
51bbf8a
Compare
…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) <[email protected]>
51bbf8a to
4f02fc3
Compare
4f02fc3 to
099cda1
Compare
099cda1 to
be74eb0
Compare
be74eb0 to
4daa449
Compare
4daa449 to
ba882e6
Compare
ba882e6 to
0d948f9
Compare
0d948f9 to
fc3dd5a
Compare
6e98212 to
4b255fe
Compare
4b255fe to
0141e18
Compare
0141e18 to
4ec2990
Compare
4ec2990 to
03e5a1d
Compare
03e5a1d to
d18f90c
Compare
d18f90c to
9e36a27
Compare
9e36a27 to
a6ac11d
Compare
a6ac11d to
199940a
Compare
199940a to
5bf74b6
Compare
5bf74b6 to
cb60114
Compare
cb60114 to
ec4c4c9
Compare
QA LLM Review1. MEDIUM -- divergent-dataflow-cancellation.td can no longer see the peek dataflow it cancels
With the flag now on in testdrive, this test's DetailsA non-constant INSERT becomes a read-then-write (
The interactive runtime also has a drop path of its own that this check would otherwise guard. The PR discussion already attributes the missing interactive introspection to CPU-222. Until that lands, two options keep the test honest. One is to pin |
|
Re the QA review on Posted by Claude Code |
bosconi
left a comment
There was a problem hiding this comment.
The flag flip itself is scoped correctly; two coverage gaps and a CI timeout question inline.
| VariableSystemParameter( | ||
| "enable_compute_interactive_runtime", | ||
| "true", | ||
| ["true", "false"], | ||
| ), |
There was a problem hiding this comment.
Like enable_upsert_chunked_stash above, say why this is on in CI and off in production, and that it only affects newly provisioned replicas.
| VariableSystemParameter( | |
| "enable_compute_interactive_runtime", | |
| "true", | |
| ["true", "false"], | |
| ), | |
| # On by default so CI exercises the interactive runtime, which is off in | |
| # production while it earns trust. Read when a replica is provisioned, so | |
| # only replicas created after the setting lands are affected. | |
| VariableSystemParameter( | |
| "enable_compute_interactive_runtime", | |
| "true", | |
| ["true", "false"], | |
| ), |
| # parameter, so unorchestrated clusterds run the same topology as | ||
| # controller-provisioned replicas. Old images ignore the env var. | ||
| unified_cluster: bool = True, | ||
| interactive_compute: bool = False, |
There was a problem hiding this comment.
Unmanaged replicas ignore the flag (clusters.rs#L467-L479), and only test/clusterd-test-driver passes interactive_compute=True, so platform-checks (UseClusterdCompute), test/cluster, test/cluster-isolation, test/replica-isolation and test/bounded-memory stay single-runtime. Please state that scope in the description, and consider opting at least one compute-focused composition in.
| for name in c.workflows: | ||
| if name == "default": | ||
| continue | ||
| with c.test_case(name): | ||
| c.workflow(name) |
There was a problem hiding this comment.
The default workflow now adds ten clusterd runs, but the nightly step still has timeout_in_minutes: 20. Has a nightly run on this head confirmed headroom, or should the timeout go up here?
| let name = opt_string(args, "name"); | ||
| let as_of = req_u64(args, "as-of")?; | ||
| let optimize = flags.iter().any(|f| f == "optimize"); | ||
| let single_read = flags.iter().any(|f| f == "single-read"); |
There was a problem hiding this comment.
Nothing asserts single-read parses to true, and the specs would still pass if it were dropped, because the maintenance runtime would serve the same rows. Add a parse check like the optimize one in parses_create_dataflow_with_mir:
// The `single-read` flag on the directive line is picked up.
let single_read = parse_command(
"create-dataflow name=r as-of=0 single-read\n import index=1001\n build id=2000\n Get u1000\n export index=2001 on=2000 key=[0]",
)
.unwrap();
assert!(matches!(
single_read,
Command::CreateDataflow { single_read: true, .. }
));Co-Authored-By: Claude Opus 5.5 <[email protected]>
10 of 14 in the interactive-runtime stack. Stacks on #40558, followed by #38676. Tracked by CPU-216. The design document is
doc/developer/design/20261107_interactive_runtime.mdin #40559.enable_compute_interactive_runtimemoves out ofUNINTERESTING_SYSTEM_PARAMETERS, where #38391 parked it, and becomes a variable system parameter defaulting to on. That is what makes every preceding piece reachable: sqllogictest, testdrive, and the mzcompose suites provision replicas with two runtimes. Parallel-workload'sFlipFlagsActionis unpinned to both values. Production keeps the dyncfg's own default, off.Four clusterd-test-driver specs join the two-runtime workflow #40558 added, which runs every spec with one and two workers, so the registry's per-worker pairing is exercised rather than assumed: an index read, a query dataflow scheduled before the index it imports has produced anything, a read through an index that re-exports another's arrangement under a publication point of its own, and a query dataflow over such a re-export.
create-dataflowtakes asingle-readflag, which setsuntilone past theas_ofand the class toOneShotRead, as the adapter does.divergent-dataflow-cancellation.tdkeeps the flag off, because its INSERT's read would render on the interactive runtime, whose work is not yet visible in introspection (CPU-222).read_at_max_timestamp.sltcovers reads atTimestamp::MAX, the one read whoseuntilis empty, so the adapter leaves its dataflow a maintained one, which the maintenance runtime renders. Such a read is answerable only against a collection that has sealed, since no finiteupperpassesMAX, so the file reads off constant-backed indexes, whose dataflows have no inputs and drop their capability after emitting once. Both shapes are covered: a fast-path index peek, served by the interactive runtime over shared indexes, and a join that cannot take the fast path and so builds a dataflow. The reads double as the assertion that those indexes sealed, since a peek finding a live frontier would never return.With this PR,
ReadIsolationUnderHydration(#38542) measures peek latency on a two-runtime replica while hydration saturates the maintenance workers.A reviewer should check that the test suites need no expectation changes beyond the two files above.
Posted by Claude Code.
🤖 Generated with Claude Code