feat(moq-net): expose broadcast demand on the read handle - #3330
Conversation
`Producer::demand` is public, `Consumer::demand` is not, so the only way to ask whether anything is reading a broadcast is to own the producer for it. That is the wrong half for a relay that pulls a broadcast from upstream: the session ingesting the remote announce owns the producer, while the code that has to decide whether the upstream connection is still worth holding open has a read handle and nothing else. The handle is already what the publishers use to price an announcement (`select_route` takes a `&Demand` derived from the consumer it is forwarding), so this exposes an existing internal signal rather than adding one. It is a weak handle: it neither keeps the broadcast alive nor counts as demand, so watching a broadcast cannot make it look watched. The spliced demand test now parks on the read handle for the unused edge, which is the shape a relay watches: a route-fed front whose logical tracks outlive any one session.
A publisher had no way to ask whether anyone was subscribed, so a capture pipeline ran whether or not it was being watched. The read handle now carries the demand the origin already tracks. Squashed from `pr/broadcast-demand` at 950827b, open as moq-dev#3330.
![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: 950827b47d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// producer for it (the ingesting session does), yet the question it has to | ||
| /// answer is whether anything downstream is still reading. Holding this | ||
| /// handle, or the [`Consumer`] it came from, is not itself demand. | ||
| pub fn demand(&self) -> Demand { |
There was a problem hiding this comment.
Making this method public changes the Rust moq-net API, but js/net/src/broadcast.ts::Consumer still has no broadcast-demand observer, so JavaScript consumers cannot implement the idle-pull lifecycle described here. The repository requires rs/moq-net API changes to be reconciled with js/net and doc/concept; add the corresponding JS surface and documentation, or explain why this API is intentionally Rust-only. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L199-L206
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. WalkthroughThe change makes Merge Risk: ⚪ Minimal · up to This change only exposes existing read-only demand information to holders of a broadcast consumer and does not change write, routing, subscription, or ownership behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
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 |
A relay watching a pulled broadcast has two endings to act on and one handle to learn both from. Readers going away means stop pulling; an upstream that vanished means the pull is over and retrying is pointless. They arrive as `unused` resolving and as `Error::Dropped`, which the docs now say on `Consumer::demand` rather than only on `Demand` itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A publisher had no way to ask whether anyone was subscribed, so a capture pipeline ran whether or not it was being watched. The read handle now carries the demand the origin already tracks, and the docs say how a relay tells lost readers apart from a producer that went away. Squashed from `pr/broadcast-demand` at ec836fe, open as moq-dev#3330.
2 participants
broadcast::Consumer::demandwaspub(crate). Making it public gives a holder that has only a read handle the same answerProducer::demandgives a writer: whether anything downstream is still reading.This PR is part of a series to update iroh-live to latest moq, see n0-computer/iroh-live#45. The code and below description was written by Claude Code
The case that wants it is a relay that pulls a broadcast from upstream. The ingesting session owns the producer, so the puller holds only a consumer, and the question it has to answer is whether any local subscriber is still reading what it mirrors. Without this it has no signal at all, and a transport-level one cannot substitute: keep-alives move every byte counter on a connection nobody reads.
For a route-fed front,
demandreports whether any spliced logical track has a live consumer, which is exactly the question. Holding the handle, or theConsumerit came from, is not itself demand.No behaviour changes. The visibility widens, the docs say what the consumer side is for, and the existing spliced-demand test now parks on the read handle for the unused edge, since that is the transition a puller acts on.
Verified
914 moq-net tests pass. Clippy clean for moq-net and moq-relay.
Downstream, this is what lets iroh-live's relay retire a pulled session once nothing reads it, with a test that fails when the call is stubbed out to return nothing.
Overlap with
devdevalready haspub fn demand(), but onProduceronly. This is the consumer-side sibling, so the two do not collide, though it is worth knowing that the branches have converged on the same idea from different ends.Review round
A relay watching a pulled broadcast has two endings to act on and one handle to learn both from: readers going away is
unusedresolving, while the upstream producer going away isError::Dropped. That distinction is now onConsumer::demandrather than only onDemand, and in a test.Cross-package sync
The table in
AGENTS.mdasks for a matchingjs/netupdate on anrs/moq-netAPI change, and this one skips that row deliberately.Broadcast-level demand has no JavaScript surface today:
js/net/src/broadcast.tsexportsProducerandConsumerwith nodemand,usedorunusedon either, and the demand that does exist in JS is per-track and per-group (track.Producer.used, and the group mirror counting ingroup.ts). So this is not a Rust-only API being introduced; it is one method reaching the read handle of a concept that was already Rust-only on both sides. Adding a JS broadcast-demand observer is worth doing, but it is the producer half's job as much as this one's, and it belongs in a change that designs the whole surface rather than in a one-method PR.doc/conceptlikewise documents demand at the track level, and nothing there becomes wrong.