[kixelated]

Problem

Nightly's benchmark_receive_windows failed, so the whole Rust benchmark tests job failed and no Nightly report was published.

• moq-dev/moq#7624 shrank the stock connection window from 128 MiB to 4 MiB. Since then, cells whose equal-priority tracks interleave incomplete units larger than the window hit the stuck-window eviction by design, and fail with frame: Evicted.
• Separately, moq-dev/moq#7729 made authored tracks default to a zero max age. It gave other integration tests' tracks with_max_age(None) but missed this ignored benchmark. Each benchmark track keeps up to 1 MiB of groups in flight, so its superseded groups expired at the publisher and never arrived. On main, even the default cell timed out (group timed out).

Decisions

All three were settled 2026-11-04 by the maintainer.

Overload policy:

• ✅ Preserve the offered workload, report delivered frames and evictions alongside throughput and latency, and keep zero-eviction assertions only for workloads whose incomplete units fit. This revises the quest Goal's acceptance.
• Not chosen: keep every cell lossless by changing production sender admission or scheduling.

Silent loss:

• ✅ Fix it at the source in the m1 quest quest/m1/group-drops.md, planned in moq-dev/moq#7890. There, the wire always names an abandoned group, and track::Subscriber's drop signal becomes public.
• Not chosen: a benchmark give-up timeout, or publisher-paced sending.

Split:

• ✅ Land the cells that fit now, and skip the three overcommitted cells (256 streams, 4 MiB frames, follow) until group-drops lands. This temporarily revises the Goal's rule against loosening what the benchmark measures, until those cells run again.
• Not chosen: keep everything waiting on group-drops.

Changes

• Fit criterion. Cell::fits: streams × min(frame, 1 MiB storage segment) ≤ connection window. Equal-priority tracks interleave by packet, so each can hold one whole frame, or one segment of a larger frame, before any completes. Cells that fit assert zero evictions and full delivery. measure counts an Evicted group as lost in a cell that doesn't fit, ready for when those cells return.
• Skip. benchmark_receive_windows splits its cells on Cell::fits. Each skipped cell prints with the reason waits on quest/m1/group-drops.md. These lines go to stderr, because the Nightly report parser reads every stdout line after the CSV header as a row.
• Reporting. The CSV adds delivered_frames and evicted_streams, each the median of the runs. sh/gh/bench-report.py reads them as frames (bigger is better) and streams (smaller is better) measurements, not as dimensions. They match by exact name, because other benchmarks print *_frames columns as workload dimensions. Existing series names are unchanged, and the parser test covers both columns.
• Max age. The benchmark's tracks set no max age, which restores the workload it measured before feat(net)!: authored tracks default to a zero max age #7729.
• Quest. The quest is rewritten to what's left: re-enable the three cells once group-drops lands, with its own ## Waiting outside condition. It records all three decisions.

Why the overcommitted cells wait

Each track paces itself on the groups its reader resolves, either delivered or Evicted. In overcommitted cells the subscriber also abandons some groups without recv_group ever returning them:

• a group stream evicted before its header was identified, or
• an evicted group whose refill FETCH was evicted too (traced as fetch abandoned).

Such a group's in-flight permit never returns, so the writer stalls and the reader times out. The 4 MiB-frame cell hit this on every run, and the 256-stream cell intermittently. Once group-drops lands, the reader returns a dropped group's permit and counts the group as lost.

Evidence

Final sweep on this head (medians of three runs):

path,axis,streams,frame,window_mib,stream_window,peak_bytes,mib_per_s,p50_ms,p99_ms,delivered_frames,evicted_streams,overflowed_packets
loopback,default,16,262144,4,finite,3234841,791,13.29,78.94,1024,0,0
bufferbloat,default,16,262144,4,finite,1763515,20,535.90,800.42,64,0,0
loopback,streams,1,262144,4,finite,1048619,767,1.17,3.52,1024,0,0
bufferbloat,streams,1,262144,4,finite,545486,19,43.12,155.36,64,0,0
loopback,frame,16,16384,4,finite,2277157,311,36.60,158.96,16384,0,0
bufferbloat,frame,16,16384,4,finite,306503,20,617.04,803.19,1024,0,0
loopback,window,16,262144,8,finite,4923927,809,15.53,53.93,1024,0,0
bufferbloat,window,16,262144,8,finite,2420830,20,655.91,807.06,64,0,0
loopback,window,16,262144,32,finite,5956958,568,22.81,82.22,1024,0,0
bufferbloat,window,16,262144,32,finite,2348858,20,660.65,814.64,64,0,0
skipped streams cell, 256 streams of 262144 B frames over a 4 MiB window: waits on quest/m1/group-drops.md
skipped frame cell, 16 streams of 4194304 B frames over a 4 MiB window: waits on quest/m1/group-drops.md
skipped follow cell, 16 streams of 4194304 B frames over a 4 MiB window: waits on quest/m1/group-drops.md

Regression boundary (unchanged benchmark, through quest gate):

Revision	Result
Before PR 7624	All 16 rows complete, three runs each; 73.9 s
PR 7624 merge	frame: Evicted in the 4 MiB-frame case; 42.7 s
PR 7597 parent / merge	Same failure in the 4 MiB-frame case
Main at claim	Same error in the 256-track case; 13.4 s
Main after #7729	group timed out in the default cell

An earlier attempt added a connection-wide admission semaphore. It was rejected: it changes the measured pressure and leaves admission wait out of frame latency.

Impact

