Skip to content

compute: publish maintained indexes into the sharing registry - #38389

Open
antiguru wants to merge 1 commit into
mh/interactive-03-multiplexfrom
mh/interactive-04-publish
Open

compute: publish maintained indexes into the sharing registry#38389
antiguru wants to merge 1 commit into
mh/interactive-03-multiplexfrom
mh/interactive-04-publish

Conversation

@antiguru

@antiguru antiguru commented Aug 21, 2026

Copy link
Copy Markdown
Member

Fourth of eight PRs splitting #37770. Stacks on #38388. Tracked by CPU-215.

Both export paths publish their oks/errs arrangements into the per-process registry when the runtime's role publishes. A re-export arm, where an index reuses another index's arrangement, has no streams of its own, so it registers its id as an alias of the arrangement's existing publication point, which leaves the re-export's dataflow without operators, as it is on a runtime that does not publish. A reader that bound the re-export's id before the render holds its own unbacked point, which only a publisher into it can back, so that case re-imports the shared traces and publishes them under the new id, and logs the imported errors because mz_compute_error_counts forwards a dependency's counts only to a re-export whose dataflow has no operators. Logging indexes publish the same way, gated strictly on Maintenance: an interactive runtime reads maintenance's slot and its own copy would clobber it, while Solo has no registry peer.

An alias shares its target's frontiers while the target lives: the alias dataflow imports the target, so the controller never advances the target's since past an alias's, and the target's frontier bounds every reader of the shared point. Once the target drops, the meet of the remaining aliases' frontiers governs the point, since the shared trace then compacts to exactly that meet. Seal notifications fan out from the target to its aliases, since a reader waits under the id it imported.

ComputeRuntimeRole::Interactive stops being test-only, because publishes() has to name it. pub mod server keeps the variant reachable, so no #[allow] is needed even though nothing constructs it yet.

No behavior change: Solo is the only role anything constructs and it does not publish, so every added block is skipped and no dataflow gains an operator. That is also why no goldens move here. They move in the last PR of the stack, which turns the flag on in CI.

@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from d31414b to d9ddff4 Compare August 21, 2026 11:23
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch 2 times, most recently from d754ecc to 1303035 Compare August 21, 2026 13:42
@antiguru
antiguru requested a review from DAlperin August 21, 2026 13:46
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 1303035 to dffb179 Compare August 21, 2026 14:31
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from dffb179 to 7d70e54 Compare August 21, 2026 17:54
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 7d70e54 to 6e430c4 Compare August 28, 2026 14:06
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 6e430c4 to 8ba6dce Compare September 3, 2026 08:48
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 8ba6dce to 3f24fa0 Compare September 3, 2026 15:58
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 3f24fa0 to d3a7b3d Compare September 4, 2026 15:57
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from d3a7b3d to 188d168 Compare September 4, 2026 17:38
@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from 188d168 to aac9441 Compare September 4, 2026 19:37
@antiguru
antiguru marked this pull request as ready for review September 4, 2026 19:39
@antiguru
antiguru requested a review from a team as a code owner September 4, 2026 19:40
@antiguru
antiguru requested a review from petrosagg September 4, 2026 19:40
@def-

def- commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- The re-export arm overwrites a registry slot an interactive import has already bound to

src/compute/src/render.rs:801

ArrangementSharingRegistry::reexport assigns slots[worker_index] = Some(arr) unconditionally (src/compute/src/sharing.rs:161), so when the interactive runtime has already created a placeholder for idx_id and built its import over it, that placeholder is orphaned. Nothing ever adopts it, its chain stays empty and its upper stays at minimum, so the importing dataflow never produces and the peek behind it hangs until cancelled.

Details

Every other publish path goes through get_or_create + PublishArrangement::adopt precisely so that a point a reader has already imported is backed in place "rather than being overwritten by a second, disconnected arrangement" (sharing.rs:111). The comment on the new call site claims that same property for the alias, but reexport does not have it.

The reader binds once and never re-resolves: import_shared_index takes slot = registry.get_or_create(idx_id, ..), mints slot.oks/slot.errs handles, builds import_snapshot_at over them and keeps the Arc in tokens for the life of the dataflow. A later map replacement is invisible to it.

