Repository navigation
feat(lite): TRACKS lists a broadcast's tracks on lite-07 - #6057
Conversation
Consumer::tracks reports each listed track with its info, then Synced, then live changes. A front forwards its serving source's listing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A subscriber opens stream type 0x8 naming a broadcast path and epoch. The publisher answers with TRACKS_ADD (name plus TRACK_INFO), TRACKS_REMOVE, and one SYNCED, then FINs when the broadcast ends. A session syncs its readers at once and forwards the peer's tracks as live adds, each holding an announces slot. Fronts reached through a covering prefix refuse the listing with NOT_FOUND. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the Tracks Stream, TRACKS, TRACKS_ADD, TRACKS_REMOVE, and SYNCED to the moq-lite draft, a listing paragraph to the moq-lite concept page, and a bench sweeping listings, tracks, and broadcasts. Splits the JS half into quest/m1/archive/tracks-js.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
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 36 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: 4ab7b3a363
ℹ️ 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".
Each listing queues the broadcast's whole listed set but took one subscription slot, so a peer could list one large broadcast up to the cap and grow memory and fanout as tracks times listings. A newer listing of the same broadcast instance now cancels the older one, rather than being refused, since a subscriber that stops and relists can open the new stream before the old one's reset arrives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7dc3be90f
ℹ️ 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 replacement TRACKS listing shares the older one's subscription slot, so relisting at the cap no longer closes the session. - `Dynamic::lister` refuses a second lister with `Duplicate`: the model holds one answer and one sync flag, which two handlers would share. - A peer's FIN on the Tracks Stream ends the source's listing through the new `Lister::finish` and releases the slots it held, even before the retraction arrives. A front whose live source finishes its listing ends its own, so the end reaches downstream readers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2a9a6690a
ℹ️ 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".
Sharing one slot across a broadcast's listings left unresolved duplicates and other instances at the same path uncharged. Which listing a newer one replaces is known only after resolution, so each listing is charged, and a replacement must fit under the cap like a repeated SUBSCRIBE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c5109b95f
ℹ️ 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".
The session registry was keyed by path, so aliases of one broadcast each held a full queue. The broadcast's listing state now keeps at most one reader per owner (a session), ending the older with Cancel, so identity is the broadcast itself and the publisher-side registry is gone. Also keep an answered name listed without churn when its own track is replaced before the earlier one unlists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve against refresh demand (#6030), announce SYNCED (#6029) and the ietf extension (#6058): TRACKS_ADD now carries TRACK_INFO's Refresh, the draft shares one SYNCED section between the Announce and Tracks streams, and the ietf draft keeps the Track Stream for listed tracks since TRACKS_ADD has no TRACK_PROPERTIES. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85ca0479d5
ℹ️ 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".
| // and `Consumer::track` answers `Unroutable` for one asked about after this | ||
| // point. Say the same to whoever asked earlier. | ||
| state.reject_unserved(Error::Unroutable); | ||
| state.listing.close(); |
There was a problem hiding this comment.
When an upstream announcement is retracted before its independently ordered TRACKS FIN, SourceServe calls self.source.close() in subscriber.rs:1917-1920, and this new call terminates every listing reader with Ok(()). The downstream publisher consequently FINs its TRACKS stream and reports a clean broadcast end even though only the route disappeared, suppressing the error and re-request path; route loss should abort these readers, while clean completion remains reserved for publisher FIN or local shutdown.
AGENTS.md reference: rs/moq-net/AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Disagree. In moq-lite a retraction means the broadcast went offline at that hop, and a Tracks Stream FIN is defined as the broadcast ending, so a clean end is the right signal; announcements, not errors, are what a subscriber restarts on. Route loss with other routes left does not reach readers either: the origin front treats Ok(None) from a closing source as "move to another source" (it only ends its listing when no source remains), and a_front_reconciles_a_takeover covers the switch. Aborting here would turn every ordinary unannounce into an error downstream.
(Written by Claude Opus 5.5)
| if !self.synced && (self.bound == Some(0) || (self.bound.is_some() && self.pending.is_empty())) { | ||
| self.synced = true; | ||
| return Ok(Some(TracksEvent::Synced)); |
There was a problem hiding this comment.
With initial tracks a,b, reading a leaves bound == 1; removing a appends its removal after b, and reading b decrements the bound to zero, so this condition emits Synced before the removal. The newly added a_removal_of_a_seen_track_may_precede_synced test instead asserts +b, -a, synced, so the moq-net unit suite deterministically fails on this path; either distinguish events belonging to the opening bound or align the assertion with the implemented ordering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Disagree: the test passes. It lists three tracks (a, b, c), so the opening bound is 3, not 2. After reading +a the bound is 2; dropping c cancels its unread add, leaving b, -a queued, and both are read before the bound hits 0, giving +b, -a, synced as asserted. The two-track trace in the finding is not what the test does. cargo nextest run -p moq-net --all-features listing passes a_removal_of_a_seen_track_may_precede_synced on this head.
(Written by Claude Opus 5.5)
| loop { | ||
| let Some(event) = ready!(stream.reader.poll_decode_maybe::<lite::TracksEvent>(&mut cx))? else { |
There was a problem hiding this comment.
When a peer continuously sends a valid alternating sequence of TRACKS_ADD and TRACKS_REMOVE events, poll_decode_maybe can remain ready for as long as bytes are supplied, but this loop has no cooperative budget and each event also locks and fans out through the listing model. A hostile session can therefore monopolize the executor thread and stall unrelated sessions without accumulating entries or exceeding the announcement cap; add a bounded kio::coop::Budget yield as used by the publisher request loops.
AGENTS.md reference: AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed: fixed in 3546a6f. TracksRecv now spends a kio::coop::Budget::new(32) pass per decoded event, matching the publisher's request loops, so a churning peer yields the thread every 32 events.
(Written by Claude Opus 5.5)
Each event fans out to every reader of the listing, so a peer streaming add/remove churn could hold the thread while its bytes keep arriving. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge summary for 3546a6f:
(Written by Claude Opus 5.5) |
TRACKS (#6057) listed TRACK_INFO without the new Compression field: the publisher now sends it (lite-07 only, as TRACK does) and the subscriber keeps it on the listed track::Info. The listing tests cover it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 participant
Implements the TRACKS quest in Rust: a moq-lite-07 Tracks Stream lists one broadcast's tracks with their TRACK_INFO, marks SYNCED, and names removed tracks. The subscriber then SUBSCRIBEs per track. The JS half is split into
quest/m1/archive/tracks-js.md, which now gates the lite-07 cut in its place.Summary
rs/moq-net/src/model/listing.rs). A broadcast lists the tracks its producer creates (create_track,unique_track, andreserve_trackonce accepted) until they leave the track table. A Finish-held track stays listed. Request-served tracks are never listed, and listing never requests one.broadcast::Lister. While any lister exists, readers wait for it to sync.rs/moq-net/src/lite/tracks.rs). Stream type 0x8. The request isTRACKS {Broadcast Path, Epoch}. Replies are typed like lite-06+ announce messages:TRACKS_ADD0x0: the name plus an embedded, length-prefixed TRACK_INFO.TRACKS_REMOVE0x1: the name.SYNCED0x4: empty.TracksServerequest takes a subscription slot, is gated by subscribe auth like TRACK, and flushes before pulling each event, so backpressure collapses in the reader's queue.Lister.announcesslot. Past the cap the listing resets with TOO_MANY_REQUESTS and the session stays.Lister::finish) and releases the slots, without waiting for the retraction. A front whose live source finishes its listing ends its own.Unsupportedrather than answer empty.just drafts checkpasses.doc/concept/moq-lite.md.rs/moq-net/benches/tracks.rs). Sweeps listings × tracks per broadcast × idle broadcasts. A change costs O(listings of the touched broadcast) and is flat in tracks and broadcasts. A new listing costs O(tracks).Merge with main
Merged
origin/mainafter moq-dev/moq#6030 (refresh demand) and moq-dev/moq#6058 (theietfextension):Refreshboth ways, with an integration test (a_listing_carries_track_info).drafts/draft-lcurley-moq-ietf.md: a subscriber MUST NOT skip the Track Stream for a listed track when the extension is negotiated, since TRACKS_ADD has no TRACK_PROPERTIES. The Rust subscriber never skips it.Ownerandtracks_ascarry// private: capability gapmarkers for main's model-marker lint.announce-synced.mdandsubscribe-ok-start.mdlanded on main andtracks.mdlands here, so all three are gone;lite07-finalize.mdliststracks-js.mdand theietfprovenance quest.Public API impact
All additive, so there is no MIGRATE entry:
broadcast::Consumer::tracks() -> Result<Tracks, Error>broadcast::Trackswithpoll_next,next(async), andbroadcastbroadcast::TracksEvent::{Added { name, info }, Synced, Removed { name }}broadcast::Dynamic::lister() -> Result<Lister, Error>, failing withDuplicatewhile the broadcast has another lister (one answer per broadcast)broadcast::Listerwithpoll_used,poll_unused,add,remove,synced,clear,finish, andabortWire impact
New lite-07-wip stream type 0x8 with four messages. No published version changes; older versions reset the stream type, as they already do for unknown types.
A publisher resets an older Tracks Stream for the same Broadcast on a session with CANCELLED.
Decisions (confirmed)
The maintainer confirmed each decision below, taking its recommendation; the alternatives stay listed.
tracks()/Tracks/TracksEvent/Lister, mirroring the wire's TRACKS. Alternative:listing()/Listing/ListingEvent. Chosen and confirmed: keepTracks, since it matches the wire and the quest.Unsupported. A moq-transport session has no lister yet, so it lists nothing (synced, empty) until SUBSCRIBE_TRACKS lands.Unsupportedfor lite: fail loud, and the archive reports it as that broadcast's error rather than recording nothing.## SYNCED {#synced}section, where an Announce Stream ending before SYNCED is a protocol violation and a Tracks Stream FIN before SYNCED means the Broadcast ended. Alternative: give each stream its own SYNCED section.announcesclose the session with TOO_MANY_REQUESTS. A listing past it only resets its stream with TOO_MANY_REQUESTS, per the quest ("reset the listing rather than holding more"). Chosen and confirmed: keep it. Alternative: close the session, matching announcements.@moq/netalready resets type 8 on lite-07 (malformed.test.ts), so a Rust relay with a JS upstream fails just that listing withUnsupported.quest/m1/archive/tracks-js.mdnow gates the lite-07 cut. Chosen and confirmed: split. Alternative: land the JS half in this PR.Follow-ups
ietfextension negotiated it must not: TRACKS_ADD carries no TRACK_PROPERTIES, which theietfdraft now says.announces = 3. Something holds more than one slot per announced broadcast, perhaps the visible and hidden announce streams. Worth a look.Checks
origin/mainat eecb86c:just test interop --all,just drafts check,quest check. On the final head (with the TRACKS decode budget):just check.cargo nextest run -p moq-net --all-features: listing unit tests, front tests (listing, takeover reconcile, covering-prefix NOT_FOUND), lite codec tests, and integration tests (direct, through a relay with two downstream listings, a listed track served from cache, lite-06 refusal, and the announces cap).just drafts check,quest check,cargo bench --bench tracks.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)