[kixelated]

Problem

An idle viewer session still costs its server a fixed amount of work every 100ms. A server holding thousands of mostly idle viewer sessions spent more than a core this way. Each session woke its driver ~35 times per second and read the QUIC path stats ~18 times per second, while moving almost no bytes. Two timers caused it, and each wake re-polls the whole session driver:

1. The send-bandwidth sampler never idled. Session::new stored producer.consume() in the Session handle, so bandwidth::Producer::is_used() was always true. Supervisor::poll_sampler therefore sampled every 100ms for the life of the session, even when nothing read the estimate.
2. PROBE serving ticked every 100ms. ProbeServe read the transport stats on a fixed 100ms tick for every open PROBE stream. Browser viewers keep one open for the whole connection. Most ticks then sent nothing because of the threshold and max-age logic.

Fix

• The handle keeps a bandwidth::Weak instead of a consumer, so only consumers a caller actually holds count as demand. A real send_bandwidth() consumer still gets 100ms-fresh samples.
• PROBE serving no longer has a timer of its own. While a PROBE stream is open, the supervisor samples on driver polls that already happen, at most once per 100ms, and publishes (send rate, rtt) only when it changes. ProbeServe wakes on that change, or when its own age deadline comes due. If nothing wakes the driver, the session is sending nothing, so the congestion controller has nothing new to report. The supervisor runs before the protocol in every driver poll, so the estimate PROBE reads is never more than 100ms old.
• The PROBE reporting rules are unchanged: the first report goes out at once, RTT moves by 25%, the bitrate threshold decays from 25% to zero over 10s, and the max age is 10s. The decay now arms a deadline at the exact age where a pending move becomes reportable, instead of rechecking on every 100ms tick.

Behavior change: a change in the congestion controller's estimate is reported when the session next does work, or at the 10s max age. This mostly matters for ACKs that land after writes stop. Before, a change was seen within 100ms.

Public API and wire impact

• Public API: none. Session::send_bandwidth() mints a fresh consumer per call, as the clone did before. Session::stats() is unchanged.
• Wire: none. PROBE carries the same messages under the same reporting rules; only when the publisher looks has changed.

Reproduction (rs/moq-net/tests/idle_session.rs)

One idle viewer holds a PROBE stream for 60s of simulated time; the test counts what the server does.

	driver polls / 60s	transport stats reads / 60s
main	2400	1200
handle holds a weak reference only	1200	600
this PR	12	6

Each driver poll is one wake, so the server-side wake rate drops from ~40/s to ~0.2/s. The only work left is one PROBE_MAX_AGE refresh every 10s. busy_viewer_sees_the_estimate_move covers the other side: a session the server is working for reports a moved estimate along with that work. It fails if passive sampling is removed.

Benchmark (cargo bench -p moq-net --bench session -- session_idle)

New session_idle group: N idle viewer sessions, each holding a PROBE stream, over the mock transport, on tokio's paused clock. The time is the CPU it takes to advance one second of protocol time, with both peers in one process.

idle sessions	main	this PR
1	36.0 µs	0.70 µs	51x
64	2.15 ms	22.5 µs	95x
1024	121 ms	1.23 ms	98x

Follow-ups

• Every timer costs two driver polls. Clock::advance wakes a due deadline through kio::Tasks, which re-wakes the driver while it is already being polled. That doubles the driver work for every timer in every session. It is a separate fix; this PR leaves only one timer per 10s on an idle session.
• PROBE on demand. Clients keep PROBE open whether or not anything reads it: js/net runs runProbe() unconditionally, and the Rust Session holds its own recv_bandwidth consumer. Planned in this PR as two m1 quests, quest/m1/probe-on-demand-rs.md and quest/m1/probe-on-demand-js.md.

Decision paper trail

PROBE wake design:

