docs/reviews/2026-07-04-scalability-infra-review.md

@@ -0,0 +1,249 @@
+# 2026-07-04 — Scalability & infra-fit review
+
+**Scope:** what single-process assumptions are baked into the code today that will fight
+horizontal scaling, autoscaling, and load balancing later — and how rutster's shape maps onto
+infra overall. **Not in scope:** the fused per-call vertical itself (deliberate, ADR-0002) or
+documented later-rung deferrals with clean seams.
+
+**Method:** 7-lens multi-agent survey over every crate (43 raw findings), dedup (17), adversarial
+per-finding verification (each verifier read the cited code with a default-skeptical stance),
+plus first-hand re-verification of 5 findings whose verifiers were lost to a session limit.
+2 findings were rejected outright (one factually wrong, one a documented deferral); several were
+downgraded. Severity: **blocker** = correctness breaks or scaling impossible at N>1 (or even N=1
+in cloud); **major** = missing seam that calcifies as code accretes; **minor** = trivial when the
+time comes.
+
+---
+
+## The infra model this system actually implies
+
+Before the findings: the frame. rutster's fused vertical means **a call is pinned to a process
+for its whole life** — DTLS/ICE state, jitter buffer, reflex state, the playout ring. Calls are
+non-migratable, same as they were on Asterisk. That is not a defect; it defines the scaling model:
+
+1. **Placement, not load balancing.** New calls get *placed* on a node (least-loaded /
+   bin-packed); every subsequent operation on that call must reach the *owning* node. A
+   round-robin L7 LB over the REST surface is the wrong mental model and breaks at N=2 today.
+   The proven patterns: (a) a shared session→node directory (Valkey — already ratified,
+   ADR-0005) behind a thin routing/API tier, or (b) the create-response carries a node-addressed
+   URL (the LiveKit/Janus pattern). The CPaaS trunk path (ADR-0007 layer 1) converges on the
+   same decision point: Twilio/Telnyx hand the media-stream URL out **per call at answer time**,
+   so trunk dispatch and API routing are the *same* placement problem with the same answer.
+
+2. **Capacity is tick-budget, and it must be measured to be scheduled.** The calls-per-node
+   bound is the media thread exhausting its 10ms meta-tick. CPU% is the wrong autoscaling
+   signal for a real-time engine (it lags and it lies); the right signals are
+   **calls-in-flight** and **tick-overrun rate** (deadline misses). Neither exists today.
+   Convergence worth exploiting: ADR-0010's pulled-forward benchmark/sim harness needs exactly
+   this gauge — its primary readout should be "max sessions at <X% tick overrun," which then
+   becomes the default admission cap.
+
+3. **Scale-in = drain, and drain is slow.** Calls last minutes. Scale-in, rolling deploys, and
+   spot reclamation all deliver SIGTERM; the only viable pattern is
+   stop-accepting → bleed-out (bounded by a deadline) → terminate. Consequences: autoscaler
+   scale-in protection / lifecycle hooks are mandatory for the engine tier, spot instances are
+   effectively off the table for it, and deploys imply version-skew tolerance across the fleet
+   (the tap protocol being versioned already is the right instinct).
+
+4. **Three tiers with different scaling laws.** The decomposition the ADRs already imply:
+   - **API/routing tier** — stateless once a session directory exists; scales on RPS; can live
+     inside the engine binary at first and split out later without design change.
+   - **Engine fleet** — scales on call capacity; drain-based lifecycle; per-node advertised
+     media address; this is the tier every finding below is about.
+   - **Brain fleet** — external by design; scales independently; the tap protocol is the seam,
+     and wire semantics (resume/terminal-bye) are the part that calcifies.
+   Substrate: Valkey (directory + bus + presence), object storage (CDR/recordings), the CPaaS
+   or SBC at the edge. This is exactly "fused where fusion buys the wedge; composable where
+   independent scaling matters" — the code just hasn't grown the horizontal seams yet.
+
+5. **The silent N>1 correctness traps** are worse than the loud ones: a DELETE that 204s on the
+   wrong node while the call (and its spend) keeps running; per-instance spend caps that
+   quietly become N× the fleet cap; CDRs that evaporate with an OOM-kill. These bite after
+   things *appear* to work.
+
+---
+
+## Blockers
+
+### B1. Media addressing: loopback bind, no advertised-address concept, no port range
+`crates/rutster-media/src/rtc_session.rs:179` binds `127.0.0.1:0`; `accept_offer`
+(rtc_session.rs:236) advertises the raw local socket as the **sole** ICE host candidate. No
+srflx/STUN, no NAT 1:1 advertised-IP config, no port-range allocator; `RtcSession::new()` is
+zero-arg, and nothing in the chain main → `MediaThread::spawn` → `MediaCmd::Register` can carry
+addressing config. The file is CI-seam-frozen (ci.yml:51–64 — a deliberate, updatable speed
+bump, but it means the seam change is a "loud" PR by design).
+**Why blocker:** RTP cannot ride the HTTP LB; even a single cloud instance behind NAT is
+unreachable — the SDP answer says `127.0.0.1:<ephemeral>`. Fleet firewalling needs a bounded
+port range. *(Adversarially verified: CONFIRMED, structurally baked in — str0m rejects 0.0.0.0
+candidates, so this needs a bind-vs-advertised split, a new concept, not a constant swap.)*
+**Fix shape:** `MediaAddressConfig { bind_interface, advertised_address, udp_port_range }`
+threaded through the construction chain; advertised addr feeds `Candidate::host`, bind addr
+feeds the socket; update the seam-gate hashes in the same PR.
+
+### B2. Session placement: ownership is process-local, with a silent-failure DELETE
+The only registry of live calls is the media thread's private
+`HashMap<ChannelId, ThreadSession>` (media_thread.rs:164). Routes reach it over a
+process-local mpsc. ADR-0005's Valkey has **zero code presence** — no dep, no crate, no trait.
+At N=2 behind any LB:
+- `POST /v1/sessions/:id/offer` on a non-owning node → "not found" → **404** (loud failure);
+- `DELETE /v1/sessions/:id` on a non-owning node → `sessions.remove` finds nothing, reply is
+  `()` regardless, route returns **204 unconditionally** (routes.rs:131–138) — the hangup
+  *reports success while the call keeps running*, burning trunk minutes and brain tokens. Once
+  the spend gate exists, "kill this runaway call" failing silently is a security-posture hole,
+  not just a routing bug.
+*(Verified first-hand after the workflow's verifier was lost to a session limit; every line
+re-checked.)*
+**Fix shape:** session directory (ChannelId → node) written at Register/Delete — Valkey per
+ADR-0005 is the obvious backend — plus one of: routing tier, consistent-hash LB, or
+node-addressed URLs returned at create. The `MediaCmd` seam gives the writes a clean landing
+spot; the missing piece is the *concept*, which also appears nowhere in ARCHITECTURE.md's
+horizontal-platform list.
+
+## Major
+
+### M1. No graceful drain — SIGTERM hard-drops every in-flight call
+`MediaCmd::Shutdown` is `sessions.clear(); return` (media_thread.rs:224–232) — it even skips
+the 750ms per-session tap teardown that `Delete` performs. axum's graceful shutdown drains
+HTTP requests; calls are not HTTP requests. main.rs:55–59 says it outright: "No in-flight call
+preservation story in the dev loop." *(CONFIRMED; not yet structurally baked — the fix is
+contained plumbing today — but it must land before the planned threadpool-shard graduation of
+this exact thread, or the two-state running/dead lifecycle gets baked into the shards, the
+readiness surface, and the trunk routing.)*
+**Fix shape:** a `Drain` lifecycle state: reject new Registers (503), flip readiness off, keep
+ticking until the map empties or a deadline, then Shutdown.
+
+### M2. No admission control, no capacity signal
+Register inserts unconditionally; the fixed `sleep(META_TICK)` (media_thread.rs:303) ignores
+how long the tick took, so saturation silently stretches the tick and degrades **every** call's
+20ms pacing at once — the worst overload mode for a real-time engine, and invisible: the
+session count surfaces exactly once, in the shutdown log. A node can never say "full" and an
+autoscaler has nothing to read. *(CONFIRMED. Notably: this repo documents its deferrals
+meticulously, and this one is nowhere documented — a genuinely silent gap. ADR-0010's benchmark
+needs the tick-lag gauge to produce its headline number.)*
+**Fix shape:** configurable max-sessions in Register (Err → 503), per-tick elapsed-vs-budget
+gauge, `MediaCmd::Stats { reply }` for the readiness/metrics surface.
+
+### M3. ADR-0005 exists only on paper — lifecycle events die with the process
+No bus, no `EventSink` trait, nothing durable: register/Connected/evict/Shutdown are tracing
+lines; the `Delete` handler drops `TapConn` without ever reading `conn.metrics`, breaking the
+in-code promise at tap_engine.rs:82–84 ("the eventual CDR/ringbus emitter"); `Channel` carries
+only a **monotonic `Instant`** — there is no wall-clock timestamp in the entire production
+workspace from which a CDR could even be built. An OOM-killed node erases all evidence of its
+calls. *(CONFIRMED, leaning baked-in: the "tracing is the record" assumption is reproducing
+slice-over-slice, and thread-shard graduation will multiply the emission points.)*
+**Fix shape:** minimal `EventSink` trait owned by the media thread (log-backed now, Valkey
+streams later), CDR-shaped started/ended events with wall-clock time + metrics snapshots.
+
+### M4. Tap protocol has no resume or terminal semantics — the calcifying one
+Brain bye, error, and stream-end all funnel into infinite re-dial (5s cap, forever); every
+reconnect restarts `seq_egress = 0` with a hello carrying no epoch/resume token; the reference
+brain acks any hello and opens a fresh OpenAI session. Mid-call reconnect = silently amnesiac
+brain **today, even at N=1**; a brain that deliberately ends a session gets re-dialed for the
+rest of the call. At brain-fleet scale: reconnects land on different instances with no
+protocol-level way to detect lost context, plus re-dial storms. *(CONFIRMED, baked in — this is
+a **wire-protocol** change, v2 + brain-side changes, the most expensive kind of retrofit. The
+protocol being versioned is the one mercy.)*
+**Fix shape:** resume token/connection epoch in hello + resume-ack/reject from the brain +
+terminal-vs-transient bye reason that exits the retry loop.
+
+### M5. Spend-gate accounting locality is undefined — and one ADR sentence steers it wrong
+The crate is a stub (nothing baked yet), but ADR-0009 prescribes where the *check* sits without
+distinguishing enforcement point from **accounting ledger**. Per-instance counters on a fleet =
+N× every cap, and toll-fraud thresholds that never trip because attempts spread across nodes.
+The subtle trap found in verification: ADR-0005's "the bus is NOT the source of truth for
+billing-critical state" is the sentence most likely to push the step-6 implementer toward local
+counters — but enforcement counters (rate/spend state) are not billing truth (durable CDR);
+they're different things with different stores. *(WEAK/major-borderline — nothing accretes yet;
+the fix today is one sentence.)*
+**Fix shape:** amend ADR-0009 now: "in-process enforcement, **shared accounting**" — gate built
+against a ledger trait with atomic check-and-reserve (in-memory impl for N=1, Valkey for N>1).
+
+### M6. Metrics are write-only — nothing aggregates, nothing exports
+`TapMetrics`/`ReflexMetrics` are per-session atomics with a snapshot method that production
+code never calls; no process-level registry, no calls-in-flight gauge, no Prometheus/OTel
+export. An LB health check and an autoscaler would both be reading a static HTML page today.
+*(Verified first-hand. The atomics+snapshot shape is export-friendly — the missing piece is
+the aggregation registry and one scrape endpoint, medium plumbing.)*
+
+### M7. Session teardown stalls every live call on the node
+`MediaCmd::Delete` runs `tokio_handle.block_on(timeout(750ms, &mut conn.join))`
+(media_thread.rs:205–217) **on the media thread, inside the tick loop**. One teardown with a
+slow/unresponsive brain freezes the 20ms loop for every other call on the node — up to ~37
+missed frames each; several Deletes drained in one batch stack sequentially. Under fleet churn
+(hangups are constant in a call center) this is a per-node isolation failure that worsens with
+density. *(Verified first-hand.)*
+**Fix shape:** hand teardown to a tokio task (fire close_tx, spawn the bounded join await);
+the tick loop should never block on brain I/O — same discipline the tap pipe already follows.
+
+## Minor
+
+- **Readiness/liveness absent** (routes.rs:151): only probe-able route is `GET /` static HTML —
+  returns 200 while the media thread is dead (its panic isn't monitored; sessions just 500).
+  Downgraded to minor by verification: the `MediaCmd` seam makes `/healthz` + `/readyz` trivial
+  additions, nothing accretes around their absence. Lands naturally with M1/M2.
+- **Trunk dispatch** (rutster-trunk stub): the "which instance gets this call's media fork"
+  question. Downgraded: Twilio/Telnyx hand out the stream URL per call by design, so answer-time
+  binding is the *default* usage — this needs one design constraint written into the step-5
+  spec, not code today. It is, however, the same placement concept as B2 — solve once.
+- **Tap URL loopback-only + wss:// not compiled** (routes.rs:87, workspace Cargo.toml:48):
+  documented step-6 deferral with the per-session override seam already in place. Non-obvious
+  detail from verification: no crate enables a TLS feature on tokio-tungstenite, so relaxing the
+  validator alone wouldn't help — the dial capability itself is compiled out, and the env
+  default goes through the same validator (no escape hatch).
+- **HTTP bind hardcoded** `0.0.0.0:8080` (main.rs:44); QUICKSTART literally says "edit main.rs
+  to change the port." `RUTSTER_TAP_BIND` in the brain binary is the pattern to copy.
+- **Brain endpoint static per-call URL** (tap_engine.rs:271): downgraded on verification —
+  `connect_async` re-resolves DNS each dial, so a brain fleet behind DNS/VIP works today; the
+  real calcification is M4's missing resume semantics, not URL indirection.
+
+## Rejected in verification (for the record)
+
+- *"EnvFilter `rutster=info` silences library-crate warnings"* — factually wrong;
+  tracing-subscriber matches by string prefix, so `rutster` covers `rutster_media::*` et al.
+  (verified empirically against the locked 0.3.23).
+- *"No packaging/compose artifact"* — documented later rung (DEVELOPMENT.md:172), purely
+  additive, nothing accretes around its absence.
+
+## What's genuinely clean (don't break these)
+
+- **`ChannelId` = UUIDv4** — globally unique, fleet-safe, doubles as the wire session id.
+- **The `MediaCmd` command channel** — the load-bearing seam. Drain, stats, admission,
+  directory writes, and event emission all have a natural landing spot because of it.
+- **Bounded backpressure with drop-counting on both tap directions** (32-frame mpscs, 5-frame
+  playout ring, drop-oldest, every drop counted) — a slow brain cannot eat unbounded memory.
+- **Per-call `tap_url` override** — per-call brain routing already exists.
+- **Versioned tap protocol** (`v:1` enforced at decode) — M4 has an evolution path.
+- **Zero production global statics** in the workspace.
+- **Tap is outbound from the call-owning node** — connection follows call ownership; exactly
+  right for multi-node.
+
+## Sequencing — what to bake in now vs. later
+
+The point is not to build the fleet now. It's that a handful of *concepts* are cheap to plant
+today and expensive to retrofit once slices accrete around their absence:
+
+**Now / next slice (cheap seams, prevents calcification):**
+1. `MediaAddressConfig` (B1) — also unblocks the first cloud demo at N=1.
+2. Admission cap + tick-lag gauge + `MediaCmd::Stats` (M2) — double-billed to ADR-0010's
+   benchmark harness, which needs the same instrumentation.
+3. Drain vocabulary (M1) + `/healthz` + `/readyz` — before thread-sharding.
+4. `EventSink` trait + wall-clock timestamps on `Channel` (M3).
+5. One-sentence ADR-0009 amendment: in-process enforcement, shared accounting (M5); clarify
+   ADR-0005's "not source of truth" ≠ "no enforcement counters in Valkey."
+6. Reserve resume-token + terminal-bye in the tap protocol's v2 plan (M4) — wire semantics
+   are the most calcifying surface in the whole system.
+7. Move Delete teardown off the tick loop (M7). Trivial, and it's a latency bug today.
+8. `RUTSTER_HTTP_BIND` (copy the existing pattern).
+
+**When N>1 actually lands (design then, not now — but write the concepts down):**
+- Valkey session directory + placement/routing tier (B2) — add "call placement" to
+  ARCHITECTURE.md's horizontal-platform list so the concept exists on paper.
+- Trunk step-5 spec constraint: media-stream URL handed out per call at answer time (placement
+  decision shared with B2).
+- Metrics aggregation registry + scrape endpoint (M6).
+- TLS on tap dial + validator policy (documented step 6).
+
+---
+*Method note: multi-agent review (7 survey lenses → dedup → adversarial verify → severity
+calibration), ~1M tokens of subagent reading; 5 of 17 findings re-verified by hand after their
+verifiers hit a session limit. All file:line citations checked against working tree @ d696536.*