Skip to content

fix(net): keep driving cache-miss fetches through subscription waits - #7903

Merged
kixelated merged 8 commits into
mainfrom
quest/m0/backport-fill-head-race
Nov 5, 2026
Merged

kixelated merged 8 commits into
mainfrom
quest/m0/backport-fill-head-race

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

An IETF subscription's cache-miss fetches (group_fetches) are only polled by a few of subscribe_once's waits. While any other wait runs, they stall:

  • PUBLISH_DONE grace. A miss parked on the joined group's head isn't polled. So when the subscription's own fill settles during the grace, its FETCH waits until the grace ends. The grace waits for every stream still being read, so it can run long past GRACE. Fetches already in flight also stop being read. The release backport moq-dev/moq#7680 adds this poll on release, and the main-first backport rule needs it on main first.
  • Resumed SUBSCRIBE setup (main only). When a reader returns to a lingering copy, the copy carries its in-flight fetches into subscribe_once. Four waits there leave them unpolled until SUBSCRIBE_OK arrives, or forever if the publisher never answers: the listed-track handoff, the request-stream open, the answer wait, and the cancel on abandonment that returns to linger. So a reader that leaves cannot cancel its FETCH.

Approach

Poll group_fetches in each of those waits. The cancel-then-linger path drives them with TaskSet::drive, as the existing cancel at the end of the function does by hand. Paths that return None drop the set, along with the track it served, so they need no change.

Each fix has a reproduction that failed on unfixed main:

  • a_miss_fetches_during_the_publish_done_grace: on unfixed main, no FETCH goes out during the grace.
  • a_resubscribe_keeps_driving_the_lingering_fetches: on unfixed main, dropping the reader stops no stream while SUBSCRIBE is unanswered.

The quest is rewritten to what remains, which is the maintainer's review and merge of moq-dev/moq#7680 and the moq-net patch release.

Impact

  • Public API: unchanged.
  • Wire: unchanged. FETCH and its cancellation go out sooner; their encoding is unchanged.

Alternatives

  • Fix only the grace, matching the backport. This would leave the same stall in the four setup waits of the function being fixed. release has no counterpart: it creates its fetch set after SUBSCRIBE_OK and never lingers, so moq-dev/moq#7680 needs nothing more.
  • Drive the fetch set once, around the whole subscribe_once/linger cycle in run_subscribe. This would make the set impossible to forget in any later wait. But it means pushing through a Tasks handle, and that breaks drain's "finish when the children finish" semantics. It is a larger refactor of a 17k-line file that moq-dev/moq#7670 is also changing.
  • Also accept new fill requests during the grace. A miss that arrives during the grace stays queued in the track's requests and is served by linger afterward, or refused when the subscription failed. That is a delay bounded by the grace, not a stall, and release behaves the same way.

Choices made without asking: I included the setup-wait fix in this PR, rather than as a follow-up, because it is the same stall in the same function.

Validation

  • Both new tests failed on unfixed main and pass with the fix.
  • just check passed: rs lint and clippy, and 10022 Rust tests.

Review

  • Codex (quest review), gpt-6.1-sol, medium effort, round 1 against origin/main, reviewed 527e4c9c17: no findings.

Follow-ups

None.

(Written by Claude Opus 5.5)

kixelated and others added 8 commits November 2, 2026 21:31
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
The PUBLISH_DONE grace and a resumed subscription's setup waits stopped
polling the track's group fetches, so a miss the joined head released, or
a reader cancelling its FETCH, stalled until the wait ended.

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 Ship the joined-head FETCH race backport fix(net): keep driving cache-miss fetches through subscription waits Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 18:58
@coderabbitai

coderabbitai Bot commented Nov 4, 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 3 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: a6d2ea3f-a4f4-44b4-ae0f-60c06239826c
Commits

Reviewing files that changed from the base of the PR and between f37d964 and 527e4c9.

Files selected for processing: 2
  • quest/m0/backport-fill-head-race.md
  • rs/moq-net/src/ietf/subscriber.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

@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: 527e4c9

[P2] Drive FETCH cancellation during failed resumed setup too — rs/moq-net/src/ietf/subscriber.rs:3436–3441. A lingering copy can have an unanswered cache-miss FETCH when SUBSCRIBE resumes. If its grant is revoked before SUBSCRIBE_OK, this branch aborts the track, then awaits cancel_subscribe without polling group_fetches. That await can remain pending on the SUBSCRIBE FIN acknowledgement. Aborting the track only wakes the FETCH task: it must still poll fill.poll_aborted to unregister and send STOP/RESET. Consequently the FETCH remains uncancelled until that unrelated ACK or session teardown. Returning None eventually drops the set, but cannot do so while this await is pending. Apply the new drive pattern to these terminal cancellation waits too (also lines 3402 and 3551), and extend the resumed-setup test with revocation and an unacknowledged FIN.

The grace and normal resumed-setup changes address the intended stall, but the same cancellation problem remains on failed setup.

Verification limits: GitHub-only static tracing of subscription/FETCH ownership, TaskSet and transport close semantics, with comparison to the existing cancellation-before-ACK tests. I did not run the new tests or reproduce the failure.

(Written by review (OpenAI))

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Grace wait drives fetches. End::Done's PUBLISH_DONE grace now polls group_fetches, so a cache miss waiting on the joined group's head FETCHes as soon as the subscription's fill settles rather than after the grace (which can run well past GRACE).
  • Resumed SUBSCRIBE setup drives fetches. The listed-track handoff, request-stream open, and answer waits now poll group_fetches, and the abandonment path uses group_fetches.drive(...) around cancel_subscribe, so a leaving reader cancels its FETCH even when the publisher never answers the resumed SUBSCRIBE.
  • Two reproductions. a_miss_fetches_during_the_publish_done_grace and a_resubscribe_keeps_driving_the_lingering_fetches, plus a JoinRun::subscribe helper that accepts a prebuilt ScriptedSession.
  • Quest bookkeeping. quest/m0/backport-fill-head-race.md now points at this PR as the main-side fix for the release backport (#7680) and leaves only review/merge and the patch release as the remaining wait.

I confirmed both tests pass at head and are not tautological: removing the grace poll makes the grace test see 0 FETCHes during the grace, and removing the answer-wait poll leaves the dropped reader's FETCH uncancelled. The quest's claim about release also checks out: its run_subscribe has no linger/resume cycle, so the backport needs only the grace poll. The per-wait poll pattern matches the existing linger/learn_track_status code, and the body records why the single outer-drive alternative was rejected.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit edc7ef7 Nov 5, 2026
13 of 14 checks passed
@kixelated
kixelated deleted the quest/m0/backport-fill-head-race branch November 5, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant