Repository navigation
fix(test): the rung test holds the bottleneck past BBR3's max-bw filter - #8590
Merged
Merged
Conversation
… BBR3's filter upstream_links runs relays over real TCP and a 50ms publish loop; without the quic feature no UDP socket exempts the process, so KIO_DENY_HOST_TIMERS aborts tiered_regions in the --no-default-features run. The rung test's 100 kbit/s phase was 12s, shorter than BBR3's two-cycle max-bw filter takes to forget the 500 kbit/s samples at a 500ms queue; nightly saw the grant fall 8-12s in and missed it once. Hold it for 20s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
marked this pull request as ready for review
November 12, 2026 20:35
Contributor
|
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. |
#8510 now carries the same fix in run_cluster_test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes: the single rs/moq-ffi/src/test/rung.rs hunk, plus how RECOVER, BELOW_LOWEST and END are used elsewhere in the test.
RECOVER32s → 40s: the sub-lowest-rung phase grows from 12s to 20s so it outlasts BBR3's max-bw filter. A comment records the reasoning. The step schedule (rung.rs:34), theelapsed >= RECOVERassertions (:156,:185) and the below-lowest window check (:180) all derive from the constant. The reported worst-case resume of about 70s still fits inside the 110sEND.
claude-sonnet-5-5 ｜ 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Nightly Tests (rs rung-disable) (today) fails:
the grant never fell below the lowest rung while the top stayed disabled. The 100 kbit/s phase lasts 12s (20s to 32s), but BBR3's max-bw filter keeps the 500 kbit/s samples longer than that. On the previous four nightlies the grant fell below the lowest rung at 28.0s, 30.3s, 31.0s, and 31.4s, and today it was still at 262 kbit/s when the path reopened at 32s. Locally it ranged from 21.4s to 30.9s.The other failing Nightly jobs are covered by moq-dev/moq#8510 (uring, features, drill-sensitivity, including the
upstream_linkshost-clock abort) and moq-dev/moq#8509 (armv7).Approach
The rung test keeps the 100 kbit/s bottleneck for 20s (
RECOVER32s to 40s). The filter holds two ProbeBW cycles, and the cycle the drop lands in must retire as well. That is up to three cycles, each a probe wait of up to 3s plus probe rounds at the RTT of the 500ms queue, so about 15s. The comment at the constant records this.Impact
rs rung-disable(nightly only) now takes 43-71s instead of about 40s. Recovery from 100 kbit/s ranged from 1.5s to 30s after the reopen, which is still well inside the 110sEND.Validation
just rs rung-disablesix times locally with this change: all passed. The grant fell below the lowest rung at 23.0s to 31.5s, and the top rung resumed at 41.5s to 70.1s. Before the change, six runs passed, but one dropped at 30.9s, 1.1s before the window closed.just checkpasses.Caused by
Models
Review
Rewritten by
quest reviewafter each round.Head: a8b4147
Base: origin/main at 3628a31
Reviewer: Codex, gpt-6.1-sol, medium effort
Session:
01a1d17a-721c-76f0-b989-e762f147dd9b, round 1Score: 9/10,
The longer bottleneck phase lets BBR3's bandwidth filter expire without weakening assertions or extending the deadline; source review found no actionable defects, but tests were not run as instructed.Findings: none