Repository navigation
quest(m1): bound what a stalled subscriber makes a publisher hold - #5965
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cbf61c8b7
ℹ️ 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".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Maintainer decisions: one [L] quest; ranked right after Rust request credit; linked, not folded. Sessions: admission control (per-token or per-address caps) belongs to the deployment's auth server, which can already refuse a connect, so nothing more is needed here. 68c1fb8 records that and adds a benign withholder (a receiver bounded by the receive-memory quest) to the reproduction. @codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68c1fb8aff
ℹ️ 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".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87f98ec4c6
ℹ️ 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".
|
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 7 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 |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Adds
quest/m1/egress-held-state.mdand ranks it in m1 right after Rust request credit. Planning only; nothing is implemented.Why
The egress audit in #5958 found that a downstream withholding stream credit across many tracks can make a relay hold an unbounded number of group serve machines, and with an inert cache pool unbounded bytes too. Verified on 8380595:
TrackRun::serve(rs/moq-net/src/lite/publisher.rs:3722-3744) and IETFTrackServe(rs/moq-net/src/ietf/publisher.rs:4002) create one machine per group with no in-flight bound. Each waits inOpenfor stream credit.Openarms check only expiry (lite/publisher.rs:3862-3867,ietf/publisher.rs:4697), andexpiry_pendingis false once a group is aborted (model/group.rs:1924,2174-2181). So a group the pool's 30 s idle window evicts leaves its machine parked forever.MAX_SAFE_DELAY_MS,lite/publisher.rs:90-104,ietf/publisher.rs:30-42). New finding: a lite-03+ peer can also name any max delay it likes (lite/subscribe.rs:65). Onlycache_duration(Duration::MAXby default,model/origin.rs:160) or the publisher'smax_ageclamps it.rs/moq-relay/src/limits.rs, 1000/10000 per client session). Nothing caps sessions (max_incomingbounds only pending handshakes).The quest states the goal and five abuse scenarios, including the benign one: a real viewer on a bufferbloated link must keep the live edge. It recommends directions without fixing the shape: serve state follows credit rather than arrival, unservable parked groups leave (named by SUBSCRIBE_DROP where the version allows it), and the serving budget is clamped by local retention. It also describes a reproduction (a credit-withholding or slow-draining subscriber, measuring held machines and billed bytes, swept tracks by groups) and says what done looks like.
Decisions
The maintainer approved planning this now. A background planning agent settled these, so challenge any of them in review:
[L]quest. Credit-driven serving would also remove the aborted-group leak, so splitting off the smaller fixes risks doing that work twice.Public API and wire impact
None. The quest expects no wire change and possibly a new relay
cache_durationdefault.quest checkandjust checkpass.Follow-up worth considering: an admission-control quest (sessions per token or address) if the drill shows sessions are the cheapest remaining abuse.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code