Conversation
…he wire The outage test slept 1500ms on a paused clock while its re-check went through loopback HTTP. Under load the reactor's zero-timeout poll missed the request and auto-advance jumped past the sleep, failing "re-checks happened" about one run in eight. Hold auto-advance with a running blocking task while the re-checks are in flight, and nudge at two seconds: the second POST only goes out once the first 503 lands, so it proves the outage left the lease alone, and the expiry must still fire at the original deadline a second later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b067a9bf6
ℹ️ 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".
| let blocking = tokio::task::spawn_blocking(move || hold.recv().unwrap_err()); | ||
| let output = f.await; |
There was a problem hiding this comment.
Synchronize the clock holder before polling the future
When the blocking pool is busy or its new worker is slow to start, spawn_blocking only enqueues this closure and may return before it is running, yet f is polled immediately. During that gap Tokio can still auto-advance the paused clock to the three-second expiry while the test awaits loopback I/O, recreating the flake this helper is intended to eliminate. Have the blocking closure signal that it has started and await that signal before evaluating f.
Useful? React with 👍 / 👎.
|
Superseded by #4291, which fixed the same flaky (Written by Claude Opus 5.5) |
Problem
client::tests::an_outage_keeps_the_grant_until_expiresflakes under load. It pauses Tokio's clock, then sleeps 1500ms while the 1s cadence re-check goes over loopback HTTP. If the reactor's zero-timeout poll misses the request, auto-advance jumps past the sleep before the server logs it, and"re-checks happened"fails. Reproduced at 36-41 of 300 runs with a CPU hog; 0 of 300 before the hog.Approach
frozen(f)keeps aspawn_blockingtask running whilefruns. Tokio documents that auto-advance is paused while a blocking task runs, so loopback I/O completes before any lease timer can fire.frozen, advance to 2s, nudge, wait for re-check Improve readme #1, then nudge again. A nudge's POST only goes out once the in-flight 503 lands, so re-check Add server-side ABR and throttling (to test) #2 proves the outage left the lease alone. If the lease closes instead, the test panics rather than hanging.After the fix: 0/300 under the same load.
Sibling tests checked under the same load:
a_grant_within_clock_skew_stays_liveandexpiry_fires_while_a_recheck_is_stalled(paused clock + HTTP) passed 0/300 failures, since neither result depends on I/O finishing before a timer fires.a_close_is_reported_while_a_recheck_is_stalledruns on real time with wiremock and passed 0/100.Impact
Alternatives
Follow-ups
None.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code