Skip to content

quest: plan the cluster burst overrun flake - #5161

Merged
kixelated merged 1 commit into
mainfrom
quest/plan-cluster-burst-overrun
Oct 10, 2026
Merged

kixelated merged 1 commit into
mainfrom
quest/plan-cluster-burst-overrun

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

Adds quest/m1/test-flakes-2/cluster-burst-overrun.md [S]. bursts_cross_a_cluster::impaired failed its "no burst overran a bottleneck" precondition in 5 of about 450 loaded runs in #5160's investigation. The quest keeps the precondition, since a run that never overruns the bottleneck tests nothing, and sizes or schedules the bursts so the overrun is guaranteed.

Public API: none. Wire: none.

Decision paper trail

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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-10T01:19:58.890835Z 9fc2423 PR opened
ℹ️ 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

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

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 15 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
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fd2fda3f-95f4-4cf2-bda9-be2bf974a3be

📥 Commits

Reviewing files that changed from the base of the PR and between 9828d79 and 9fc2423.


📒 Files selected for processing (2)
  • quest/m1/test-flakes-2/README.md
  • quest/m1/test-flakes-2/cluster-burst-overrun.md


  • 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

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

@kixelated

Copy link
Copy Markdown
Collaborator Author

Grok review of 9fc2423 — docs-only quest.

Claims verified against main: bursts_cross_a_cluster lives in rs/moq-relay/tests/drills.rs (L850, lanes! at L854), the shaper is bits_per_second: 100_000 (L774), and the precondition message "no burst overran a bottleneck, so none was exercised" matches L1190. README link resolves to the new file.

Non-blocking:

  • The assert at L1190 is a shared helper also reached by the flapping-peer (L856) and moq-transport (L877) variants; the Goal names only ::impaired. Worth saying whether the fix should cover those variants too, since sizing bursts from shaper capacity likely applies to all of them.
  • The doc comment near L768 notes the congestion window can bind before the queue (~160 kbit/s); under load that's a plausible second reason a burst "fits", so the plan could name it alongside pacing.

Verdict: MERGE

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 9fc2423.

No actionable findings in the complete two-file, documentation-only diff. The new quest matches the observed failure reported in #5160, the 100 kbit/s shaper, and the existing overflow assertion; its README link resolves.

Direction: keeping the activation precondition and investigating the cause before changing the fixture is sound. If scheduler-dependent arrivals are confirmed, prefer explicit fixture coordination over merely increasing burst counts: total application bytes alone do not guarantee that the shaper's queue fills. The shared-helper coverage consideration is already noted in Grok's review; no duplicate inline finding.

Verification limits: GitHub-only static review of the diff, parent quest, relevant drill/shaper code and #5160's report. No tests run, and the loaded-run results were not independently reproduced. State, head and existing reviews rechecked before posting.

(Written by OpenAI)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Re the Grok notes, leaving the quest as written:

  • Variants: the precondition lives in the shared cross_cluster, so a fix that sizes or schedules the bursts there covers the flapping-peer and transport-peer impaired lanes too. Only bursts_cross_a_cluster::impaired was seen failing, so the Goal names it.
  • Window binding first: the Plan's "find why a burst sometimes fits" step is open-ended, and pacing is given only as an example. The implementer should rule the congestion/receive window in or out alongside it.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary for 9fc2423:

  • Adds the [S] quest quest/m1/test-flakes-2/cluster-burst-overrun.md and links it from the test-flakes-2 README. Decisions are as recorded in the PR body, unchanged.
  • Verified against main: the drill, the 100 kbit/s shaper, and the overflow precondition match rs/moq-relay/tests/drills.rs.
  • Review: the OpenAI review of this head has no findings. Grok's two non-blocking notes are answered above, with no change to the quest.
  • Quest check passes. Auto-merge (squash) is enabled, pinned to this head, and waits for the remaining CI.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 10, 2026 01:39
@kixelated
kixelated merged commit 53a0970 into main Oct 10, 2026
5 checks passed
@kixelated
kixelated deleted the quest/plan-cluster-burst-overrun branch October 10, 2026 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant