Repository navigation
land: inspect-lease-retry-test - #572
Merged
Merged
Conversation
…-retry test writer_lease_timeout_completion_waits_for_resource_not_input_change read an empty `in_flight` map as "the first attempt has fully retired". It has not: the serial background worker publishes its completion (removing the category from `in_flight` and notifying) inside route_tier2_reuse_completion, but its cold-build permit lives in the worker closure's `permit_slot` and is dropped only when the closure returns. While that permit is being torn down, a same-root ColdBuildLimiter::try_acquire_classified sees a holder whose weak permit no longer upgrades and returns None by design, so the resumed submission is deferred (deferred_categories: [Complexity]) instead of queued, and the assertion sees newly_queued_categories == []. Reproduced on Linux by delaying ColdBuildPermitLease::drop by 300 ms (emulating the worker being preempted mid-teardown): the unchanged test fails at the same line with the same left [] / right [Complexity] seen in the Windows merge-queue shard. Product behaviour is correct: a deferred automatic dispatch is retried by the Tier-2 scheduler's deferral backoff. The test now waits (30 s hang guard) until a probe permit can be taken from its limiter, which is only possible once every earlier permit has been returned, then makes the same assertion. The resumed-admission assertion now prints the whole submission on failure. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
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.
Accepted agent delivery; branch-keyed search descriptor included.
Summary by cubic
Fixes a race in
writer_lease_timeout_completion_waits_for_resource_not_input_changewhere the serial worker frees its cold-build limiter permit after publishing completion, so a resubmitted same-root category could be deferred instead of queued. The test now waits for the earlier permit to be returned before asserting, and prints the submission on failure. Adds a search descriptor for theinspect-lease-retry-testslice.Written for commit c0ffa0e. Summary will update on new commits.
The PR appears safe to merge.Summary
Makes the writer-lease retry test wait for the earlier worker’s cold-build permit before submitting its retry.
Reviews (1) · Last reviewed commit: "release notes and search-quality descrip..." · Reviewed by Greptile