Repository navigation
test(net): record fastest-reader relay and qmux evidence - #7664
Conversation
Quest-Claim: 1e2652c8d6738507
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
kixelated
changed the title
# Conflicts: # sh/gh/nightly.sh
A session's ingest now stops reading a group's stream once it is the stream's receive credit past the group's fastest reader, capped at the 16 MiB ahead half of the live tail, instead of always 16 MiB. Each stream takes the window from its session's receive windows when it is accepted or opened. Local writers keep the 16 MiB window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rvey Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Design change: per the maintainer's 2026-11-03 /quest-unblock decision, ingest now paces each group at its stream's receive credit, capped at 16 MiB, instead of the fixed With 1 MiB of stream credit per hop, the two-relay test's downstream cache drops from about 16.7 MB to about 0.99 MB. The upstream cache, including sent history, drops from about 33.5 MB to about 3.5 MB. The quiet-host benchmark comparison is still waiting on a free host, so this stays a draft. (Written by Claude Opus 5.5) |
…oups/fastest-reader
The maintainer accepted noisy numbers on 2026-11-04, so the paired group benchmarks ran on nixos with load recorded per run. The transport-sized window costs nothing measurable over main; pacing itself costs group_relay_frames 4.6-8.1% on groups of 512+ frames, which the quest now waits on the maintainer to accept or trim. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… note main's drift 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 13 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 commit: f4d1a02
The per-stream pacing direction is reasonable and I found no actionable production pacing defect in static inspection. The new nightly comparison has a baseline-discovery issue below.
Static review only; I did not run tests or reproduce the reported measurements.
| ;; | ||
| # Keep added coverage out of the rust-bench and rust-shared legs. | ||
| rust-extra) | ||
| criterion qmux --manifest-path rs/qmux/Cargo.toml --bench receive |
There was a problem hiding this comment.
[P2] Scope baseline target discovery to this manifest. criterion_build_base recognizes -p/--package but not --manifest-path, so on a previous-nightly baseline without qmux's new receive benchmark it finds moq-quic's same-named target across the workspace and retains --bench receive. The qmux baseline build then fails rather than treating the benchmark as new, causing the paired scheduled gate to fail. Teach discovery to honor the manifest (and test a new target sharing another package's name).
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor doc omission.
Reviewed changes
- Transport-sized ingest pacing — a session's ingest stream now takes its pacing window from the session's receive credit (
min(stream, connection, 16 MiB AHEAD)) once at accept/open, instead of the fixed 16 MiB; local writers keepAHEAD.Pause::window,coding::Reader,recv::Group::poll_admit, and the boxed stream carry it through. - Two-hop retention evidence — new 64 MiB two-relay test pins each hop's cache charge while the final reader is parked, then verifies every byte and clean completion.
- Relay WebSocket accept evidence — a real
Relaytest decodes the QMux transport parameters advertised during the WS upgrade and pins the operator's windows and stream counts across qmux-00/01/02. - qmux receive benchmark — new
qmux_receivetarget sweeps pipe/WebSocket × versions × 1/8/64 sessions × 1/8 streams with a live-allocator heap counter, wired into Nightly'srust-extraleg. - Quest/docs — the finished quest is deleted, prose links point at this PR, and the pacing-report/relay-CPU follow-ups are recorded.
I traced the production path: the stored window is this endpoint's grant to the peer, window() min's it with the connection window and the 16 MiB tail half, and pause() still reserves min(stream, connection), so a paced stream always has credit to reserve; a reader that took every committed frame is never paced, so large frames still cannot deadlock. None windows keep the old AHEAD behavior, and nothing here changes the public API or wire.
ℹ️ Relay config reference still describes the old fixed 16 MB ingest window
doc/bin/relay/config.md:366 still tells operators the relay "receives at most 16 MB of it past the fastest one". After this PR the bound is the stream receive window capped at 16 MB (1 MiB at the client defaults), so the one user-facing place that explains the relay's memory under [cache] now overstates it by 16x. The same paragraph was updated in doc/concept/moq-lite.md, so this looks like a missed sibling rather than an intentional choice.
Technical details
# Relay config reference still describes the fixed 16 MB window
## Affected sites
- `doc/bin/relay/config.md:366` — "the relay receives at most 16 MB of it past the fastest one" predates the transport-sized window.
## Required outcome
- The paragraph states that a hop holds its stream receive window of a group past the fastest reader, capped at 16 MB (half the 32 MB tail), matching `doc/concept/moq-lite.md:397-401` and the PR's Impact section.
## Open questions for the human
- Whether to mention the client default (1 MiB) explicitly here, since operators tuning `[quic].stream_receive_window` now also move this bound.deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Problem
The remaining fastest-reader work after moq-dev/moq#7388 and moq-dev/moq#7624:
model::pace::AHEAD = 16 MiB, even with 1 MiB of stream credit.Decisions
Decided 2026-11-03 (maintainer, /quest-unblock):
model::pace::AHEAD = 16 MiB. Reason: memory follows what the peer can actually take in flight.Decided 2026-11-04 (maintainer): no test host is quiet, so the comparison accepts noisy numbers from the quietest host available, recording its load during each run. This replaces the earlier
## Waitingitem asking for a host without concurrent workloads.Approach
receive_window(model::pace::window,coding::Pause::window). The window is the stream credit, no more than the connection window, capped at the 16 MiB ahead half of the live tail (AHEAD).group::Producer::poll_paced,frame::Producer::poll_readyandrecv::Group::poll_admitnow take the window. Local writers (group::Producer::poll_ready/ready) keep 16 MiB, since they have no transport. A transport that reports no window never pauses, as before.Relayfrom operator configuration and inspect its WebSocket qmux transport parameters before admission. Pin connection, all stream receive windows, and stream counts across qmux-00/01/02.doc/concept/moq-lite.mddescribes the per-hop window as the stream receive window capped at 16 MB.Evidence
Two-relay cache charges while parked, with 4 MiB connection and 1 MiB stream credit per hop:
With the transport-sized window, the origin stopped after 327 of 1,024 frames. All 64 MiB arrived after reading resumed. Cache charges include frame slots and relay metadata. They exclude transport buffers and RSS.
The one-hop idle reader now holds three 1 MiB frames under 4 MiB of stream credit, and fifteen when the credit is past the 16 MiB cap.
All 36 qmux cases completed with exactly one heap report per case. With 64 sessions and eight streams per session, each receiver held 4,194,304 unread bytes:
How to read the qmux table:
Benchmark comparison (2026-11-04)
Maintainer decision 2026-11-04: no host is quiet, so accept noisy numbers on the quietest available host and record its load. Surveyed with
uptimeat 17:55 and 19:12 UTC:nixos2(12 threads): load 4.6 at 17:55, quietest then. It went to 9-12 once another session's pinned relay-CPU bisect (quest/m1/release-perf/relay-cpu-bisect) began running there. That session asked me to stop because each run voids the other's numbers, so I stopped my first round onnixos2and discarded it.nixos(Ryzen 9 5900XT, 16 cores, 32 threads): load 25-29, similar per core tonixos2, and no pinned benchmark ran there. All numbers below come fromnixos.kixtop(10 cores): load 15-24.Method.
cargo bench -p moq-net --features async,fuzz --bench group, built separately for each revision. Each run used Criterion with 1 s warm-up and 2 s measurement. The revisions alternated per group, and the order reversed every round. Each cell is the median over six rounds, with the spread as min/max relative to that median. Ratios are medians, and each geomean is taken over a group's cases.mainf37d964, and this head before the quest edit (fdec1c3,mainmerged in).group_paceddoesn't exist before pacing, so it pairsmainwith head only. 1-minute load: 8-49, 25-61, 26-52, 31-49, 15-65, and 3-32 in rounds 1-6.group_relay_framesgap at or before feat(net): pace relay ingest by the group's fastest reader #7388, and Run B attributes it.Summary (ratio < 1 is faster):
main(this PR)group_read_frames(35 cases)group_relay_frames(18)group_contended(36)group_paced(3)Reading it:
main.group_contended's 16-thread cases swing by two orders of magnitude between rounds at this load; its 1- and 4-thread cases geomean 1.036 and 1.021, against 1.185 at 16 threads.group_pacedis +1-6%, inside its -18/+45% round spread.group_read_framesis unchanged.group_relay_framesis 4.6-8.1% slower in every case of 512 or 8,192 frames at 1, 8, and 64 subscribers, and its 4-frame cases are unchanged. The bench frames carry 64-byte payloads, so a per-frame cost shows at its largest share. That fits the per-read progress report feat(net): pace relay ingest by the group's fastest reader #7388 added (Pace::tookinrs/moq-net/src/model/pace.rs) and the pace offset it stores on each log entry; it wasn't profiled. Media-sized frames dilute it.maindrift: in the noisy Run A,mainreadsgroup_relay_framesabout 15% slower than e7a5c10. Pacing explains about 4% of that, so most of it comes from other commits between e7a5c10 andmain.group_relay_frames, Run B: #7388 against its parent (load 1.8-16.5)group_relay_frames, Run Agroup_read_frames, Run Agroup_read_frames, Run Bgroup_contended, Run Agroup_paced, Run AImpact
[quic]windows) of a paced group past its fastest reader, instead of 16 MiB. A transport whose stream credit is 16 MiB or more paces at 16 MiB as before.Validation
All local.
just ciran on fdec1c3, the merge ofmain; head 23ffbbfa adds only the quest edit:just ci: every lane passed exceptcheck flake, including all 11,676 Rust tests (133 skipped).check flakeis a pre-existing failure. The Nixgithub-runnerimage build fails withunshare: write failed /proc/self/uid_map: Operation not permitted.mainf37d964 withCHECK_MODULES=flake just ci --all, on this host.quest checkandquest check --base origin/main.just checkon head passed every lane, including 11,676 Rust tests.test ghwas cut off by a tool time limit while it waited inquest gate.CHECK_MODULES=gh just ci --all).Follow-ups
{qmux}.{moq}pairs list only 00/01. Recommend a separate quest to decide whether to advertise versioned qmux-02 pairs; this attempt does not change negotiation.group_relay_frameswith 512+ frames.group_relay_framesdrift between e7a5c10 andmain. Measure it on a quiet host first, since Run A was noisy. The relay-CPU bisect inquest/m1/release-perf/may already cover it.Decisions (maintainer, 2026-11-04)
quest/m1/release-perf/pacing-report.md, added in this PR.main's remaining ~10%group_relay_framesdrift:This PR now finishes the quest. It deletes
quest/m1/unbounded-groups/fastest-reader.mdand points prose links to it at this PR.Review
gpt-6.1-sol, effort medium (quest review), on headf4d1a02dbe609a6dfcd412d528eeeb180c30c8aa, no findings. That head deletes the finished quest and adds the release-perf follow-ups on top of23ffbbfa5, which had its own clean round.quest check --base origin/main(605 documents) andsh/markdown.sh checkpass.🤖 Generated with Claude Code
(Written by Claude Opus 5.5)