cmux-tui: no request waits for the disk while it holds the state lock or the registry connection

Merged
LA
5d ago
+183-42

Summary

On a loaded Mac (#27698: daemon lock waits p90 5 s, p99 17 s), two request paths waited for the disk while they held a lock. Other requests queued behind them:

  • rename-workspace held the mux state lock across its journal writer receipt wait (up to the 30 s durable wait). Every state reader (list-workspaces, identify, terminal event placement) waited for the disk. The rename now releases state before the wait and takes it again to apply the committed name, finding the workspace by its key. The registry stays held, so no other topology writer interleaves.
  • A request that found an earlier indeterminate commit waited for its answer (2 s) with the registry connection pinned. The journal writer needs that connection to commit and answer, so the wait could only run out and fail the request. lock_state_pinned now waits for those answers before it pins, and drain_unsettled never waits while the thread holds the connection: it takes only the answers that already arrived. A commit still unanswered after the unpinned wait is refused at once, as before.

With nothing left to unpin, PinnedState::unpin and the release_pin hook are removed. A debug assert now refuses any journal writer receipt wait while the thread holds the mux state lock. It did not fire in any full test run of this branch.

What callers see after a rename. The rename replies only after the new name is in state, so a caller that renames and then lists sees the new name. Until the rename takes state again after its commit is durable, other readers see the old name. A journal subscriber that the writer wakes can read the old name for that short time, as after other writer-batch commits (resource_effects.rs:217). The test checks both: no reply before durability, and only the old name visible to readers during the wait.

This PR also carries one follow-up from the review of #28147: server-stats locks now formats its site labels after it releases the site table, and two schema doc lines are corrected.

This PR contains #28147 (the deadlock fix and the lock statistics). Review only the commits after 100b8e317c35; once #28147 lands, the diff shrinks to them. #27780 (Austin) touches the same registry lock, in a different function.

Owner: PL-daemon (session cmux-b6).

Testing

Regression tests, both in cmux-tui-core/src/mux/tests/writer_lock_order.rs:

  • a_rename_waiting_for_its_writer_receipt_does_not_hold_state pauses the journal writer inside its batch and probes the state lock while the rename waits.

  • waiting_for_an_earlier_indeterminate_commit_leaves_the_connection_free holds an unanswered receipt and probes the connection while a rename waits for it.

  • Red, fleet step a7af786698ead5c101d1d25c: scripts/ci/cmux-tui-rust-check.sh test writer_lock_order at b702c984cf62 (tests only). Both new tests FAILED on their "lock is free" asserts (state_free, line 238; connection_free, line 292). The other four tests passed.

  • Green: scripts/ci/cmux-tui-rust-check.sh all at 352210fa9976, fleet step 4aa52296785c6974c78b8d10: fmt and clippy pass. 12705 tests ran and 12703 passed. The 2 failures are base reds on feat-cmux-next: every_safe_transport_operation_has_a_noun_first_path (fix #28119) and the acpmux hermetic_daemon_guard check for web_image_attachments (fix #28150). The new debug assert did not fire.

Observed / Not run

  • Observed: the red failures above. In the earlier full run 87ecb023 (at 04bb5f0a4b38), the only failures were the two base reds named in #28147 (CLI noun-first path, fixed by #28119; acpmux hermetic guard), plus the unexplained reds below.
  • Reds this PR does not cause. They are listed in the handoff followups for a fixer, not cleared as flaky; each failed once and passed when run again alone:
    • surface::tests::local_resize_order::local_resize_bounds_the_drain_to_a_queued_snapshot ("the resize consumed output without publishing its title", local_resize_order.rs:111; step dd8687a1). This PR cannot reach it: its title event takes only mux.state, through none of the changed functions.
    • activity::v2_terminal_input_counts_only_from_a_persons_attached_client (a 12 s read timeout, activity.rs:25; step 87ecb023). This PR cannot reach it: it runs on an in-memory mux with no journal writer, so there are no unsettled receipts, and it does no rename.
    • acpmux daemon_restart_sends_close_1012_before_wireguard_shutdown and subsessions_safety::a_message_never_reaches_a_session_that_asks_less_than_its_sender. This PR cannot reach them: acpmux does not depend on cmux-tui-core.
  • Not run: Austin's-day before and after numbers, which the harness owner runs and posts on this PR. My own diagnostic bench (bench-daemon-latency.py --scenarios agents, before: step 73e93323) only ranks lock holders and is not the merge evidence. Perf claim: none until those numbers are posted.

Known follow-ups (from review, not regressions)

  • A pinned flow can still miss a late answer that arrives between its pin and its settle. It then builds on state without that answer. The window existed before this PR and is now smaller (Low 1 in the review). The fix, refusing or settling an answer that resolves after the pin's settle, goes in the next PR of this series, with a correction of the drain_unsettled doc text that this PR overstates.
  • The error path for a workspace that left state after a durable rename should raise the revisions first. It cannot be reached today (Low 3).

Performance

No perf numbers: this PR claims no measured speedup. The Austin's-day before and after numbers come from the harness owner and will be posted on this PR when they exist.

Changelog

Fixed: Renaming a workspace on a busy Mac no longer makes the rest of cmux NEXT wait for the disk.

Proof

No capture: daemon-only change, no UI.