Skip to content

Commit 293b006

Browse files
antiguruclaude
andcommitted
fixup: bound the shared import by the dataflow's until and alias re-exports
The shared import synthesised its own bound one step past `as_of` instead of passing the dataflow's `until`, and documented the result as serving single-time dataflows only. The maintenance import passes `(as_of, until)` and follows the trace when `until` is empty, and nothing about the shared import needs a different bound. The router already sends only single-time dataflows to the interactive runtime, so the two bounds were equal for every dataflow that reached the import, and the narrowing bought nothing. The variant's single-time note and the `Timestamp::MAX` special case go with it. An export whose arrangement is an imported shared arrangement was `unreachable!`. Nothing in the optimizer promises that a peek dataflow never re-exports its import under the export key, and a panic on either runtime aborts the process. Both export paths now alias the export's id to the imported publication point, the shared analogue of the `Trace` arm. The alias has no `TraceBundle`, so `report_frontiers` reads the frontier through the registry. `import_shared_index` is renamed `import_published_index` so it no longer reads as a permutation of the `import_index_shared` method that calls it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDm7opomJLxbNUEP3r9BLk
1 parent 8ed5a24 commit 293b006

7 files changed

Lines changed: 72 additions & 53 deletions

File tree

‎src/compute/src/compute_state.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,14 @@ impl<'a> ActiveComputeState<'a> {
11311131
traces.oks_mut().read_upper(&mut new_frontier);
11321132
} else if let Some(frontier) = &collection.sink_write_frontier {
11331133
new_frontier.clone_from(&frontier.borrow());
1134+
} else if let Some(upper) = self
1135+
.compute_state
1136+
.sharing_registry
1137+
.published_upper(&id, self.timely_worker.index())
1138+
{
1139+
// An index that re-exports an imported shared arrangement has no trace of its own,
1140+
// only an alias to the published point. See `export_index`.
1141+
new_frontier.clone_from(&upper);
11341142
} else {
11351143
error!(id = ?id, "collection without write frontier");
11361144
continue;

‎src/compute/src/render.rs‎

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ fn report_compacted_past(
598598
/// keeps the trace can downgrade it and the publisher compacts behind a long-lived import.
599599
///
600600
/// Panics if the point's `since` is already beyond `as_of`, see [`report_compacted_past`].
601-
fn import_shared_index<'outer>(
601+
fn import_published_index<'outer>(
602602
outer: Scope<'outer, mz_repr::Timestamp>,
603603
registry: &ArrangementSharingRegistry,
604604
idx_id: GlobalId,
@@ -788,8 +788,7 @@ where
788788
///
789789
/// Imports the published index as an arrangement, [`ArrangementFlavor::SharedTrace`], keyed and
790790
/// permuted as the plan expects, so a `Get` of `idx.on_id` and the joins and reduces below it
791-
/// consume an arrangement rather than re-deriving one. The import is a snapshot at `as_of`, so
792-
/// it serves single-time dataflows only.
791+
/// consume an arrangement rather than re-deriving one.
793792
fn import_index_shared<'outer>(
794793
&mut self,
795794
outer: Scope<'outer, mz_repr::Timestamp>,
@@ -801,26 +800,13 @@ where
801800
start_signal: StartSignal,
802801
) {
803802
let name = format!("Index({}, {:?})", idx.on_id, idx.key);
804-
// Bound the snapshot to the single read time `as_of`. Interactive work is single-time, so the
805-
// import's capability must drop once the shared trace seals past `as_of`, letting the one-shot
806-
// result complete. `self.until` may be empty (unbounded) for a long-lived dependency, which a
807-
// live `upper` never reaches, so it cannot serve as the snapshot bound.
808-
//
809-
// `try_step_forward` yields the frontier strictly greater than `as_of`. For an `as_of` at
810-
// `Timestamp::MAX` there is no such finite time, so the element drops out and the bound is the
811-
// empty (end-of-time) frontier, matching the semantics of "read the final state".
812-
let snapshot_until = Antichain::from_iter(
813-
self.as_of_frontier
814-
.iter()
815-
.filter_map(|t| t.try_step_forward()),
816-
);
817-
let (mut oks_arranged, errs_arranged, slot) = import_shared_index(
803+
let (mut oks_arranged, errs_arranged, slot) = import_published_index(
818804
outer,
819805
&compute_state.sharing_registry,
820806
idx_id,
821807
&name,
822808
&self.as_of_frontier,
823-
&snapshot_until,
809+
&self.until,
824810
);
825811

826812
// Attach the input probe to the replayed batch stream so hydration tracking observes it,
@@ -846,7 +832,7 @@ where
846832

847833
// The slot Arc's strong count marks a live reader, so it must outlive the dataflow. The read
848834
// hold is not in here: it lives in the `Arranged`s the bundle above retains, so that a
849-
// consumer can downgrade it. See `import_shared_index`.
835+
// consumer can downgrade it. See `import_published_index`.
850836
tokens.insert(idx_id, Rc::new(slot));
851837
}
852838
}
@@ -931,15 +917,8 @@ impl<'g> Context<'g, mz_repr::Timestamp> {
931917
}
932918
compute_state.traces.set(idx_id, trace);
933919
}
934-
Some(ArrangementFlavor::SharedTrace(..)) => {
935-
// Only the interactive runtime produces `SharedTrace`, and only for imports it reads
936-
// from the sharing registry. Its exports are transient query outputs, which are
937-
// freshly rendered `Local` arrangements (a join/reduce output), never a direct
938-
// re-export of an imported shared arrangement. The maintenance runtime's imports are
939-
// `Local`/`Trace`. So an export can never observe a `SharedTrace` input.
940-
unreachable!(
941-
"interactive runtime does not re-export an imported shared arrangement"
942-
);
920+
Some(ArrangementFlavor::SharedTrace(gid, _, _)) => {
921+
alias_shared_reexport(compute_state, &self.scope, idx_id, gid);
943922
}
944923
None => {
945924
println!("collection available: {:?}", bundle.collection.is_none());
@@ -1052,13 +1031,8 @@ where
10521031
}
10531032
compute_state.traces.set(idx_id, trace);
10541033
}
1055-
Some(ArrangementFlavor::SharedTrace(..)) => {
1056-
// See `export_index`: only the interactive runtime produces `SharedTrace`, and its
1057-
// exports are freshly rendered `Local` query outputs, never a re-export of an
1058-
// imported shared arrangement, so an export can never observe this variant.
1059-
unreachable!(
1060-
"interactive runtime does not re-export an imported shared arrangement"
1061-
);
1034+
Some(ArrangementFlavor::SharedTrace(gid, _, _)) => {
1035+
alias_shared_reexport(compute_state, &outer, idx_id, gid);
10621036
}
10631037
None => {
10641038
println!("collection available: {:?}", bundle.collection.is_none());
@@ -1102,6 +1076,30 @@ fn publish_reexport<'scope>(
11021076
registry.publish(idx_id, &oks, &errs);
11031077
}
11041078

1079+
/// Publishes `idx_id` as an alias of `gid`'s publication point, for an export whose arrangement is
1080+
/// an imported shared arrangement. The shared-arrangement analogue of [`publish_reexport`].
1081+
///
1082+
/// There is no `TraceBundle` to install for such an export, so `report_frontiers` reads its
1083+
/// frontier through the registry instead.
1084+
fn alias_shared_reexport<'scope, T: timely::progress::Timestamp>(
1085+
compute_state: &ComputeState,
1086+
scope: &Scope<'scope, T>,
1087+
idx_id: GlobalId,
1088+
gid: GlobalId,
1089+
) {
1090+
// Only `import_published_index` creates slots, and a dataflow rendered here imports no transient
1091+
// id (see the multiplexer's routing), so no reader can have created `idx_id`'s slot ahead of
1092+
// this publisher and the alias always registers.
1093+
let aliased =
1094+
compute_state
1095+
.sharing_registry
1096+
.publish_alias(idx_id, gid, scope.index(), scope.peers());
1097+
assert!(
1098+
aliased,
1099+
"re-export {idx_id} of shared arrangement {gid} found a reader-created slot"
1100+
);
1101+
}
1102+
11051103
/// Information about bindings, tracked in `render_recursive_plan` and
11061104
/// `render_plan`, to be passed to `render_letfree_plan`.
11071105
///

