Skip to content

compute: report how far readers hold a shared arrangement back - #38736

Draft
antiguru wants to merge 1 commit into
mh/interactive-10-benchmarks-loadfrom
mh/interactive-11-hold-gap
Draft

antiguru wants to merge 1 commit into
mh/interactive-10-benchmarks-loadfrom
mh/interactive-11-hold-gap

Conversation

@antiguru

@antiguru antiguru commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

13 of 14 in the interactive-runtime stack. Stacks on #38678, followed by #40559. Tracked by CPU-216. The design document is doc/developer/design/20261107_interactive_runtime.md in #40559.

Publishing an arrangement couples its compaction to the importing runtime's drain rate: a shared trace applies the meet of what its own handles allow and what the readers hold. Before this the coupling was invisible.

mz_compute_shared_arrangement_hold_gap_ms is the largest gap, over the arrangements a worker publishes, between the logical compaction frontier the publishing trace's own handles allow and the one the readers' holds let it apply, in timestamp units (milliseconds in the epoch-millisecond timeline). mz_compute_shared_arrangement_held_count is how many have a gap at all. Each publication point records what the trace's same-thread agents asked for (requested_logical) beside the since the trace applied, and the gap is the difference. Reported at most once a second, since a report takes the registry lock and up to four point locks per published index. Reported from the runtime's publishing registry, so a runtime that publishes nothing reports zero and a sum or max over the role label stays correct.

Two blind spots, stated next to hold_gaps:

  • Only logical compaction is measured. Physical holds that stall merges, and batches a reader's chain clone pins, are not.
  • Frozen slots are skipped. Their points stop publishing frontiers once the writer closes, but the trace lives on while a same-thread agent holds it, and readers of the frozen points still hold it back.

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.

Tests: logical_frontiers_report_what_the_holds_withhold in shared_trace/tests.rs and a_frozen_slot_reports_no_hold_gap in sharing/tests.rs.

A reviewer should check the throttle in compute_state.rs and that the metric help texts in metrics.yml state both blind spots.

Posted by Claude Code.

🤖 Generated with Claude Code

@antiguru
antiguru requested review from a team as code owners September 9, 2026 21:22
@def-

def- commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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.

@antiguru
antiguru requested review from a team as code owners September 12, 2026 13:28
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from d460f14 to 050855f Compare September 12, 2026 13:28
@antiguru
antiguru marked this pull request as draft September 14, 2026 17:45
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 050855f to a87206f Compare September 16, 2026 19:43
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from a87206f to 2469218 Compare September 16, 2026 20:58
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 2469218 to 238d0d5 Compare September 17, 2026 01:50
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 238d0d5 to 19e104c Compare September 23, 2026 12:00
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 19e104c to fa959cc Compare September 23, 2026 14:13
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from fa959cc to dc4e800 Compare September 24, 2026 11:39
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from dc4e800 to 89f23bc Compare September 24, 2026 13:00
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 89f23bc to acfbd15 Compare September 25, 2026 10:58
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from acfbd15 to 8ebd13d Compare September 25, 2026 11:14
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 8ebd13d to d73cb95 Compare September 25, 2026 12:59
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from d73cb95 to 5a3b3f3 Compare September 25, 2026 17:08
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 7473a69 to 55f3031 Compare October 2, 2026 14:09
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 55f3031 to a6353e1 Compare October 2, 2026 14:14
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from a6353e1 to 4e54a48 Compare October 2, 2026 14:19
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 4e54a48 to 7e75035 Compare October 2, 2026 15:15
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from 7e75035 to 2859180 Compare October 2, 2026 15:29
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch 2 times, most recently from 7ac74ba to b80a570 Compare October 3, 2026 08:00
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from b80a570 to 7ac74ba Compare October 3, 2026 08:06
@antiguru
antiguru added this pull request to stack #39514 October 3, 2026 08:15
@antiguru
antiguru removed this pull request from stack #39514 October 3, 2026 08:15
@antiguru
antiguru added this pull request to stack #39485 October 3, 2026 08:15
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch 2 times, most recently from a809ec6 to f10da0d Compare October 3, 2026 12:02
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from f10da0d to a3a6f8b Compare October 5, 2026 13:10
@antiguru
antiguru force-pushed the mh/interactive-11-hold-gap branch from a3a6f8b to f84d5da Compare October 6, 2026 07:25
The publishing runtime reports, once a second, the largest gap between the
logical compaction frontier an arrangement's own handles allow and the one the
readers' holds let it apply, and how many arrangements have a gap. Only logical
compaction is measured, not physical holds or batches a reader's chain pins.

Frozen slots are not measured. Their points stop publishing frontiers once the
writer closes, but the trace can live on while a same-thread agent holds it,
and the readers of the frozen points still hold it back. The metric is blind
to that case.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

This branch has not been deployed

No deployments
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