deflake some Crucible replacement tests #9217
Merged
Conversation
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
This should fix flake opportunities in both: * `crucible_replacements::test_delete_volume_region_replacement_state_replacement_done` * `crucible_replacements::test_Delete_volume_region_replacement_state_requested` This is my proposed change in #9216. From the logs of a flake and careful reading it looks like we were almost-but-not-quite handling the condition described in `wait_for_request_state` if the start and end states for a replacement request are the same. Consulting `get_region_replacement_by_id` and the returned replacement request state tells us when the step has completed, in addition to checking that the step had started.
iximeow
commented
Oct 15, 2025
Comment on lines
+589
to
+592
| // The replacement step is in progress, but | ||
| // not done yet. We're still waiting, but | ||
| // probably not for long. | ||
| Err(CondCheckError::<()>::NotYet) |
Member
Author
There was a problem hiding this comment.
having said a whole lot of words, the real critical point would to be able to see this somehow. I tried to coax evidence of that:
- added a
panic!("race hit!")here locally - ran the test as
while true; do pbind -e 1 -- cargo nextest run test_delete_volume_region_replacement_state_replacement_done; done - in another shell, ran
pbind -e 1 bash -c 'while true; do :; done
while this extremely slowed things down (.. more than the 50% I'd expected), I have yet to see the panic get hit.
the possibility that simply calling get_region_replacement_request_by_id adds enough time to consistently resolve the race is not lost on me...
jmpesp
approved these changes
Oct 15, 2025
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.
This should fix flake opportunities in both:
crucible_replacements::test_delete_volume_region_replacement_state_replacement_donecrucible_replacements::test_Delete_volume_region_replacement_state_requestedThis is my proposed change in #9216. From the logs of a flake and careful reading it looks like we were almost-but-not-quite handling the condition described in
wait_for_request_stateif the start and end states for a replacement request are the same.Consulting
get_region_replacement_by_idand the returned replacement request state tells us when the step has completed, in addition to checking that the step had started.