Stabilize cancel/terminate isolation tests with sync steps - #10456
Open
gayyappan wants to merge 1 commit into
Open
Stabilize cancel/terminate isolation tests with sync steps#10456gayyappan wants to merge 1 commit into
gayyappan wants to merge 1 commit into
Conversation
cagg_cancel_kill_refresh flaked on PG19beta2: the terminated backend's "step tr3_refresh: <... completed>" was reported after wp2_release instead of before it. Same lines, different position. isolationtester's (blocked_step) marker only delays the *reporting* of a step's completion, never the *launch* of the next step. A step launches as soon as the preceding one is done or deemed blocked, and a marker-delayed step counts as deemed blocked. So the waitpoint-release step was launched and reported while the signalled backend was still unwinding, and which of the two got reported first came down to timing. Fix it the way src/test/isolation/README prescribes: add an empty step in the signalling session right after the signal step. Rule (A) -- all prior steps of a session must be done before the next one launches -- then transitively holds the release step until the cancelled or terminated refresh has been fully reported. This also closes a visibility race in the cancel permutations, where s1_registered_ranges could read the catalog before R1's PG_CATCH cleanup committed. Applied to the two sibling specs carrying the same latent race. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
gayyappan
marked this pull request as ready for review
August 13, 2026 21:39
|
@kpan2034, @akuzm: please review this pull request.
|
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.
cagg_cancel_kill_refresh flaked on PG19beta2: the terminated backend's "step tr3_refresh: <... completed>" was reported after wp2_release instead of before it. Same lines, different position.
isolationtester's (blocked_step) marker only delays the reporting of a step's completion, never the launch of the next step. A step launches as soon as the preceding one is done or deemed blocked, and a marker-delayed step counts as deemed blocked. So the waitpoint-release step was launched and reported while the signalled backend was still unwinding, and which of the two got reported first came down to timing.
Fix it the way src/test/isolation/README prescribes: add an empty step in the signalling session right after the signal step. Rule (A) -- all prior steps of a session must be done before the next one launches -- then transitively holds the release step until the cancelled or terminated refresh has been fully reported. This also closes a visibility race in the cancel permutations, where s1_registered_ranges could read the catalog before R1's PG_CATCH cleanup committed.
(https://github.com/postgres/postgres/blob/ac5cea86e8a474e13d7dcd22a4a7fabf722c5514/src/test/isolation/README#L207)
Applied to the two sibling specs carrying the same latent race.