Repository navigation
fix(net): a restarted publisher wins over its lingering session at once - #6452
Conversation
A publisher that restarts without a clean close announces its path under a new first hop while its old session lingers until the transport idles out. On the default wire (moq-lite-06) neither route carries an epoch, so the rendezvous hash ranked the two at random, and requests landed on the dead session: a viewer waited ~10 s and then failed `Unroutable`. `route_order` no longer hashes a route that entered on this origin at the requested path. No other node ranks it, so convergence does not need the hash, and the newest announcement wins. Peer routes, and a pool claiming a prefix spreading the paths beneath it, are hashed as before. 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 8 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. |
![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: 5952246a4f
ℹ️ 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".
A JS origin never peers, so every route it holds entered on it, as a client session does in rs/moq-net. Leave a tie at a route's own path to the newest instead of the spread hash, so a reconnect under a new hop wins there too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
A publisher that restarts without a clean close (SIGKILL, flaky uplink) leaves its old session on the relay until QUIC idles it out. The new process announces under a new first hop (its own random origin id). The default wire, moq-lite-06, carries no epoch: only lite-07 does, and it is opt-in, so even
main'smoq, which mints one, arrives epochless. The two routes then tie on everything up toroute_order's Spread Hash, which picks the dead session half the time.A viewer that requests the path in that window lands on the dead session. Its
catalog.jsonTRACK_INFO waits about 10 s, then failsUnroutablewhen the session idles out. A client that does not follow theRestart, such asmoq export, exits.Local repro: one
moq-relay,ffmpeg | moq import fmp4, SIGKILL both, restart the publisher at once, thenmoq export fmp41 s later. Onmain, 9 of 18 runs fail withmoq0.14.2, and 2 of 6 withmain's ownmoq.Approach
route_orderno longer hashes a route that entered on this origin (Link::Local: an in-process producer or a client session) at the requested path. No other node ranks that route, so convergence does not need the hash, andReverse(id)lets the newest announcement win.The hash still applies to:
equal_cost_pool_spreads_pathsis unchanged).With this PR, the repro answers the catalog within ~20 ms in 12 of 12 runs.
Tests:
replacement::restart_while_the_old_session_lingers_wins: the old session goes silent, the publisher restarts under another hop, and a viewer subscribes. It runs both hop orders on lite-06, lite-07, and moq-transport-22. Without the fix it fails withsubscribe hung.an_equal_cost_replacement_waits_for_the_old_routenow pins the peer case, renamed..._peer_replacement_.... The newan_equal_cost_local_replacement_takes_over_at_oncecovers the local case.source_flip_restartsanda_restart_pending_when_the_grant_grows_still_restartsannounce the competing route from a second local session instead of a peer. A peer route at an exact tie with a local one now loses, since the local one is not hashed.Impact
js/netmirrors it: a JS origin never peers, so it leaves any tie at a route's own path to the newest (compareRouteCandidates), and advertisements break a full tie by newest too. The JS pool-advertisement test now asserts that a replacement at its own path wins whatever its hop;an equal-cost pool spreads its pathsis unchanged.Alternatives
Restart. That keeps the wait, and clients that ignore announcements still fail.releaseand was closed as too risky to ship without a staging soak. This targetsmain, where staging runs it first.Follow-ups
moq export0.14.2 sometimes exits with "rendition 0.m4s went back in time" right after a clean subscribe: 4 of about 45 runs here, including runs that never touched the dead session. It looks like a separate exporter issue. Not investigated.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code