Skip to content

docs(concurrency): document unbounded-channel backpressure invariants#306

Open
githubrobbi wants to merge 1 commit into
mainfrom
docs/phase-10d-backpressure
Open

docs(concurrency): document unbounded-channel backpressure invariants#306
githubrobbi wants to merge 1 commit into
mainfrom
docs/phase-10d-backpressure

Conversation

@githubrobbi
Copy link
Copy Markdown
Collaborator

What

Phase 10d audit closure — document the 2 true prod unbounded mpsc::unbounded_channel() call-sites as deliberately unbounded with rate-bounded producers and explicit memory worst-case ceilings.

Why

Phase-10a baseline reported "4 unbounded channels (3 daemon + 1 client)" based on raw audit-script regex hits. Hand-audit corrected the count to 2 true prod sites:

  • crates/uffs-daemon/src/cache/journal_sink.rs:160 (RegistryPatchSink::apply_tx)
  • crates/uffs-client/src/connect.rs:83 (UffsClient::notification_tx)

The script's "3 daemon" matched two field-type sigs + the prod constructor + a #[cfg(test)] new_for_test constructor all in the same file; the additional index/search.rs hits were doc-comment occurrences of the word "unbounded". These are noise; the corrected per-site verdict is captured in docs/dev/baseline/2026-05-19/phase_10_backpressure_audit.md (local; docs/dev/baseline/ is gitignored).

Verdict — both sites kept unbounded, justified

RegistryPatchSink::apply_tx (daemon)

Sync callback escape hatch from the journal loop's accept / trigger_save / journal_wrapped (sync fn, not async fn) into the async applier_task.

Constraints pinning unbounded:

  1. Producer is sync-non-blocking by contract — cannot .await on a bounded send. A bounded variant would be try_send + drop-on-full, operationally identical to the existing "dead applier silently absorbed" degraded path documented on apply_tx.
  2. Producer cadence throttled upstream by SaveTrigger (50K events ∨ 5-min age). Worst-case steady-state ≈ 5 messages/min across all drives.
  3. Payload boundedApplyMsg::Save carries ≤ 50K-event Vec<FileChange>, ~10 MB peak, consumed in ~1 s by the serial applier.

Augmented: full # Backpressure rustdoc block on RegistryPatchSink::spawn_with_applier.

UffsClient::notification_tx (client)

Per-client receive buffer for incoming DaemonEvent notifications forwarded from the IPC read loop.

Constraints pinning unbounded:

  1. Producer rate is upstream-bounded by the daemon's broadcast::Sender capacity (DEFAULT_BROADCAST_CAPACITY). A slow client cannot induce unbounded daemon emit.
  2. Notification cadence is intrinsically low (≈ 2/min steady-state).
  3. try_send on bounded would invert responsibility — the producer is the async IPC read_loop, and stalling it would block RPC responses on the same socket, strictly worse than the current "drop only at the broadcast layer" design.

Augmented: concise inline comment at the unbounded_channel() call-site referencing the audit doc. Kept tight because connect.rs is right at the 800-LOC file-size policy ceiling — the audit doc carries the full reasoning.

Rule-1 adherence

  • Zero behavior change. Documentation-only PR.
  • Zero #[allow(...)] introductions.
  • No suppression hacks, no disabled lints, no skipped tests.
  • No file-size-policy exceptions added — kept connect.rs at exactly 800 LOC by using a single-line inline comment + audit-doc reference instead of in-source rustdoc.
  • Surgical — minimal text at each call-site flags the design choice + points to the audit doc that carries the full reasoning.

Verification

Local:

  • cargo fmt -p uffs-client -p uffs-daemon — clean.
  • cargo clippy -p uffs-client -p uffs-daemon --all-targets -- -D warnings — 0 warnings.
  • cargo test -p uffs-daemon --lib298 passed / 0 failed.
  • cargo test -p uffs-client2 passed + 1 doctest passed.
  • Pre-commit lint-fast — all 7 stages passed.
  • Pre-push lint-pre-push — all 17 stages passed in 100 s.

