Repository navigation
cloud: the quota halt is flagged on the worker, not the main thread - #20
Merged
Merged
Conversation
The badge now goes to https://olewave.com/en/leaderboard/fa-bench in place of the records page in this repo. The records links in the README are unchanged, and the Unreleased section of the CHANGELOG says so.
align_corpus set halt when the main thread got round to the failed future, while the worker that raised it had already started its next item. On a loaded CI runner that was four calls past the wall, and the ci run on the #17 merge failed on the test for it, four calls where two were allowed. The worker now flags the halt as it raises, so its next item sees it before any call leaves, and the test asserts exactly one call. Reproduced locally one time in fifteen with the test pinned to a CPU shared with two busy loops, and fifteen of fifteen pass after.
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
1 participant
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.
The
cirun on the #17 merge (171948d) failed in thetestjob ontest_an_exhausted_quota_stops_the_cell_instead_of_bleeding, four calls past the quota wall where two were allowed. The PR run minutes earlier had passed, and the #18 merge run passed again.It is a race in
align_corpus. The worker thread raised the quota error, but only the main thread sethalt, after it woke fromas_completed, and by then the worker had started its next items. On an idle machine that is one or two items, on a loaded runner it was four. Reproduced here one time in fifteen with the test pinned to a CPU shared with two busy loops.The worker now flags the halt as it raises, so its next item sees it before any call leaves, and the test asserts exactly one call. Fifteen of fifteen pass under the same contention, the cloud test file passes, lint passes. One line under Unreleased, Fixed, in the CHANGELOG. No published number changes.
Stacked on #19, so until #19 merges this PR also shows its README link commit.