Repository navigation
quest: the relay session hour is lost to a super-linear lite-07 route teardown - #8536
Conversation
Quest-Claim: 1ab77f4b48a02a81
… teardown 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 2 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. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ Minor suggestions only. The teardown diagnosis checks out against
route/node.rs,route/mod.rs, androute/view.rs, and the plan is executable as written.
Reviewed changes
- The m0 quest is rewritten around the root cause —
nightly-relay-session-hour.mdnow names the lite-07follow=1024cell's teardown (retire->disconnect->update->recompute->prune->restrict_all->merge_claims), resizes the quest from[S]to[M], records the maintainer decision and its order against the per-scope router, and turns the remaining work into an executable plan with afollow=1024-preserving acceptance check. - The router quest gains a teardown requirement —
routes/scope.mdadds one Work bullet keeping a session's teardown about linear in what it held, so the rebuilt router does not reintroduce the cost.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
| The job does not hang and the sweep did not grow: one cell's teardown went | ||
| super-linear. In | ||
| [the 11-10 run](https://github.com/moq-dev/moq/actions/runs/42842827718/job/129922278379) | ||
| was still finishing `relay_session_join_broadcasts` cells seconds before the | ||
| cancel. The scheduled runs from 11-01 to 11-05 failed inside the workload after | ||
| 14 to 20 minutes, the | ||
| [11-01 job](https://github.com/moq-dev/moq/actions/runs/41313466339/job/124868517800) | ||
| with `Evicted`. The | ||
| [11-06](https://github.com/moq-dev/moq/actions/runs/42236107954) and | ||
| [11-07](https://github.com/moq-dev/moq/actions/runs/42377867523) runs | ||
| completed the sweep in about 25 and 23 minutes (their jobs failed later, at | ||
| `Fail on paired regressions`). Find what grew between those sweeps and the | ||
| 60-minute cancellations from 11-08, then trim it or split the group across | ||
| matrix entries, as the Rust benchmark tests did. The sweep still covers both | ||
| fan-out axes. | ||
| each pass (this commit, then the base) spent about 22 minutes between the | ||
| `relay_session_delivery_prefixes/moq-lite-07-wip/broadcasts=1025/follow=1024` | ||
| cell and the next one; on | ||
| [11-07](https://github.com/moq-dev/moq/actions/runs/42377867523) that gap was |
There was a problem hiding this comment.
This baseline predates lite-07's move onto the route layer: the 11-07 Nightly head is b5805859a (run created 13:38Z on 11-07), while #8023 "lite-07 routes through the route layer" merged at 20:53Z that day, and #8352 (11-10 00:11Z) moved the bench runtime per cell so a cell's teardown is now charged before the next one starts. Read together with "the sweep did not grow", the gap looks like a regression in existing code, when it is the route layer landing plus per-cell teardown. Naming those two settles the "find what grew" question the previous text asked and keeps a later reader from hunting a regression commit that never existed.
Technical details
# The 11-07 comparison is a pre-route-layer baseline
## Affected sites
- `quest/m0/ci/nightly-relay-session-hour.md:12` — "the sweep did not grow: one cell's teardown went super-linear" reads as a regression in pre-existing code.
- `quest/m0/ci/nightly-relay-session-hour.md:18` — the 11-07 datum is offered as the before, but that build had no lite-07 routing at all.
## Evidence
- Nightly `runs/42377867523` created 2026-11-07T13:38:12Z at `b5805859a`.
- `ce4b849a1` "feat(net)!: lite-07 routes through the route layer (#8023)" merged 2026-11-07T20:53:29Z.
- `d9800c1a6` "fix(net): the session bench gives each cell its own runtime and a proxying relay (#8352)" merged 2026-11-10T00:11:08Z, moving `let rt = runtime()` inside each cell and so charging `retire`'s `drop(built)` per cell.
- The 11-10 gap and the 60-minute cancel match the per-cell `retire` wait (`rs/moq-net/benches/session/mod.rs:436`).
## Required outcome
- The plan says when the cost arrived (the route layer for lite-07 announce streams) and what makes Nightly charge it per cell, instead of implying a route-layer regression between 11-07 and 11-08.| of what the router holds, computed incrementally the way `show` is today, | ||
| with the test build comparing it against the whole computation. | ||
| - Keep a session's teardown about linear in the prefixes it held, as | ||
| [the relay session hour](/quest/m0/ci/nightly-relay-session-hour.md) makes it. |
There was a problem hiding this comment.
Completed quests are deleted from the tree (for example #8352 removed quest/m0/nightly/session-bench-handshake.md and its entry in quest/m0/nightly/README.md), and this fix lands first while the router waits behind its model, so this link will dangle by the time the work item is read. Cite the fix's PR or commit once it lands, or state the requirement without the link.
| prefixes as one batch. Keep `/verify-route` passing. Then confirm the Relay | ||
| session group finishes inside the hour (the 11-06 and 11-07 sweeps took | ||
| about 25 minutes) without dropping the `follow=1024` cell, which is the case | ||
| that exposed this. Add a route-layer benchmark sweeping held prefixes on |
There was a problem hiding this comment.
rs/moq-net/benches/routes.rs already sweeps route work over claims x streams (watched, unwatched, received, lagging, confirmed), so a teardown case belongs there rather than in a new bench: drop N announce streams over a table of C claims, over the same two axes. Naming it keeps the existing sweep the primary reproducer instead of leaving the Nightly cell alone with the cost.
Investigates Nightly's relay session benchmarks fit their hour and rewrites it to the root cause; no code changes yet.
Findings
relay_session_delivery_prefixes/moq-lite-07-wip/broadcasts=1025/follow=1024; on 11-07 that gap was under a second.AnnouncePrefixstreams drops aroute::node::Stream, which runsdisconnect->update->prune->View::restrict_allover every remaining region, whose per-regionmerge_claimsis claims x floors plus floors squared. About 1e9 steps per closed prefix.Changes
follow=1024cell.Decisions
restrict_all, indexed floors, batch disconnect), verified with/verify-route(rejected: (b) temporarily drop the lite-07follow=1024Nightly cell). Maintainer, 2026-11-12.Choices for review
pruneand the per-region floors this teardown spends its time in. I kept the fix before that rebuild and unblocked (noRequiredeither way), since Nightly loses its paired gate every night meanwhile, the rebuild waits behind its model, and the batch disconnect carries over. The quest steers the fix toward batch disconnect and single-passrestrict_allfirst, indexing floors only if still super-linear, to spend little on code the rebuild deletes. Alternative: fold it into scope.md and accept a failing Nightly until then.Public API / wire impact
None.
Follow-ups
None; the rewritten quest owns the fix.
🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with low effort)
Review
Rewritten by
quest reviewafter each round.01a1ce78-48a8-7600-a3f3-66b1e6a7bf3c, round 1The head is ready to land as a planning-only update: the teardown diagnosis matches the routing source, executable behavior is unchanged, and no actionable defects were identified.