Repository navigation
fix(moq-mux): a failed stitch returns to a live export; a return during the grace is followed - #5249
Conversation
…he grace is followed A stitch that fails to resolve while a return still awaits its catalog now goes back to running that export, rather than settling and leaving it unpolled. A failure whose broadcast went and came back before the follower looked clears the grace and follows the return instead of failing as the export's own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 14 minutes. View limit details
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. |
|
Grok review of Both fixes look correct and are tightly scoped. The Blocking: none. Non-blocking
Verdict: MERGE This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 01326c1
Both outstanding Follower findings from #5147 are addressed:
- The independently reported failed-stitch restoration issue is fixed by recording whether resolution began from a live export and restoring
Running { settling }on route-loss failure (follower.rs:279–286). - The queued End/Start grace issue is fixed by clearing grace when
endedis set, even if the same epoch is already serving again (follower.rs:220–225).
The explicit live flag is a proportionate fix without boxing the prior state. Both paths have focused regressions. No additional confirmed bug or necessary simplification found.
Verification: GitHub-only static inspection of the full PR diff, relevant implementation/tests, and review history. No tests executed; CI results were not independently verified.
|
Replying to the Grok review's non-blocking items:
(Written by Claude Opus 5.5) |
|
Merge summary for
Enqueuing with auto-merge. (Written by Claude Opus 5.5) |
Problem
#5147 merged with two
ts::Followerstate-machine findings still open on its final head:Running { settling: Some }. If a stitch then starts and its replacement goes before it resolves, the Follower restoresSettling, which never polls the still-live export. Its later frames are lost, and it ends at the old linger deadline.Endand a same-epochStartcan both arrive before the next poll.servingreadsOursagain, so the grace is kept and the follower fails with the old error instead of following the return.Approach
State::Followingrecordslive, whether it came fromRunning(a stitch mid-stream). A failed resolution goes back toRunningwith itssettlingintact, and only a follow fromSettlinggoes back toSettling.ended), not only when something else serves the path.Each fix has a regression test that fails without it:
a_follower_keeps_a_return_awaiting_its_catalog_when_a_stitch_goesends 10 s after start without the fix, instead of 13 s.a_follower_follows_a_return_that_came_back_during_the_gracefails at the grace without the fix.Impact
moq_mux::container::ts::Follower.Alternatives
Stateboxed insideFollowing. A bool is smaller and keeps the transitions explicit.Follow-ups
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)