• ✅ Piggyback on the sampler: an open PROBE stream samples on driver polls that already happen, and ProbeServe wakes on a change or its age deadline
• Back off the PROBE tick from 100ms to a 1s cap while nothing moves
• Fix the sampler demand only

PROBE on demand (quest-plan):

• Scope: ✅ JS and Rust, as two separate quests / JS only / Rust only / drop it
• Milestone: m2 / ✅ m1
• Rust demand: consumers and stats reads / consumers only / ✅ (answer) an explicit consumer, which Session::recv_bandwidth() already is
• JS demand: ✅ (answer) either Signal observation or a consumer handle, chosen in the code
• Stats::estimated_recv_rate: ✅ remove it (breaking, dev) / keep it, documented

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[chatgpt-codex-connector]

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-13T02:51:18.116155Z	b363605	Manual request

ℹ️ 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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b2d3566c-1ea2-4fec-98ec-5ef7aa112be5

📥 Commits

Reviewing files that changed from the base of the PR and between c5b1150 and b363605.

📒 Files selected for processing (14)

• quest/m1/README.md
• quest/m1/probe-on-demand-js.md
• quest/m1/probe-on-demand-rs.md
• rs/moq-net/benches/session.rs
• rs/moq-net/src/client.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/lite/session.rs
• rs/moq-net/src/model/bandwidth.rs
• rs/moq-net/src/server.rs
• rs/moq-net/src/session.rs
• rs/moq-net/src/time.rs
• rs/moq-net/tests/idle_session.rs
• rs/moq-net/tests/support/harness.rs
• rs/moq-net/tests/support/mock.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

Walkthrough

Rust sessions now carry optional estimate producers, and Lite PROBE reporting uses transport estimates to send reports when values change or reach an age deadline. Tests and benchmarks cover idle sampling, updated estimates, and session polling. The quest notes also describe planned demand-driven PROBE behavior for Rust and js/net.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b3636

The change reduces idle-session wakeups while keeping PROBE reporting thresholds and public APIs unchanged. No concrete merge-blocking risk is identified.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 70 functions across 11 files. (3 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: preventing idle viewer sessions from waking every 100ms.
Description check		The description directly explains the idle-session problem, the implementation changes, validation results, API and wire impact, and follow-up work.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 70 functions across 11 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.1)

Clippy execution timed out

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

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

[kixelated]

@codex review

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed b363605 against base c5b1150. No actionable bugs found. Rechecked the base-aware delta against the earlier full inspection: the Rust implementation, tests and benchmark are unchanged; the new PR-specific edits clarify the Rust PROBE quest's branch and test path.

Direction looks sound: weak consumer demand and passive PROBE sampling remove both recurring idle timers without adding a backoff timer. The documented trade-off remains: ACK-only estimate changes can wait for subsequent driver work or the next max-age refresh.

Static review only; I did not execute tests or benchmarks.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: b363605d7f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kixelated]

Merge summary: stops idle viewer sessions from waking every 100ms. The Session handle now holds a bandwidth::Weak, so the send sampler idles unless a caller holds a consumer, and PROBE serving samples on driver polls that already happen instead of a 100ms tick. No public API or wire change; the documented trade-off is that ACK-only estimate moves are reported with the next session work or at the 10s max age.

Changes while merging:

• Merged main. fix(net)!: the application owns a replacement track's start sequence #5068 reformatted test/interop/clients/rust/tail.rs, so cargo fmt --all --check is clean again and the earlier Check failure is gone.
• Quest fix: probe-on-demand-rs.md said the breaking change "lands on dev" and pointed at this branch name; it now says main allows it and points at rs/moq-net/tests/idle_session.rs.

Verification: just check passes locally; the idle_session tests pass on simulated time; the session_idle bench reproduces the PR numbers (1: 0.69 µs, 64: 22.3 µs, 1024: 0.64 ms per simulated second). Codex and the OpenAI review both cover b363605 with no findings.

(Written by Claude Opus 5.5)