Repository navigation
The cloud probe budget test stops counting machine stalls against the probe (#5600) - #5601
Closed
erikdarlingdata wants to merge 1 commit into
Closed
erikdarlingdata wants to merge 1 commit into
erikdarlingdata wants to merge 1 commit into
Conversation
…wn resume hop against the probe (#5600) The post-cancel bar now reads the probe task's own completion through a synchronous continuation, and a dedicated stall watcher thread grants machine stalls as allowance. An unbounded wait after the cancel still fails.
5 of 6 tasks
Owner
Author
|
Superseded by #5604. That PR keeps this one's change to how the test stamps the probe's return time, and runs the test in a timing collection that runs on its own. It drops the stall allowance. The branch stays on the remote. Claude posting for Erik Darling. |
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.
Fixes #5600.
SECTION: None
Why
The cloud probe budget test failed in the whole-suite guards job: "ProbeAsync returned 1083 ms after the handler's request ended, not within 1000 ms", on a pull request that touches no probe code. The probe cancels its request on its own 200 ms budget and returns inline, so a gap of over a second is time the machine took, not time the probe spent.
What changes
Test only, in
Darling/Darling.Tests/DarlingCloudIdentityProbeTests.cs. The probe is unchanged.await ... WaitAsync(...), so it also counted the hop from the probe task's completion back to the test method (WaitAsync's own completion, then the test's continuation). That hop can wait on a busy process.StallWatcherruns on a dedicated thread (never a pool thread) and records every 10 ms tick that came 25 ms or more late. A stop-the-world pause or a descheduled process delays all threads at once. The two timed steps (call to first request, and request end to return) get the stall time that fell inside their window as extra allowance. A probe that really waits leaves the watcher ticking on time and gets no allowance.I could not reproduce the load on this laptop (no CPU load allowed), so the cause above is from reading the code and the earlier #4741 notes, not a measured repro. The failure message change exists so CI will tell us if a stall or the resume hop was it.
RED-plant
A temporary plant in the probe (an untokened 3 s
Task.Delayin the catch after the budget cancels), then the test:The plant is not committed.
Test plan
*DarlingCloudIdentityProbe*: Total 20, Failed 0 (three runs in a row also 20/0).*Census*: Total 485, Failed 0, Skipped 1.*Ratchet*: Total 15, Failed 0.-trait Stage=Guard -failSkips: Total 1432, Failed 0.