feat(js/net): enforce the subscriber latency budget - #2919
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da5a4d2478
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.#closed.set(closed); | ||
| return; | ||
| } | ||
| if (this.#expire()) return; |
There was a problem hiding this comment.
When an open group has been handed out with unread frames and a newer group advances beyond its budget, a subsequent frame arrival on the old group wakes #runClosed; this unconditional #expire() marks the group as cleanly ended even though #state.frames still contains unread data. Later tryReadFrame or readFrame calls then return undefined, silently truncating an overlapping group despite the stated rule that buffered frames are always drained. Only apply this non-truncating expiry after the frame buffer is empty. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
| const drift = this.#drift(); | ||
| for (let i = 0; i < groups.length; ) { | ||
| const group = groups[i]; | ||
| if (!this.#isStale(group, drift)) { |
There was a problem hiding this comment.
When callers use Subscriber.readFrameSequence() or readFrame() on an open multi-frame group, this stale check supplies no cursor Position, so it compares the live edge with the group's first timestamp instead of the last frame consumed. For example, after consuming through 1900 ms in a group that began at 0, a 2000 ms group makes the old group fail a 1-second budget and removes it, even though a late 1950 ms tail frame is only 50 ms behind. Track-level reads need to measure drift from their current frame position just like handed-out group consumers do. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
The JS mirror of the moq-net work in the parent PR: js/net applies the same drift budget as it reads, and js/watch requests the playback latency ceiling from its first subscription. Drift is measured against both a presentation-time and a wall-clock live edge, from the reader's own position rather than the group's first frame, so a reader keeping pace is not convicted by how long ago its group opened. A group with frames in hand always drains; only a read that would wait is judged, which is also what keeps the policy off the read path. A drained group the budget gives up on ends rather than fails, since nothing was lost; the latency error is reserved for a caller still holding content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
da5a4d2 to
4b63fb6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b63fb6e74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| #evict() { | ||
| const frames = this.#state.frames.peek(); | ||
| if (frames.length > 0) { |
There was a problem hiding this comment.
When the only frame has already been dequeued into a slow IETF or Lite encode, frames is empty even though hooks.guardGroup still owns unread bytes. If retention pruning runs then, this condition skips expiry, Producer.#evict immediately removes the group from timeline, and subsequent guard checks can no longer recognize it as stale, so the blocked write continues past the latency and retention budget. Track pending guarded operations and expire them before removing the timeline entry even when the frame queue is drained. (Written by GPT-5.6 Sol)
Useful? React with 👍 / 👎.
|
Closed in error, not actually merged. A force-push of mine briefly put these commits onto the base branch ( Nothing landed on (Written by Opus 5) |
1 participant
Stacked on #2890, which does the same work in
rs/moq-net. Split out so each language is reviewable on its own; this was originally filed as #2892.Summary
js/netcarriedlatencyMaxas metadata and never acted on it. It now applies the same drift budget as it reads, andjs/watchrequests the playback latency ceiling from its first subscription instead of only after acceptance.performance.now()so it does not react to system-clock changes.Lagged).fetchGroupstays exempt: a fetch names historical content explicitly, so there is no live edge to be late against.Public API changes
None. Enforcement and the watch subscription plumbing use package-internal helpers (
hooksininternal.ts), so no exported surface moved.Wire behavior changes
None on the encoding. What changes is what a local subscriber is handed: a group that has drifted past its own
latencyMaxis skipped rather than delivered, and one that stalls after handoff ends (or fails, if content was unread) instead of waiting indefinitely. The publisher side resets a group stream whose encode or write is blocked as the group expires, which it previously would not have done. The default budget is zero, so a delayed live subscriber deliberately advances to the live edge; callers that need history must request a replay window.Cross-package sync
Mirrors
rs/moq-netin #2890. No draft change: the wire format and parameters are unchanged, and the existing draft already specifies the two age backstops.Test plan
nix develop --command just checknix develop --command just test: 3258 Rust tests pass, 2 skipped; all JS suites green, including 517@moq/netand 147@moq/watch.real time reads a live stream without truncating it— 2s GOPs read as they arrive at the default budget, with the group's close landing after its successor's first frame.a budget is measured from the reader's position— a straggler frame arriving after the next group opened still reaches a reader that is inside its budget.(Written by Opus 5)