Skip to content

Commit e6a88fa

Browse files
antiguruclaude
andcommitted
fixup: the interactive runtime installs no logging dataflow
The interactive runtime installed empty copies of the logging indexes, with logging forced off, so that `initialize_logging`'s bookkeeping stayed uniform. Every command handler then had to tell those copies apart from the runtime's own collections: `handle_allow_compaction` carried a role check and a transience check to route broadcast compaction around them, and `report_frontiers` skipped non-transient ids so the copies' frontiers would not regress the maintenance runtime's reports. The copies served nothing. Peeks on the logging indexes read the maintenance runtime's publications from the registry, and only the maintenance runtime publishes them. With the interactive runtime installing no logging dataflow, "is this a collection this runtime hosts" is a question `collections` answers, and `handle_allow_compaction` reduces to: an unhosted id is the peer's, so its frontier is the standing hold. The role check survives only as a soft assertion that the maintenance runtime never sees compaction for an unknown collection, which used to panic in `drop_collection`. `report_frontiers` loses its filter. `handle_peek` selects the trace source by matching on the role. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDm7opomJLxbNUEP3r9BLk
1 parent f298fe3 commit e6a88fa

1 file changed

Lines changed: 30 additions & 59 deletions

File tree

‎src/compute/src/compute_state.rs‎

