Repository navigation
fix(net): close the relay demand gaps - #7052
Conversation
Quest-Claim: c1f9f0c188840278
A group a fill cached below the copy's start is no hole, so a later reader never asked for it, and the arrival walk never reaches backfill, so it never read it either: on moq-transport and lite-05 a newcomer on a warm relay got only the live edge. The miss judge now takes held backfill as filled. Also pins that a lite-07 max-age widening pulls nothing upstream for later newcomers, and records the remaining relay demand gaps in the quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A cold reader asks for a hole the live feed may still deliver only once its grace folds, so a held backfill there waits the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lite-06 resolves a copy's start from the budget but drops media time and sends FETCH without a max age, so a fill below that start pulled the whole backlog for any reader. Its readers now leave those groups to the publisher's re-offer of a widened budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 33 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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 3b9c417
No actionable bugs found in the changed areas. Serving cached backfill through the existing miss/fill path and gating below-start fills specifically for lite-06 are reasonable, contained fixes. The warm-cache and widening regression cases cover the intended distinction; accepting bounded duplicate upstream delivery is simpler than adding legacy-only per-reader floor history. The documented lite-05 and moq-transport age limitations remain outside this fix.
Verification: static inspection of the changes and surrounding miss, delivery, expiry, and version-gating logic. I did not run tests, builds, or interoperability checks.
|
Merging at
The follow-ups in the body (moq-transport backfill staleness, and the lite-05 reader with no max age) are left for (Written by Claude Opus 5.5) |
Finishes quest/m1/relay-demand-gaps.md and deletes it. Each of its three gaps is now fixed, ruled out, or accepted with a test pinning it.
Summary
I reproduced each gap through a relay. The setup was 10 groups one second apart, a 2.5s and a 6.5s reader, and a meter on the groups the publisher sends upstream.
Gap 2 (fixed): a newcomer on a warm relay got only the newest group.
Misses::judge(rs/moq-net/src/model/track/miss.rs). A reader takes held backfill as filled wherever it would ask a fill for a hole:Gap 1 on lite-06 (fixed): a narrow reader got the whole backlog.
lite::Version::fills_from_start(lite-06 only) has the subscriber mark the copy withtrack::Producer::fills_from_start, andMisses::judgedrops candidates below the start for it. A 2.5s reader now gets 6..9 on lite-06, directly or through a relay, as on lite-07.Gap 1 on lite-05 (ruled out). The copy starts at the live edge, and the wire carries neither a start resolved from the budget nor media time. Nothing on it bounds the backlog, so any bound would be a guess.
Gap 3 (accepted as bounded): one group pulled upstream twice.
widened_max_age::wider_newcomers_after_the_first_pull_nothing_upstreampins that.Decisions
Settled by the maintainer on 2026-10-28, each as this PR recommended:
Range::oldestreader on lite-05 as live-edge-only.Choices made
main, where the narrow reader got 0..9 up front. Skipping them would mean recording the floor and budget each SUBSCRIBE_START was resolved for, and judging walked groups below the start against them. That is a lot of new track state for a legacy wire, so I didn't.a_warm_relay_serves_a_wider_newcomer_what_a_cold_one_wouldpins the behavior per version.fills_from_startis keyed to lite-06's wire (start resolved, no media time, no FETCH max age), not to any untimed copy. A lite-07 untimed track's FETCH carries the reader's max age, so its fills are already bounded.judge, away from theMisses::retainand join bookkeeping that moq-dev/moq#6995 rewrites. Its only overlap is the struct, so a merge conflict should be small.refilled_sequence_stays_out_of_subscriptions. The arrival walk still never hands out the refilled group; the test now checks the walk directly. A newcomer now reads that group as the fill answer for the group the live feed gave up, which is what a cold reader asking a fill gets.Public API and wire impact
Tests
model::track::test::a_newcomer_reads_the_backfill_another_fill_cached(fails onmain: reads[2, 3]).model::track::test::no_fill_below_a_start_resolved_from_the_budget(fails without the flag: a fill is asked).warm_backfill::a_newcomer_on_a_warm_relay_gets_the_backfillover lite-05/06/07 and moq-transport-18/22 (fails onmainat lite-05: warm{9}).widened_max_age::a_widened_reader_gets_what_was_skippedanda_warm_relay_serves_a_wider_newcomer_what_a_cold_one_wouldnow run on lite-06 as well as lite-07. Both fail on lite-06 without the fix.widened_max_age::wider_newcomers_after_the_first_pull_nothing_upstream(lite-07, upstream metered bystats::Registry).main:just check origin/main(all 8968 tests, clippy, fmt, doc, shear, markdown, quest). Not run:just test interop --all(no wire change), JS (js/netis not a relay and was not touched).Suggested follow-ups
TrackState::successorskips backfill, so every backfilled group looks fresh. Judge a filled group by a cached backfilled successor too. Plan it after Linear fill path (moq-dev/moq#6995), which rewrites the successor search.Subscription::default()(nomax_delay) withRange::oldestreceived only{9}cold on lite-05, while a 100s budget received every group. This looks like the "not stated" vs "zero" confusionlite::Version::carries_max_delaywarns about, but I have not confirmed it.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code