fix(relay): keep only finished groups warm when a track goes idle - #3977
Conversation
When the last reader leaves, the origin front parks the track: it copies the cached groups to a warm local track, then drops the source copy, which cancels the upstream subscription and resets the group still in flight. The warm copy had adopted that open group, so its resume boundary pointed mid-group into a now-dead head. A rejoin then subscribed upstream from that frame and the group never reached the returning reader. Adopt only finished groups. The re-splice resumes at the open group's first frame and delivers it whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Merged (squash, 5056a96).
(Written by Claude) Generated by Claude Code |
Problem
Found while chasing the Smoke "audio/video sync: after rejoin" flake (player-side fix in #3973).
When the last reader leaves a relayed track, the origin front parks it: it copies the cached groups to a warm local track, then drops the source copy. That cancels the upstream subscription and resets the group still in flight. The warm copy had adopted that open group, so the logical track's resume boundary pointed mid-group (
(18, N)) into a head that was now aborted.On rejoin the relay re-spliced from that boundary and sent upstream
SUBSCRIBE { Group Start: 18, Frame Start: N }. It only got the tail, and nobody could read the head, so group 18 never reached the returning reader. The player got 17 (warm) and then 19.The cache itself is not at fault.
create_groupalready evicts an aborted incarnation (claim_sequence), the same waywriteGroupdoes in JS.The repro needs lite-06 in the browser because pre-lite-06 peers widen a mid-group boundary to frame 0. In Rust-to-Rust relaying the group is lost on lite-05 as well.
Approach
warm_copynow adopts only finished groups. An open group is left to the re-splice, which resumes at its first frame and delivers it whole.Regression test
rs/moq-net/tests/rejoin.rs(mock transport, lite-05 and lite-06): leave mid-group, the publisher cuts the group, rejoin, and group 1 must come back with every frame. It fails on both versions without the fix.Impact
group::Producer::is_finished.Group Start = <open group>, Frame Start = 0instead of a mid-group frame.Alternatives
track::resume_positionignore an aborted group'sresume_frame. That would break failover, where readers still hold the aborted head and want only the tail.Follow-ups
Group Startto last+1. No draft change.🤖 Generated with Claude Code
(written by Claude Opus 5.5)