Lines changed: 30 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ impl<'a> ActiveComputeState<'a> {
853853
// would diverge that order across workers, latently unsound under a multi-worker interactive
854854
// runtime. On the interactive runtime a query dataflow imports its maintenance-index inputs
855855
// from the sharing registry, binding each through a registry placeholder that a maintenance
856-
// publisher adopts later (see `render::import_shared_index`). A not-yet-published dependency
856+
// publisher adopts later (see `render::import_published_index`). A not-yet-published dependency
857857
// therefore yields an empty import held at the minimum frontier, so the build is always
858858
// possible without waiting.
859859
let dataflow_index = Rc::new(self.timely_worker.next_dataflow_index());
@@ -995,42 +995,25 @@ impl<'a> ActiveComputeState<'a> {
995995
fn handle_allow_compaction(&mut self, id: GlobalId, frontier: Antichain<Timestamp>) {
996996
let worker_index = self.timely_worker.index();
997997

998-
let interactive = self.compute_state.role == ComputeRuntimeRole::Interactive;
999-
1000-
// The multiplexer broadcasts compaction for the collections its peer publishes, which are the
1001-
// ones this runtime may import, so on the interactive runtime a non-transient id is one of
1002-
// those. This runtime's own publications are its transient query outputs.
1003-
let peer_published = interactive && !id.is_transient();
1004-
if peer_published {
1005-
// The standing hold: this runtime's own position in the command stream, which the peer's
1006-
// publisher bounds its compaction by. An importing dataflow of ours whose `CreateDataflow`
1007-
// is still queued here has registered no reader hold yet, so nothing else keeps the
1008-
// arrangement at or below the `as_of` it is about to read at.
998+
// A collection this runtime does not host is one its peer publishes and this runtime may
999+
// import. The multiplexer broadcasts `AllowCompaction` for those, and the frontier becomes
1000+
// the standing hold: this runtime's own position in the command stream, which the peer's
1001+
// publisher bounds its compaction by. An importing dataflow of ours whose `CreateDataflow`
1002+
// is still queued here has registered no reader hold yet, so nothing else keeps the
1003+
// arrangement at or below the `as_of` it is about to read at.
1004+
//
1005+
// Hosting is a question about `collections`, not about the id. The peer also renders
1006+
// transient collections of its own (subscribes and copy-tos) this runtime has never seen,
1007+
// and neither of those may reach `drop_collection`, which would panic on the untracked
1008+
// collection or unpublish an arrangement this runtime does not own.
1009+
if !self.compute_state.collections.contains_key(&id) {
1010+
mz_ore::soft_assert_or_log!(
1011+
self.compute_state.role == ComputeRuntimeRole::Interactive,
1012+
"compaction for a collection this runtime does not host: {id}"
1013+
);
10091014
self.compute_state
10101015
.sharing_registry
10111016
.note_standing_hold(id, worker_index, &frontier);
1012-
}
1013-
1014-
// Whether there is local work is a question about `collections`, NOT about the id: this
1015-
// runtime holds empty local copies of the peer's introspection indexes, whose ids are the
1016-
// peer's to publish, and the peer renders transient collections of its own (subscribes and
1017-
// copy-tos) that this runtime has never seen. Asking the id instead sends a broadcast frontier
1018-
// for one of those down the drop path, where `drop_collection` panics on a collection that was
1019-
// never installed here.
1020-
if interactive && !self.compute_state.collections.contains_key(&id) {
1021-
return;
1022-
}
1023-
1024-
if peer_published {
1025-
if !frontier.is_empty() {
1026-
// Keeps this runtime's empty local copy of an introspection index in step.
1027-
self.compute_state
1028-
.traces
1029-
.allow_compaction(id, frontier.borrow());
1030-
}
1031-
// Never `drop_collection` for one of those. It would also `sharing_registry.remove(&id)`
1032-
// and so unpublish an arrangement this runtime does not own. The empty copies live for
1033-
// the process lifetime, and the peer drops the real collection on its own stream.
10341017
return;
10351018
}
10361019

@@ -1053,16 +1036,17 @@ impl<'a> ActiveComputeState<'a> {
10531036
fn handle_peek(&mut self, peek: Peek) {
10541037
let pending = match &peek.target {
10551038
PeekTarget::Index { id } => {
1056-
let traces = if self.compute_state.role == ComputeRuntimeRole::Interactive {
1039+
let traces = match self.compute_state.role {
10571040
// The interactive runtime maintains no traces of its own. It reads the
10581041
// arrangements the maintenance runtime publishes into the sharing registry.
1059-
IndexTraces::Shared {
1042+
ComputeRuntimeRole::Interactive => IndexTraces::Shared {
10601043
registry: self.compute_state.sharing_registry.clone(),
10611044
worker_index: self.timely_worker.index(),
1062-
}
1063-
} else {
1045+
},
10641046
// Acquire a copy of the trace suitable for fulfilling the peek.
1065-
IndexTraces::Local(self.compute_state.traces.get(id).unwrap().clone())
1047+
ComputeRuntimeRole::Maintenance | ComputeRuntimeRole::Solo => {
1048+
IndexTraces::Local(self.compute_state.traces.get(id).unwrap().clone())
1049+
}
10661050
};
10671051
PendingPeek::index(peek, traces)
10681052
}
@@ -1214,15 +1198,14 @@ impl<'a> ActiveComputeState<'a> {
12141198
panic!("dataflow server has already initialized logging");
12151199
}
12161200

1217-
let mut config = config;
1218-
// The interactive runtime maintains no introspection indexes of its own: it serves
1219-
// introspection peeks from the maintenance runtime's registry-published copies (see
1220-
// `logging::publish_logging_index`). Force logging off so its replay stays empty. The
1221-
// dataflows still install (empty, per database-issues#4545), so the logging indexes are
1222-
// still created and the sanity check below still holds, but they hold no data and, being
1223-
// non-maintenance, are never published.
1201+
// The interactive runtime keeps no introspection state of its own. Its peeks on the logging
1202+
// indexes read the maintenance runtime's publications from the registry (see
1203+
// `logging::publish_logging_index`), so installing the logging dataflow here would only
1204+
// create collections under ids this runtime does not own, which every command handler
1205+
// would then have to tell apart from its own. Without a `compute_logger` this runtime's
1206+
// own events are not logged. TODO(CPU-222): log them through the maintenance runtime.
12241207
if self.compute_state.role() == ComputeRuntimeRole::Interactive {
1225-
config.enable_logging = false;
1208+
return;
12261209
}
12271210

12281211
let LoggingTraces {
@@ -1291,14 +1274,6 @@ impl<'a> ActiveComputeState<'a> {
12911274
pub fn report_frontiers(&mut self) {
12921275
let mut responses = Vec::new();
12931276

1294-
// The interactive runtime installs empty copies of the maintenance runtime's
1295-
// logging/introspection indexes (see `initialize_logging`) and shares every non-transient
1296-
// collection's identity with the maintenance runtime, which owns and reports the real
1297-
// frontiers. Reporting our empty copies' frontiers races the owner's report for the same
1298-
// collection id in the controller's single per-collection frontier stream, regressing it.
1299-
// Report only the wholly-transient query dataflows this runtime exclusively hosts.
1300-
let report_only_transient = self.compute_state.role() == ComputeRuntimeRole::Interactive;
1301-
13021277
// Maintain a single allocation for `new_frontier` to avoid allocating on every iteration.
13031278
let mut new_frontier = Antichain::new();
13041279

@@ -1309,10 +1284,6 @@ impl<'a> ActiveComputeState<'a> {
13091284
continue;
13101285
}
13111286

1312-
if report_only_transient && !id.is_transient() {
1313-
continue;
1314-
}
1315-
13161287
let reported = collection.reported_frontiers();
13171288

13181289
// Collect the write frontier and check for progress.

0 commit comments

Comments
 (0)