Repository navigation
fix(net): a moq-transport FETCH run reads at its stream's speed - #8231
Conversation
Quest-Claim: 268ff44e8cad95e0
Remove the group-boundary pacing #8117 added: once a reader lost a group to eviction, the FETCH stream was held unread until a reader took the group before. Pacing between groups is head-of-line blocking, so a slow reader now falls back on Evicted and refill-once or a refresh, as on lite-07. 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 5 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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: b1f3b35.
No actionable findings in this removal. GroupFetchObjects now advances directly to RangeWriter::open, which still finishes the preceding group, validates ordering/range bounds, and settles errors. Within-group admission/payload pacing remains in FetchPayloadRecv and Reader; local batch-fill pacing and the eviction counter used by refill-once handling also remain.
Direction: keep the focused removal of cross-group wire pacing, consistent with the recorded decision to let slow readers use eviction/refill behavior. Verification was GitHub-only static review of the diff and relevant FETCH, reader, fill, miss, and test-transport paths. Tests, builds, and quest tooling were not run.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
A complete, faithful revert of the FETCH-run pacing added in #8117, per the 2026-11-07 no-flow-control-between-groups decision (#8215). I diffed this branch against #8117's own patch: every line that PR added is removed here, and the pre-#8117 shape is restored in each file.
GroupFetchObjectsreads at the stream's speed (rs/moq-net/src/ietf/subscriber.rs:7648): thepacedhold and the per-boundarypoll_pacecall are gone, leavingwriter.open(...)as the only step.RangeWriter::open(:539) callsfinish()itself, so the prior group still completes exactly as before; only the wait is removed.Fill::poll_run_paceddeleted (rs/moq-net/src/model/track/fill.rs:1049):Fill::poll_pacedstays and still has a live consumer in lite'sFillServeRun(rs/moq-net/src/lite/subscriber.rs:12326), which is the #8017 batch-fill pacing the body keeps.TrackState::evictionsis still bumped (rs/moq-net/src/model/track.rs:1132) and still read bytrack/miss.rs:662, so nothing is orphaned.Reader::poll_paceprivate again (rs/moq-net/src/coding/reader.rs:482): no callers outside the module (poll_admitat:474,poll_leaseat:394), so within-group pacing and thePausebudget are untouched.- #8117's test and hooks removed:
ScriptedSession::with_receive_window, itsreceive_windowimpl, and theStream::openroute inGroupFetchRun::with_sessionall go; the harness returns tosession.open_bi+Reader::new, matching pre-#8117. The remainingwith_receive_window/receive_windowin that file belong toSinkSession, notScriptedSession. - Quest bookkeeping:
fetch-run-unpaced.mdand its epic README entry are deleted, with no other references to the path or title left in the tree (includingsh/gh/test-inventory.allow). The epic README's remaining "paced" prose describes flights and rungs, not FETCH runs.
No stale prose about FETCH-run pacing remains in doc/, drafts/, or the crate. The residual behavior change is the one the maintainer decided: a run wider than the cache now evicts its own groups, and the existing, bounded refill-once path in track/miss.rs handles a reader that falls off the tail.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Finishes quest/m2/unbounded-groups/fetch-run-unpaced.md, per the 2026-11-07 /quest-unblock decision (moq-dev/moq#8215): no flow control between groups on either wire.
Summary
Undoes the moq-transport FETCH-run pacing moq-dev/moq#8117 added. Once a reader had lost a group to eviction, a FETCH stream was held unread at each group boundary until a reader took the group before. That is head-of-line blocking: under backpressure the head is evicted and cancels everything queued behind it. A FETCH run now reads at the speed its stream arrives, as a lite-07 history run does, and a reader that falls off the cache tail gets
Evicted, then refill-once or a refresh.track::Fill::poll_run_pacedremoved (rs/moq-net/src/model/track/fill.rs).GroupFetchObjectsno longer holds the next group's first object while pausing (rs/moq-net/src/ietf/subscriber.rs).Reader::poll_paceis private again, with its pre-feat(net): pace a moq-transport FETCH run once a reader lost a group to eviction #8117 doc (rs/moq-net/src/coding/reader.rs); within-group pacing still uses it.ScriptedSession::with_receive_windowand the test harness opening its stream throughStream::open.TrackState::evictions, whichtrack/miss.rsreads, and the batch-fill pacing of moq-dev/moq#8017 (holds no wire stream).Overlap checked: moq-dev/moq#8041 and moq-dev/moq#8201 touch the same files but neither removes this pacing; expect textual conflicts only.
Public API: none. Wire: none.
Validation
cargo clippy -p moq-net --all-targets --all-features -- -D warningscargo clippy -p moq-net --all-targets --no-default-features -- -D warningscargo fmt --check -p moq-netcargo test -p moq-net --all-features(all pass)quest checkFollow-ups
None.
Models
Review
Rewritten by
quest reviewafter each round.01a1b894-eb36-7cf1-8a4e-b1f96c27e561, round 1The focused removal preserves group completion, within-group pacing, and existing eviction and refill handling; builds and tests were not run, as instructed.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)