The ordering that triggers this is the one get_or_create is built for ("whichever side ... touches id first creates the slot"): a peek dataflow routed to interactive renders as soon as it arrives, while maintenance's own CreateDataflow for idx_id may still be queued behind other render work. Reaching the re-export arm at all needs only a second index on an already-indexed key, e.g. CREATE INDEX i2 ON t (k) while i1 ON t (k) exists: i2's dataflow imports i1 and re-exports its arrangement.

Either make reexport fill only an unoccupied slot (and report loudly otherwise, since silently orphaning a live reader is worse than a panic), or resolve the alias on the read side so an importer of idx_id reaches gid's slot rather than a second map entry that can be swapped underneath it.

2. MEDIUM -- Aliasing two ids onto one publication point makes them share per-collection compaction state, refusing reads the arrangement can still serve

src/compute/src/render.rs:801

reexport stores the same Arc<SharedIndexArrangement> under gid and idx_id, so the two collections share one Published/SharedTrace. The controller's compaction frontier is per collection, but the registry funnels both ids into that one point: note_writer_logical assigns (shared_trace/publish.rs:111) so the last id the controller sent wins, and advance_standing_hold joins (shared_trace/state.rs:158) so the standing hold becomes the max of the two. When the two frontiers diverge, the publisher's since can settle at the faster id's frontier, and a read on the slower id at a timestamp the arrangement still holds is refused.

Details

The refusal surfaces two ways. On the fast path, the peek's handle is registered at the published since, gate_peek compares it to peek.timestamp and returns PeekResponse::Error("Arrangement compaction frontier ... is beyond the time of the attempted read") -- a spurious user-visible error on a valid read. On the slow path, import_shared_index's handle_at(as_of) fails and report_compacted_past panics the replica.

The underlying trace is not actually over-compacted: compute_state.traces.set(idx_id, trace) clones the TraceBundle, so gid and idx_id keep distinct TraceAgents and the TraceBox meet still protects the data. That is why this shows up as a false refusal rather than wrong results, and why it cannot be caught by watching for corruption.

Divergence between two indexes on the same key is ordinary: a freshly created duplicate starts at its create as_of while the original sits a compaction lag behind, and a read hold (a SUBSCRIBE, an explicit AS OF) on one and not the other keeps them apart for as long as it is held. writer_logical then flips between the two ids' frontiers as the controller's AllowCompaction messages interleave, so the failure is intermittent rather than sticky.

The publication point's writer_logical and standing_hold are properties of a collection, not of an arrangement. If two collections are going to share one point, those two fields need to be tracked per id and combined by meet, not overwritten and joined.

@antiguru

antiguru commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Both confirmed and fixed in aea40da, together with the registry change in #38387 (0225ab1).

The re-export arms no longer alias gid's slot. They re-import the shared traces (TraceBundle::import_named) and publish them under idx_id through ArrangementSharingRegistry::publish, which adopts the slot rather than replacing it. A placeholder an interactive import already bound to is therefore backed in place, and each id carries its own writer frontier and standing hold.

Posted by Claude Code.

@antiguru
antiguru force-pushed the mh/interactive-04-publish branch from aea40da to 5911208 Compare September 5, 2026 11:47
Both export paths now publish their `oks`/`errs` arrangements into the per-process
registry when the runtime's role publishes. A re-export arm has no streams of its
own, so it registers its id as an alias of the arrangement's existing publication
point, which leaves the re-export's dataflow without operators, as it is on a
runtime that does not publish. A reader that bound the re-export's id before the
render holds its own unbacked point, which only a publisher into it can back, so
that case re-imports the shared traces and publishes them under the new id, and
logs the imported errors because `mz_compute_error_counts` forwards a dependency's
counts only to a re-export whose dataflow has no operators. Logging indexes
publish the same way, gated strictly on `Maintenance`: an interactive runtime
reads maintenance's slot, and its own copy would clobber it, while `Solo` has no
registry peer at all.

An alias shares its target's frontiers while the target lives: the alias dataflow
imports the target, so the controller never advances the target's `since` past an
alias's, and the target's frontier bounds every reader of the shared point. Once
the target drops, the meet of the remaining aliases' frontiers governs the point,
since the shared trace then compacts to exactly that meet. Seal notifications fan
out from the target to its aliases, since a reader waits under the id it imported.

