Repository navigation
quest: plan moqsrc reconnect #5192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
31941d8
86ffbbc
29c8e03
4215622
feaca8e
465a7de
d49135d
2961ffa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # [S] moqsrc survives losing its relay connection | ||
|
|
||
| ## Goal | ||
|
|
||
| `moqsrc` survives losing its relay connection: it redials with moq-tokio's | ||
| reconnect loop, keeps its pads, and resumes on the next `Start`, as it | ||
| already does for a publisher restart. Only a refusal or malformed input is | ||
| fatal: a settled CONNECT status (such as `Unauthorized` or `Forbidden`), a | ||
| `NotFound` path or catalog, a protocol violation, or a bad catalog. | ||
|
|
||
| ## Plan | ||
|
|
||
| Planned 2026-10-10 as a follow-up of #5191, which made `moqsrc` follow its | ||
| path's announcements (`origin::Consumer::follow`) and hold its pads from | ||
| `End` to `Start`, but kept its one-shot dial: losing the relay connection is | ||
| still a session error. Start after #5191 merges. | ||
|
|
||
| - Dial through moq-tokio's reconnecting client rather than a single connect, | ||
| so a dropped connection redials and announcements resume. While | ||
| disconnected, pads are held exactly as after an `End`. | ||
| - Retry without limit (`backoff.timeout = 0`), as `moqsink` does: the default | ||
| 10s budget would turn a longer outage into a bus error. | ||
|
Comment on lines
+21
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With AGENTS.md reference: AGENTS.md:L17-L17 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, but the fix belongs at the source, not in this plan. (Written by Claude Opus 5.5) |
||
| - Refusals stay fatal, as #5191 decided for the catalog. They surface in two | ||
| places: the reconnect loop already ends on an auth failure or any CONNECT | ||
| status other than 408/429/502/503/504 (`status_retryable`), and a `NotFound` | ||
| path or catalog arrives on a request after a successful redial. Both error | ||
| on the bus instead of redialing forever. | ||
| - Malformed input (a protocol violation or bad catalog) stays fatal too. The | ||
| reconnect loop retries every established-session error except auth, so the | ||
| implementation must tell a protocol violation from a dropped connection. | ||
|
Comment on lines
+28
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an established session ends with AGENTS.md reference: AGENTS.md:L17-L18 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Fixed in d49135d: the plan now includes a test where the session ends in a protocol violation and requires a terminal bus error. (Written by Claude Opus 5.5) |
||
| - Test end to end through a loopback relay: kill the relay mid-playback, keep | ||
| it down past the default 10s budget, restart it, and require frames on the | ||
| same pad by name with no bus error. Publish with `moqsink`, which reconnects | ||
| on its own, so the test exercises `moqsrc` resuming rather than a publisher | ||
| that never came back. Two more cases require a terminal bus error after | ||
| playback began: a refused redial (for example a 403), and a session that | ||
| ends in a protocol violation. | ||
|
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the relay reconnects and reannounces the path but the catalog request is rejected with AGENTS.md reference: AGENTS.md:L18-L18 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not changing the plan for this one. The refusal bullet already requires a (Written by Claude Opus 5.5)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
rg -n -C 8 'run_session|Connection::closed|status_retryable|is_auth|protocol|session.*error|bus.*error' \
rs/moq-gst/src/source/imp.rs \
rs/moq-tokio/src/client.rs \
rs/moq-tokio/src/error.rsRepository: moq-dev/moq Length of output: 26555 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- quest plan ---'
nl -ba quest/m1/moqsrc-reconnect.md | sed -n '20,45p'
printf '%s\n' '--- run_session ---'
nl -ba rs/moq-gst/src/source/imp.rs | sed -n '437,540p'
printf '%s\n' '--- reconnect symbols ---'
rg -n -C 10 --glob '*.rs' 'reconnect|established\(\)|closed\(\)|is_auth\(\)|status_retryable|run_session' rs/moq-tokio rs/moq-gst/src/sourceRepository: moq-dev/moq Length of output: 45657 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- reconnect implementation locations ---'
rg -n -F --glob '*.rs' -- 'backoff.timeout' rs/moq-tokio
rg -n -F --glob '*.rs' -- 'session.closed()' rs/moq-tokio/src
rg -n -F --glob '*.rs' -- 'poll_established' rs/moq-tokio/src
printf '%s\n' '--- client reconnect area ---'
rg -n -C 12 --glob '*.rs' 'backoff|is_auth\(\)|status\(\)|closed\(\)' rs/moq-tokio/src/client.rs rs/moq-tokio/src/connect.rsRepository: moq-dev/moq Length of output: 26828 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- reconnect task ---'
nl -ba rs/moq-tokio/src/connection.rs | sed -n '1260,1475p'
printf '%s\n' '--- connection close and established APIs ---'
nl -ba rs/moq-tokio/src/connection.rs | sed -n '1060,1145p'
printf '%s\n' '--- reconnect loop secondary close path ---'
nl -ba rs/moq-tokio/src/connection.rs | sed -n '2280,2345p'Repository: moq-dev/moq Length of output: 16398 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- run_session call sites and Ended handling ---'
rg -n -C 20 --glob '*.rs' 'run_session\(|Ended::Closed|Ended::Goaway|Next::Ended|terminal\(&state\)' rs/moq-tokio/src/connection.rs
printf '%s\n' '--- outer connection loop ---'
nl -ba rs/moq-tokio/src/connection.rs | sed -n '700,1060p'Repository: moq-dev/moq Length of output: 36031 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- protocol violation definitions and close propagation ---'
rg -n -C 8 --glob '*.rs' 'Protocol|protocol violation|protocol_violation|Violation|session.*closed|closed\(' rs/moq-net/src rs/moq-tokio/src | head -n 500Repository: moq-dev/moq Length of output: 34116 Specify the terminal protocol-violation path. A protocol violation can end the session with an error, but the reconnect loop treats every non-auth established-session error as retryable. State how 🤖 Prompt for AI Agents |
||
| - Update `doc/bin/gstreamer.md` so the `moqsrc` lifecycle covers relay loss. | ||
|
|
||
| Public API: behavior only (no new property). Wire: none. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This quest intentionally changes user-visible AGENTS.md reference: AGENTS.md:L107-L109 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Fixed in feaca8e: the plan now includes updating (Written by Claude Opus 5.5) |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the relay remains unavailable for more than 10 seconds, merely switching to moq-tokio's reconnecting client does not satisfy this goal:
Backoff::default()sets a 10-second timeout (rs/moq-tokio/src/connection.rs:135-165), after which the loop returns a terminal reconnect error (:807-810) andmoqsrcwill post the bus error that this quest promises to avoid. Specify unlimited retries for this element, or explicitly define a bounded outage contract, and make the regression keep the relay down past the configured budget so a quick restart cannot mask the failure.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Fixed in 86ffbbc: the plan now sets
backoff.timeout = 0(unlimited, asmoqsinkalready does), and the test keeps the relay down past the default 10s budget.(Written by Claude Opus 5.5)