Skip to content

fix(net): close the relay demand gaps - #7052

Merged
kixelated merged 7 commits into
mainfrom
quest/m1/relay-demand-gaps
Oct 28, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m1/relay-demand-gaps

Conversation

@kixelated

@kixelated kixelated commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

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.

  • Below the copy's start, groups arrive through fills, outside the live feed's arrival walk.
  • A newcomer found those groups cached, so it never asked for them. The walk never reaches them either.
  • The bug was not specific to moq-transport. It affects any copy whose start sits above a backfill, including lite-05 when the newcomer changes nothing upstream.
  • The fix is in 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:
    • under the feed's start, or once the feed will never deliver it: taken at once;
    • a gap the live feed may still fill: deferred until its grace folds, as a hole would be.

Gap 1 on lite-06 (fixed): a narrow reader got the whole backlog.

  • Lite-06 copies are untimed (their timestamps may be send times), and their FETCH carries no max age. So a fill below the copy's start was never judged against the reader's budget.
  • The publisher already resolves the start from the budget (SUBSCRIBE_START says 6), so only the FETCH backfill of groups 5..0 was excess.
  • Such a copy now asks no fills below that start. lite::Version::fills_from_start (lite-06 only) has the subscriber mark the copy with track::Producer::fills_from_start, and Misses::judge drops 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.
  • A wider budget that lowers the resolved start reaches the groups between through the publisher's re-offer (moq-dev/moq#5929), not through fills. The tests cover both a reader widening its own update and a wider newcomer on a warm relay.
  • This also removes lite-06's re-offer of groups the relay had already backfilled.

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.

  • A warm lite-07 relay pulls 9 upstream groups to serve 8 distinct ones, with or without 50ms of link latency.
  • The newcomer's fill FETCHes one group below the start while the widening's re-offer sends it too.
  • This happens once per widening of the relay's aggregate max age, not once per newcomer, so it does not grow with subscribers. widened_max_age::wider_newcomers_after_the_first_pull_nothing_upstream pins that.

Decisions

Settled by the maintainer on 2026-10-28, each as this PR recommended:

  1. Gap 1 on lite-06.
    • (a) ✅ An untimed copy whose start the publisher resolved from the budget asks no fills below that start. A floor later lowered by an update relies on the publisher's re-offer (moq-dev/moq#5929), with a test.
    • (b) Rule lite-06 out as a legacy-wire limitation.
  2. Gap 1 on lite-05.
    • (a) ✅ Rule it out: the wire states neither a start resolved from the budget nor media time.
    • (b) Treat a budgeted Range::oldest reader on lite-05 as live-edge-only.
  3. Gap 3's one group per widening.
    • (a) ✅ Accept it as bounded, keeping the test that pins it.
    • (b) Fix it: hold below-start fills after a widening, or skip re-offering a group the same session is fetching.
  4. moq-transport backfill staleness (outside this quest).
    • (a) ✅ Leave it as a suggested follow-up, not this PR.
    • (b) Fix it here.

Choices made

  • After a widening, a narrow lite-06 reader also takes the re-offer. A lite-06 copy has no media time, and each re-offered group arrives just now. So the narrow reader on the same relay reads 2..5 when a wider newcomer joins; on lite-07 it skips them. This happens once per widening, and only for groups within the widest budget. It is still well below 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_would pins the behavior per version.
  • fills_from_start is 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.
  • I kept both fixes in judge, away from the Misses::retain and join bookkeeping that moq-dev/moq#6995 rewrites. Its only overlap is the struct, so a merge conflict should be small.
  • I updated 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.
  • Gap 3 has a test instead of a benchmark. It does not scale with subscribers, which was the plan's reason for a benchmark.

Public API and wire impact

  • Public API: none.
  • Wire: none. A relay now serves cached backfill to a later reader where it used to serve nothing, and a lite-06 copy sends no FETCH below its resolved start.

Tests

  • New model::track::test::a_newcomer_reads_the_backfill_another_fill_cached (fails on main: reads [2, 3]).
  • New model::track::test::no_fill_below_a_start_resolved_from_the_budget (fails without the flag: a fill is asked).
  • New integration warm_backfill::a_newcomer_on_a_warm_relay_gets_the_backfill over lite-05/06/07 and moq-transport-18/22 (fails on main at lite-05: warm {9}).
  • widened_max_age::a_widened_reader_gets_what_was_skipped and a_warm_relay_serves_a_wider_newcomer_what_a_cold_one_would now run on lite-06 as well as lite-07. Both fail on lite-06 without the fix.
  • New integration widened_max_age::wider_newcomers_after_the_first_pull_nothing_upstream (lite-07, upstream metered by stats::Registry).
  • Passed locally after merging 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/net is not a relay and was not touched).

Suggested follow-ups

  • moq-transport backfill staleness. Moq-transport over-delivers the backlog to a narrow reader, even though its copy is timed. A filled group is judged only against its live successor, because TrackState::successor skips 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.
  • Lite-05 reader with no max age gets only the live edge. Subscription::default() (no max_delay) with Range::oldest received only {9} cold on lite-05, while a 100s budget received every group. This looks like the "not stated" vs "zero" confusion lite::Version::carries_max_delay warns about, but I have not confirmed it.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 4 commits October 28, 2026 00:02
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>
@kixelated kixelated changed the title Relay demand gaps left by the widened-age re-offer fix(net): a newcomer on a warm relay reads the cached backfill Oct 28, 2026
kixelated and others added 3 commits October 28, 2026 08:08
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>
@kixelated kixelated changed the title fix(net): a newcomer on a warm relay reads the cached backfill fix(net): close the relay demand gaps Oct 28, 2026
@kixelated
kixelated marked this pull request as ready for review October 28, 2026 15:26
@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a7cae40a-29e8-415b-b6ce-64eeb1cecb20
Commits

Reviewing files that changed from the base of the PR and between 4fdbad7 and 3b9c417.

Files selected for processing: 9
  • quest/m1/README.md
  • quest/m1/relay-demand-gaps.md
  • rs/moq-net/src/lite/subscriber.rs
  • rs/moq-net/src/lite/version.rs
  • rs/moq-net/src/model/track.rs
  • rs/moq-net/src/model/track/miss.rs
  • rs/moq-net/tests/integration/main.rs
  • rs/moq-net/tests/integration/warm_backfill.rs
  • rs/moq-net/tests/integration/widened_max_age.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T15:28:14.274454Z 3b9c417 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merging at 3b9c41712794d3fb5e846375004d790f358cd7e6.

  • Review: Codex reviewed this head and had no findings (👍). The automated OpenAI review on the same head found no actionable bugs. CodeRabbit was rate-limited, so it doesn't count. No threads are unresolved.
  • CI: 12/12 passed. The merge is clean against main, and the queue re-runs Check against the latest main.
  • Quest: quest check --base origin/main passes. quest/m1/relay-demand-gaps.md and its link are deleted.
  • Decisions: all four were settled by the maintainer as recommended. The maintainer accepted the listed choice that, on lite-06, a narrow reader still receives groups that a widening re-offers. a_warm_relay_serves_a_wider_newcomer_what_a_cold_one_would pins that behavior.
  • Overlap: moq-dev/moq#6995 touches the same miss.rs struct. Whichever PR lands second resolves the conflict.

The follow-ups in the body (moq-transport backfill staleness, and the lite-05 reader with no max age) are left for /quest-plan.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit bdfa5b1 Oct 28, 2026
12 checks passed
@kixelated
kixelated deleted the quest/m1/relay-demand-gaps branch October 28, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant