From b1502d70f566488c95e4549e98bf84d576f04dc8 Mon Sep 17 00:00:00 2001 From: CoffeeKnyte <67730400+CoffeeKnyte@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:10:22 +0000 Subject: [PATCH] docs(playback): score the tracker-lifecycle batch and correct the async claim Marks GAP-15 resolved and records the three wrong-over-cap-count defects the tracker-lifecycle batch closed, with why they had to precede the revocation batch: decision A1 removes the self-healing that limits the damage of a miscount. Notes why the v3 identity split had not yet caused visible harm -- fresh v3 starts sent no owner attribution at all, so the transport record landed under user 0, which the enforcer skips, silently exempting the stream from the cap rather than double-counting it. That is a worse failure than the double count it masked, and worth recording so the next reader does not "fix" only the visible half. Corrects the "fully async monitoring" claim instead of rushing the queue: the first Redis projection write per session is synchronous so the record is visible before the request returns, and later liveness and byte updates ride the refresh tick. The consequence -- a slow Redis adds latency to the first request of a stream -- is now stated. The ordered, lifecycle-aware projection queue is deliberately deferred until its startup, drain, cleanup ordering, backpressure and refresh interaction can be designed together; a naive fire-and-forget projection is exactly what caused the ghost-session defect this batch fixed. Follow-up list renumbered accordingly; the GAP-14 (A7) and opMu items are retained, not dropped. Part of #305. --- .../playback-paths-monitoring-kill-matrix.md | 44 ++++++++++++------- docs/architecture/stream-abuse-matrix.md | 28 +++++++++--- 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/docs/architecture/playback-paths-monitoring-kill-matrix.md b/docs/architecture/playback-paths-monitoring-kill-matrix.md index ced21978..70fc5dc7 100644 --- a/docs/architecture/playback-paths-monitoring-kill-matrix.md +++ b/docs/architecture/playback-paths-monitoring-kill-matrix.md @@ -35,8 +35,22 @@ > **Update 2026-07-30 — serve-path batch landed.** **GAP-10, GAP-11, GAP-12 and > GAP-13 are RESOLVED**; each is scored inline below. GAP-12 was pulled forward from > the revocation batch because it makes the GAP-10 fix inert (see its entry). -> **GAP-14 and GAP-15 remain open** — GAP-14 to the liveness/replica batch under -> decision A7 (fail closed + connection cap), GAP-15 to the tracker-lifecycle batch. +> +> **Update 2026-07-30 — tracker-lifecycle batch landed.** **GAP-15 is RESOLVED** +> (edge visibility is now separate from served liveness, and only bytes written from a +> 200/206 upstream response advance `LastServedAt`). Also closed in that batch: the +> overlapping-request defect that let one finishing request delete another live +> request's record, the async-tracking race that could leave a permanently +> non-expiring ghost session, and the protocol-v3 logical-vs-transport identity split +> that counted one stream twice — all three were sources of a **wrong over-cap +> count**, which is why they precede the revocation batch. **GAP-14 remains open**, to +> the liveness/replica batch under decision A7 (fail closed + connection cap). +> +> Monitoring projection is **not** uniformly async, and the claim has been corrected +> rather than the code rushed: the first Redis write per session is synchronous, later +> updates ride the refresh tick. An ordered, lifecycle-aware projection queue is +> deliberately deferred rather than shipping a lossy or reorderable one — a naive +> fire-and-forget projection is what caused the ghost-session defect above. > > Two claims elsewhere in this doc are therefore **still not true** and are called out > where they appear: the kill switch does not "keep a stream dead" (an over-cap kill @@ -179,19 +193,19 @@ bytes but withholds/falsifies progress must still be counted. holds exactly: `Session.LastServedAt` is written only by `BeginTransport`, `EndTransport` and `AddServedBytes` (`playback/session.go:1132,1151,1169`) — no progress-report path touches it. - ⚠️ **One edge exception (GAP-15, open):** the proxy's transcode path calls - `touchTranscodeSession` immediately after token verification and *before* proxying - to the node (`proxy/server.go:308,317`), so `Touch` advances the edge record's - `LastServedAt` even when the node returns a 404 and no media byte is served. Edge - transcode liveness is therefore **request-observed, not byte-observed**. The blast - radius is bounded: the enforcer groups by user and keeps the freshest sessions - (`streamenforcer/enforcer.go:140`), so a client hammering dead segment URLs can - only change *which of its own* over-cap streams gets trimmed — it cannot exceed the - cap or shield another account. Worth fixing so the claim above is literally true. -- Report-to-central: **C** = async via Redis (edge writes `silo:sessions:*`, - `streammonitor.RedisSource` reads). **A/B** = in-process SessionManager IS - central; read via `streammonitor.FuncSource`. MultiSource unions both, deduped - by session id. + **GAP-15 — RESOLVED.** The proxy now creates transcode visibility separately + from served liveness. Only bytes actually written from a 200/206 upstream + response advance `LastServedAt`; zero-byte responses, upstream failures, and + non-success error bodies do not. +- Report-to-central: **C** = Redis-backed (edge writes `silo:sessions:*`, + `streammonitor.RedisSource` reads). The first projection write for a session is + synchronous so visibility is established before the request returns; later + liveness and byte updates are asynchronous on the refresh tick. Consequently, + a slow Redis adds latency to the first request of a stream. An ordered, + lifecycle-aware projection queue is deliberately deferred rather than making + this path lossy or reorderable. **A/B** = in-process SessionManager IS central; + read via `streammonitor.FuncSource`. MultiSource unions both, deduped by + canonical logical session identity when available. - Owner + attribution: the transcode node's start record now carries owner + route + client (threaded via `TranscodeStartRequest`), and `streammonitor.mergeStreams` additionally backfills any missing owner/route/client from another record for the diff --git a/docs/architecture/stream-abuse-matrix.md b/docs/architecture/stream-abuse-matrix.md index 46db3b04..316e0bd4 100644 --- a/docs/architecture/stream-abuse-matrix.md +++ b/docs/architecture/stream-abuse-matrix.md @@ -78,6 +78,16 @@ The branch docs oversell several things. The stories below are scored against th > durable lesson. Correction **8 (GAP-14) remains open**, now with decision A7 > attached. See the follow-up list at the end for what is left. > +> **Update 2026-07-30 — tracker-lifecycle batch landed.** GAP-15 is also fixed, along +> with three defects that all produced a **wrong over-cap count**: overlapping edge +> requests deleting a live record, an async-tracking race that could strand a +> permanently non-expiring ghost session, and the protocol-v3 logical-vs-transport +> identity split that counted one stream twice (masked until now because fresh v3 +> starts sent no owner at all, so the record landed under user 0, which the enforcer +> skips — silently exempting the stream from the cap). These precede the revocation +> batch on purpose: decision A1 removes the self-healing that limits the damage of a +> miscount, so the count must be trustworthy first. +> > Two claims this document makes elsewhere are **still false** and must not be > restored to blanket phrasing: the kill switch does not "keep a stream dead" (an > over-cap kill still reopens after 5m until the revocation batch lands), and @@ -550,17 +560,23 @@ enhancements.** In rough fix-cost order: can never reach it. Hence the context side channel. The re-applying watcher was also kept, as belt-and-braces. (GAP-12) 0d. ~~**Merge `BytesServed` as a max, not wholesale**~~ — **DONE** in both - `mergeStreams` and `DedupeSessionInfos` (GAP-13). **Still open:** advance the edge - transcode record from real bytes rather than request entry (GAP-15) — deferred to - the tracker-lifecycle batch, which also fixes the overlapping-request defect that - makes edge record lifecycle unsafe to touch piecemeal. -0e. **Decide the registry-saturation policy explicitly** (GAP-14) — **DECIDED (A7), + `mergeStreams` and `DedupeSessionInfos` (GAP-13). **GAP-15 is also DONE:** edge + transcode visibility is created before proxying, while liveness and byte totals + advance only for bytes actually written from a 200/206 upstream response. +0e. **Monitoring projection is not uniformly asynchronous.** The first Redis write + for each edge session remains synchronous so the record is visible before the + request returns; subsequent liveness and byte projection happens asynchronously + on the refresh tick. A slow Redis therefore adds latency to the first request of + a stream. The proposed ordered, bounded projection queue is deliberately deferred + until its startup, drain, cleanup ordering, backpressure, and refresh interaction + can be designed together. +0f. **Decide the registry-saturation policy explicitly** (GAP-14) — **DECIDED (A7), not yet implemented:** fail *closed* for download-class pours once the registry is full, **and** add a per-user/credential concurrent-connection cap (E28) so saturation is unreachable by a single actor in the first place. Scheduled for the liveness/replica batch. Today it still fails open, logging at Debug at the call sites. -0f. **Bound the kill-list propagation lock.** `RevokeWithWarnings` holds `opMu` +0g. **Bound the kill-list propagation lock.** `RevokeWithWarnings` holds `opMu` across durable-Postgres and Redis I/O and strips the caller's deadline with `context.WithoutCancel` (`streamrevoke/store.go:313,330`). A hung Redis or an exhausted PG pool blocks every subsequent revoke/unrevoke indefinitely. The hot