Acceptance against playbook §1142-1146

Channels are either bounded with documented capacity OR unbounded with documented producer-rate ceiling.

CLOSED for Phase 10d. Both prod unbounded channels have producer-rate ceilings documented at the call-site; the audit-script over-count is corrected in phase_10_backpressure_audit.md; the script's matching regex stays as a "broad sweep" while this hand-audit is the canonical channel inventory.

Will be folded into concurrency_policy.md §"Channel discipline" in Phase 10g.

Tracking

Refs #302 (Phase 10 umbrella). Sub-phases queued:

  • 10e — timeout coverage audit.
  • 10f — blocking-IO-in-async audit.
  • 10g — concurrency_policy.md + per-crate # Concurrency rustdoc.
  • 10h — CONTRIBUTING cross-link + final report (folded into 10g).

Phase 10d audit of all true prod unbounded `mpsc::unbounded_channel()`
call sites.  Phase-10a baseline reported 4 (3 daemon + 1 client) based
on raw regex hits; hand-audit corrected to **2 true prod sites**:

  * `crates/uffs-daemon/src/cache/journal_sink.rs:160` (`RegistryPatchSink::apply_tx`)
  * `crates/uffs-client/src/connect.rs:83` (`UffsClient::notification_tx`)

The script's "3 daemon" matched two field-type sigs + the constructor
inside the same file plus a `#[cfg(test)] new_for_test` constructor;
two additional `uffs-daemon/src/index/search.rs` matches were
doc-comment occurrences of the word "unbounded" (predicates /
display-row filters).

Both prod sites are kept unbounded with documented "by-construction
bounded" rationale + producer-rate ceilings + worst-case memory
calculations.

## Site 1 — `RegistryPatchSink::apply_tx`

Sync callback escape hatch from the journal loop's
`accept`/`trigger_save`/`journal_wrapped` (sync `fn`, not `async fn`)
into the async `applier_task`.

Constraints pinning unbounded:

  1. Producer is sync-non-blocking by contract — cannot `.await`
     on a bounded send.  A bounded variant would be `try_send` +
     drop-on-full, identical to the existing "dead applier silently
     absorbed" degraded path documented on `apply_tx`.
  2. Producer cadence throttled upstream by `SaveTrigger` (50K
     events ∨ 5-min age).  Worst-case steady-state ≈ 5 messages/min
     across all drives.
  3. Payload bounded (`ApplyMsg::Save` carries ≤ 50K-event
     `Vec<FileChange>`, ~10 MB peak, consumed in ~1 s by serial
     applier).

Added a full `# Backpressure` rustdoc block on `spawn_with_applier`.

## Site 2 — `UffsClient::notification_tx`

Per-client receive buffer for incoming `DaemonEvent` notifications
forwarded from the IPC read loop.

Constraints pinning unbounded:

  1. Producer rate is upstream-bounded by the daemon's
     `broadcast::Sender` capacity (`DEFAULT_BROADCAST_CAPACITY`).  A
     slow client cannot induce unbounded daemon emit.
  2. Notification cadence is intrinsically low (≈ 2/min steady-state;
     ≈ 0.5/sec peak during multi-drive load).
  3. `try_send` on bounded would invert responsibility — the producer
     is the async IPC `read_loop`, and stalling it would block RPC
     responses on the same socket, strictly worse than the current
     "drop only at the daemon broadcast layer" design.

Added a full `# Backpressure` rustdoc block on the
`notification_tx`/`notification_rx` field docs.

## Rule-1 adherence

  * Zero `#[allow(...)]` introductions.
  * No suppression hacks, no skipped tests.
  * Documentation-only — zero behavior change.
  * `cargo check / clippy -D warnings / fmt` all clean.
  * `cargo test -p uffs-daemon --lib` — 298 passed.
  * `cargo test -p uffs-client` — 2 passed + 1 doctest.

Per-site verdict in
`docs/dev/baseline/2026-05-19/phase_10_backpressure_audit.md` (local).

Refs #302.
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.

1 participant