`ComputeRuntimeRole::Interactive` stops being test-only. Nothing constructs it
yet, but `publishes()` has to name it, and `pub mod server` keeps the variant
reachable so dead-code analysis is satisfied without an attribute. The stale
`owns_process_globals` note claiming every constructible role owns the globals
goes with it.

Carrying the role and the registry to the render path is what the rest of this
change is: `Config` and `Worker` gain both, `ComputeState` stores them and
exposes `role()`, and clusterd builds one registry per process. Per process, not
per runtime, because a reader on one runtime looks up the slot a publisher on
another filled.

No behavior change. `Solo` is the only role anything constructs and it does not
publish, so every added block is skipped and no dataflow gains an operator.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VDm7opomJLxbNUEP3r9BLk

antiguru commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Revised in 6d76ee5: the re-import per re-export cost 125 to 185 KB of clusterd memory each (five operators and a trace listener per re-export, measured in the nightly's ManyReexportsIdle), so re-exports alias again, with both findings addressed in the registry rather than in the render arm.

  1. publish_alias fills only an unoccupied slot. When a reader already created the alias's slot, it returns false and the render arm falls back to re-importing and publishing under the new id, which backs that reader's point in place.
  2. While the target lives, only its AllowCompaction and standing-hold notes reach the shared point. That is sound because the alias dataflow imports the target, so the controller never advances the target's since past an alias's. Once the target drops, the point takes the meet of the remaining aliases' noted frontiers. Tests: alias_refused_once_a_reader_holds_its_own_point, alias_frontiers_follow_the_target_then_the_aliases_meet.

Posted by Claude Code.

@def-

def- commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- publish_alias decides per worker but records the alias relation process-wide, so a worker that falls back to publishing never receives a frontier note

src/compute/src/sharing.rs:254

publish_alias aliases or falls back per (id, worker), but it writes target_of/aliases_of with no worker ordinal. When the reader race resolves differently on two workers, the worker that fell back owns a real, independent publication point for idx_id, yet Aliases::note still routes it to the target and returns None, so that point never receives note_allow_compaction or note_standing_hold. Its standing hold stays at the seed adopt installed, which pins the target's trace and stops that arrangement compacting on that worker for as long as the re-export lives.

Details

The slot check is per worker (sharing.rs:249): publish_alias returns false only when this worker's slot for alias is already taken, which is what happens when the interactive reader ran get_or_create(idx_id, worker, peers) before maintenance rendered the re-export. Interactive and maintenance workers are independent threads racing on independent slots, so worker 0 can alias while worker 1 falls back to registry.publish (render.rs:909-916).

allowed and holds are correctly keyed (GlobalId, usize) (sharing.rs:97-99), but target_of/aliases_of are not. On the fallback worker Aliases::note computes target = target_of[idx_id] = gid, finds gid live, and returns None because id != target (sharing.rs:121-124). So the fallback point's writer_logical stays None and its standing_hold stays at the initial_logical seeded in adopt. The publisher forwards that accumulated hold to its agent on every activation (shared_trace/publish.rs:270-278, :317), so the meet over the trace's agents never rises above the frontier at which the re-export was rendered. Reads stay correct, since the published since is pinned just as low, but the target index's arrangement grows without bound on that worker.

The global relation also misdirects the remove handover: with one fallback alias and one true alias on the same worker, remaining.iter().find_map(...) (sharing.rs:289-295) can apply the aliases' meet to the fallback's private point and leave the genuinely shared point untouched.

A milder consequence of the same split: the fallback adds import and publisher operators that outlive the dropped import token, so the re-export's dataflow no longer empties out and stays in mz_dataflows. mz_compute_error_counts_per_worker derives index_reuses from mz_dataflows, which is worker 0 only, so if worker 0 falls back while the others alias, forwarding from the dependency is suppressed and only worker 0 ever logged direct counts, leaving the reported error count at one worker's share.

Fix: key target_of and aliases_of by (GlobalId, usize) the way allowed and holds already are, so the relation is recorded exactly where publish_alias established it and a fallback worker keeps being its own target.

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.

2 participants