Skip to content

test(auth): hold the paused clock while the outage re-checks are on the wire - #4264

Closed
kixelated wants to merge 1 commit into
mainfrom
claude/auth-outage-test-clock
Closed

kixelated wants to merge 1 commit into
mainfrom
claude/auth-outage-test-clock

Conversation

@kixelated

@kixelated kixelated commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

client::tests::an_outage_keeps_the_grant_until_expires flakes 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

  • New test helper frozen(f) keeps a spawn_blocking task running while f runs. Tokio documents that auto-advance is paused while a blocking task runs, so loopback I/O completes before any lease timer can fire.
  • In 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 freeze, expiry must fire within 2s (the deadline is at 3s). This makes the fix(auth): keep accepted grants on fixed expiry deadlines #4237 regression check deterministic: recomputing the deadline on every loop iteration now fails 5/5 runs, where the old test caught it in 1 of 3.

After the fix: 0/300 under the same load.

Sibling tests checked under the same load: a_grant_within_clock_skew_stays_live and expiry_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_stalled runs on real time with wiremock and passed 0/100.

Impact

  • Test-only. No public API or wire changes.

Alternatives

  • Wait on the log instead of sleeping: fixes the assertion, but the 503 reply still races the expiry timer, so the test would no longer prove the outage kept the lease.
  • Unpaused real time: slower, and it loses the paused-clock coverage fix(auth): keep accepted grants on fixed expiry deadlines #4237 added for fixed deadlines.

Follow-ups

None.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

…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>
@kixelated
kixelated marked this pull request as ready for review September 26, 2026 17:44
@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 51a503ff-73dd-4ce9-a278-475b8a010435

📥 Commits

Reviewing files that changed from the base of the PR and between 7910cc5 and 6b067a9.

📒 Files selected for processing (1)
  • rs/moq-auth/src/client.rs

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 26, 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-09-26T17:47:27.120387Z 6b067a9 Draft marked ready
ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread rs/moq-auth/src/client.rs
Comment on lines +354 to +355
let blocking = tokio::task::spawn_blocking(move || hold.recv().unwrap_err());
let output = f.await;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Superseded by #4291, which fixed the same flaky an_outage_keeps_the_grant_until_expires by running it on the real clock.

(Written by Claude Opus 5.5)

@kixelated kixelated closed this Sep 27, 2026
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