compute: report how far readers hold a shared arrangement back

[antiguru]

Publishing an arrangement couples its compaction to the importing runtime's drain rate: a shared trace applies the meet of what its controller stream asked for and what the readers hold. Before this the coupling was invisible and would first surface as a memory incident.

mz_compute_shared_arrangement_hold_gap_ms is the largest gap, over the arrangements a worker publishes, between the frontier the controller asked for and the one the holds let the trace apply. mz_compute_shared_arrangement_held_count is how many are held at all. Reported from the maintenance runtime alone, so the interactive series stays at zero and a sum or max over the label stays correct.

Zero is the healthy reading: both runtimes get the same AllowCompaction, so an interactive runtime that drains promptly leaves the two frontiers equal. Measured on an 8-worker replica under continuous churn, the gauges read zero with no reader and peaked at one compaction round with 31 arrangements held while 509 interactive joins ran over the published index.

🤖 Generated with Claude Code

https://claude.ai/code/session_015tLhSbZdXrTSK2KwSocT59

[def-]

QA LLM Review

1. MEDIUM -- hold_gaps scans the whole process-wide registry under its global mutex, ~100 times per second per worker

src/compute/src/compute_state.rs:1392

report_metrics runs on every compute_server_maintenance_interval tick (default 10 ms) on every maintenance worker, and each call now takes the process-global sharing-registry mutex and holds it across a scan of every published id, acquiring two Shared state locks and cloning two antichains per arrangement. That mutex is also on the interactive runtime's per-iteration take_dirty path, on notify for every seal, and on handles for every shared peek, so a gauge that is only ever read at scrape time serializes both runtimes' hot paths at 100 Hz per worker.

Details

hold_gaps (src/compute/src/sharing.rs:338) holds self.lock() for the entire loop body. Per map entry it does a BTreeSet::insert(Arc::as_ptr(..)) plus slot.oks.logical_frontiers() (src/compute/src/shared_trace/publish.rs:129), which is two separate acquisitions of that point's state mutex and two Antichain::clone() heap allocations. Cost is workers x published ids per 10 ms, all serialized on one process-wide lock.

What contends with it: take_dirty (src/compute/src/sharing.rs:418) on every interactive loop iteration (src/compute/src/server.rs:548); notify (src/compute/src/sharing.rs:502) from each publisher's on_seal, i.e. once per sealed batch per published arrangement; handles (src/compute/src/sharing.rs:320) once per shared peek; and publish/remove on every DDL. O(N)-per-tick work in report_metrics is precedented (report_frontiers, traces.maintenance()), but neither of those holds a lock the other runtime needs.

Worth fixing together with two related points:

• The comment at src/compute/src/compute_state.rs:1384 ("only it has slots in the registry under its own worker ordinal") is not accurate. ComputeRuntimeRole::publishes() (src/compute/src/server.rs:117) is true for Interactive as well, so interactive's transient peek-dataflow index exports are published into the same map at the same worker ordinals (both runtimes span equal peers and share the process ordinal), and import_published_index additionally creates reader-side slots there. The maintenance scan therefore covers points it neither publishes nor writes. Scoping the walk to points this runtime published would both fix the attribution and shrink the scan.
• Reading the pair as (self.shared.since(), self.shared.writer_since()) is two lock acquisitions. The commit message states the pair is "written in the same critical section as the applied frontier so the pair is always consistent" -- the write is atomic, the read is not. Adding a Shared::logical_frontiers() that returns both from one guard, mirroring the existing Shared::frontiers() (src/timely-util/src/shared_trace.rs:187), halves the per-point lock traffic and delivers the consistency the message claims. It only tears for a point whose writer is another thread, which today is exactly the interactive-published ones above.

Simplest cadence fix: recompute on its own, much coarser interval (or every Nth maintenance tick) rather than on the 10 ms maintenance tick -- the gauge is sampled at scrape granularity, so 100 Hz is three to four orders of magnitude more often than it is read.