• Public API: none. Wire and production behavior: none.
• Nightly: the benchmark passes with 10 rows (70 report measurements) instead of failing the job. The three overcommitted cells stop reporting until group-drops lands.

Validation

• quest gate just rs test -p moq-tokio --test integration --release -E 'test(benchmark_receive_windows)' --run-ignored only --success-output immediate: PASS in 24.3 s, output above.
• The sweep's stdout through bench-report.py's printed() and checked(): 70 measurements, units MiB/s, bytes, frames, ms, packets, streams.
• python3 sh/gh/bench-report.test.py: 15 tests OK.
• cargo clippy --locked -p moq-tokio --tests -- -D warnings, cargo fmt --all --check, git diff --check: pass.
• quest check --base origin/main, with quest freshly built from the merged checkout's pin: 608 documents ok.

Review

Codex via quest review --base origin/main --new, gpt-6.1-sol, medium effort, session 01a1a785-1b2b-79d2-a2f2-d64aaa8a5f0b. Round 1 at dec38b16ba: no findings.

Earlier rounds, by Claude Code, covered only the quest-only version of this PR. The approach changed since, as recorded in a Design change: comment.

Follow-ups

• When moq-dev/moq#7890 lands, it adds a ## Required edge from this quest to quest/m1/group-drops.md. This branch's quest file ends with ## Waiting where quest: report every group a subscriber gives up on #7890 appends ## Required, so a later merge keeps both sections.
• Once group-drops lands, re-enable the three cells (this quest).

(Written by Claude Opus 5.5)

[kixelated]

Design change: local review rejected the application-level semaphore because it changes the offered concurrency and excludes admission wait from latency. I removed it and restored the original benchmark and its quest. Both sides of PR 7597 reproduce the failure; the comparison at PR 7624 is running. This attempt will hand back a draft with an explicit maintainer choice between measuring intentional overload and changing production admission/scheduling. No policy choice is being made while the maintainer is AFK.

(Written by GPT-6.1 Sol)

[kixelated]

Design change: the PR no longer only records a policy wait. Per the maintainer's 2026-11-04 decisions, it now implements the overload policy: zero-eviction assertions only for cells whose incomplete units fit, plus delivered_frames/evicted_streams columns. It restores the benchmark tracks' max age, which moq-dev/moq#7729 had zeroed. And it skips the three overcommitted cells until quest/m1/group-drops.md (moq-dev/moq#7890) lands. Review restarts with a new session.

(Written by Claude Opus 5.5)

[coderabbitai]

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 23 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: ee60bf7a-0ea1-4bec-8e9e-47c40e180382

Commits

Reviewing files that changed from the base of the PR and between 7697e2d and dec38b1.

Files selected for processing: 4

• quest/m0/nightly/receive-windows-evicted.md
• rs/moq-tokio/tests/integration/receive_credit.rs
• sh/gh/bench-report.py
• sh/gh/bench-report.test.py

• 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

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head dec38b16baa4f371378c0700638058c3576f895c against base 7697e2d82073ea12f8222b0b797a047ead329c09. No actionable findings in the fitting-cell assertions, credit accounting, reporting/parser changes, or the documented split.

Verification: GitHub-only static review; I did not run the benchmark or reproduce scheduler behavior. The three overcommitted cells remain intentionally unvalidated until group-drop reporting is available.

(Written by review (OpenAI))

[pullfrog]

ℹ️ Minor suggestions only.

Reviewed changes

• Fit criterion and skips. Cell::fits compares streams × min(frame, 1 MiB segment) against the connection window, and benchmark_receive_windows partitions its cells on it. Fitting cells keep the zero-eviction and full-delivery assertions; the three overcommitted cells print a stderr skip line and wait on quest/m1/group-drops.md.
• Eviction accounting. measure now treats Error::Evicted from read_frame as a lost frame instead of panicking, records credit.evicted, and only asserts full delivery for cells that fit.
• Reporting. The CSV gains delivered_frames and evicted_streams, and sh/gh/bench-report.py recognizes those exact names as frames (bigger is better) and streams (smaller is better) measurements rather than dimensions.
• Max age. Benchmark tracks set with_max_age(None), undoing #7729's zero default that expired their superseded in-flight groups.
• Quest. Rewritten to re-enable the three cells once group drops land, with its own ## Waiting condition.

ℹ️ bench/README.md no longer lists every measurement column

sh/gh/bench-report.py now treats delivered_frames and evicted_streams as measurements, but bench/README.md:795-799 still says only the time columns, size_bytes/peak_bytes, mib_per_s, and *_packets are measurements and that "every other column is a workload dimension". The new columns contradict that documented contract.

Technical details

# Update the measurement-column contract in bench/README.md

## Affected sites
- `bench/README.md:795-799` — the documented set of measurement columns omits `delivered_frames` and `evicted_streams`.

## Required outcome
- The README's list matches `measurement_unit()` in `sh/gh/bench-report.py`, including the exact-name matches for these two columns.

ℹ️ The Nightly quest index still describes the old goal

quest/m0/nightly/README.md:38 still titles this child "The receive-windows benchmark runs to the end" and says it "stops failing frame: Evicted". The rewritten quest's goal is instead that the three overcommitted cells run again later, with the benchmark skipping them until then, so the index contradicts the quest it links to.

Technical details

# Refresh the quest index entry

## Affected sites
- `quest/m0/nightly/README.md:38` — link text and one-line summary describe the pre-rewrite goal.

## Required outcome
- The entry's title and summary match `quest/m0/nightly/receive-windows-evicted.md`'s new H1 and goal.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}