‎src/compute/src/render/context.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,8 @@ pub enum ArrangementFlavor<'scope, T: RenderTimestamp> {
238238
/// shared-trace primitive. Backed by `SharedTraceHandle`, so it is a real arrangement the plan
239239
/// can `Get`, not a re-derived collection. Only the interactive runtime produces this.
240240
///
241-
/// The `GlobalId` mirrors [`Self::Trace`]'s: it names the imported index. The interactive
242-
/// runtime never re-exports maintained indexes, so it is never consumed by an export path.
243-
///
244-
/// NOTE: the `TraceFrontier` bound on the imported handles is derived from the dataflow's
245-
/// `as_of`, not from its `until`, so this variant serves single-time dataflows only. Nothing
246-
/// checks that at the type level.
241+
/// The `GlobalId` mirrors [`Self::Trace`]'s: it names the imported index, so an export of this
242+
/// same arrangement can alias it instead of arranging again.
247243
SharedTrace(
248244
GlobalId,
249245
Arranged<'scope, SharedOksEnter<T>>,

‎src/compute/src/render/tests.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::shared_trace::SharedOksFrontier;
2626
use crate::sharing::ArrangementSharingRegistry;
2727
use crate::typedefs::{ErrBatcher, ErrBuilder, ErrSpine, RowRowAgent, RowRowSpine};
2828

29-
use super::import_shared_index;
29+
use super::import_published_index;
3030
use crate::server::ComputeRuntimeRole;
3131

3232
fn test_rows() -> Vec<(Row, Row)> {
@@ -112,7 +112,7 @@ fn interactive_import_replays_rows_and_holds_at_as_of() {
112112
let probe = ProbeHandle::new();
113113
let (mut oks_trace, mut errs_trace) = worker.dataflow::<Timestamp, _, _>(|scope| {
114114
// `until` empty: no upper suppression, so the whole snapshot at `as_of` flows.
115-
let (oks_arranged, errs_arranged, _slot) = import_shared_index(
115+
let (oks_arranged, errs_arranged, _slot) = import_published_index(
116116
scope.clone(),
117117
&registry_in,
118118
id,
@@ -236,7 +236,7 @@ fn tick(
236236
/// Mirrors the differential-dataflow primitive's own `import_hold_pins_then_releases`
237237
/// (`differential-dataflow/tests/sharing.rs`), which demonstrates the identical pin-then-release
238238
/// contract one layer down, directly on a bare `SharedTraceHandle` with no compute-level
239-
/// wrapping. This test drives the same `import_shared_index` primitive that
239+
/// wrapping. This test drives the same `import_published_index` primitive that
240240
/// `import_index_shared` calls in production, rather than re-deriving the contract from
241241
/// scratch.
242242
///
@@ -272,7 +272,7 @@ fn interactive_import_hold_releases_on_drop() {
272272
// lets a consumer downgrade the hold as its frontier advances. The `stream`s are dropped,
273273
// as a consumer that only needs the trace would.
274274
let (oks_trace, errs_trace) = worker.dataflow::<Timestamp, _, _>(|scope| {
275-
let (oks_arranged, errs_arranged, _slot) = import_shared_index(
275+
let (oks_arranged, errs_arranged, _slot) = import_published_index(
276276
scope.clone(),
277277
&registry,
278278
id,
@@ -368,7 +368,7 @@ fn interactive_import_holds_after_construction() {
368368
// handle it produced go out of scope with the builder, exactly as production does.
369369
let probe = ProbeHandle::new();
370370
worker.dataflow::<Timestamp, _, _>(|scope| {
371-
let (oks_arranged, _errs_arranged, _slot) = import_shared_index(
371+
let (oks_arranged, _errs_arranged, _slot) = import_published_index(
372372
scope.clone(),
373373
&registry,
374374
id,
@@ -436,7 +436,7 @@ fn published_since_does_not_chase_reader_holds() {
436436
// A reader at the higher as_of. Its handles go out of scope with the builder; the
437437
// import operator's own hold remains.
438438
worker.dataflow::<Timestamp, _, _>(|scope| {
439-
let (_o, _e, _slot) = import_shared_index(
439+
let (_o, _e, _slot) = import_published_index(
440440
scope.clone(),
441441
&registry,
442442
id,
@@ -501,7 +501,7 @@ fn import_reports_physical_within_chain_coverage() {
501501
);
502502

503503
let mut trace = worker.dataflow::<Timestamp, _, _>(|scope| {
504-
let (oks_arranged, _e, _slot) = import_shared_index(
504+
let (oks_arranged, _e, _slot) = import_published_index(
505505
scope.clone(),
506506
&registry,
507507
id,
@@ -553,7 +553,7 @@ fn interactive_import_hold_downgrades_while_live() {
553553
});
554554

555555
let (mut oks_trace, mut errs_trace) = worker.dataflow::<Timestamp, _, _>(|scope| {
556-
let (oks_arranged, errs_arranged, _slot) = import_shared_index(
556+
let (oks_arranged, errs_arranged, _slot) = import_published_index(
557557
scope.clone(),
558558
&registry,
559559
id,
@@ -623,7 +623,7 @@ fn interactive_import_hold_downgrades_while_live() {
623623
}
624624

625625
/// A published slot's `since` may already sit above the dataflow's requested `as_of` if the
626-
/// controller offered an unreadable `as_of`, a protocol error: `import_shared_index` must
626+
/// controller offered an unreadable `as_of`, a protocol error: `import_published_index` must
627627
/// panic rather than let the read silently see coalesced data, mirroring the maintenance
628628
/// path's `compaction_frontier` assert in `import_index`.
629629
///
@@ -663,7 +663,7 @@ fn import_asserts_since_at_most_as_of() {
663663

664664
// Importing at `as_of` now finds a `since` already beyond it: the assert must panic.
665665
worker.dataflow::<Timestamp, _, _>(|scope| {
666-
let _ = import_shared_index(
666+
let _ = import_published_index(
667667
scope.clone(),
668668
&registry,
669669
id,
@@ -720,7 +720,7 @@ fn standing_hold_pins_until_the_importing_runtime_applies() {
720720
// The queued create is now applied. It must import, and the rows it reads at `as_of` must
721721
// be the ones a read at `as_of` should see rather than a coalesced history.
722722
let (oks_trace, errs_trace) = worker.dataflow::<Timestamp, _, _>(|scope| {
723-
let (oks_arranged, errs_arranged, _slot) = import_shared_index(
723+
let (oks_arranged, errs_arranged, _slot) = import_published_index(
724724
scope.clone(),
725725
&registry,
726726
id,

‎src/compute/src/shared_trace/publish.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,12 @@ where
122122
self.shared.upper()
123123
}
124124

125+
/// The published seal frontier.
126+
pub(crate) fn upper(&self) -> Antichain<Tr::Time> {
127+
let state = self.shared.state.lock().expect("shared trace poisoned");
128+
state.upper.clone()
129+
}
130+
125131
/// Why this point would refuse an `as_of`. See [`Diagnostics`].
126132
pub(crate) fn diagnostics(&self) -> Diagnostics<Tr::Time> {
127133
Diagnostics {

‎src/compute/src/shared_trace/tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,7 @@ fn live_import_does_not_pin_merging() {
10381038
);
10391039
drop(arranged.trace);
10401040
});
1041-
// Drop the minting handle, as `crate::render::import_shared_index` does: the import owns
1041+
// Drop the minting handle, as `crate::render::import_published_index` does: the import owns
10421042
// its own clone, and a live mint would pin the floor at its own registration coverage and
10431043
// mask what this test is about.
10441044
drop(handle);

‎src/compute/src/sharing.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,17 @@ impl ArrangementSharingRegistry {
309309
}
310310
}
311311

312+
/// The `oks` seal frontier published for `id` on `worker_index`, if published.
313+
pub(crate) fn published_upper(
314+
&self,
315+
id: &GlobalId,
316+
worker_index: usize,
317+
) -> Option<Antichain<Timestamp>> {
318+
let inner = self.lock();
319+
let slot = inner.map.get(id)?.get(worker_index)?.as_ref()?;
320+
Some(slot.oks.upper())
321+
}
322+
312323
/// Mints reader handles for `id` on `worker_index`, if published.
313324
pub(crate) fn handles(
314325
&self,

0 commit comments

Comments
 (0)