-
Notifications
You must be signed in to change notification settings - Fork 62
Reduce test flakes #8989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce test flakes #8989
Conversation
Rust 1.89 wasn't available on that nixpkgs revision, but it was specified in the `rust-toolchain.toml` file.
| RegionSnapshotReplacementState::Requested | ||
| ) { | ||
| eprintln!( | ||
| "loop {i} Failed {:?} != Requested", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What state was this in when you saw this? This seems like a failure to properly unwind the saga as it should never have reached this check without going through rsrss_set_saga_id_undo. Likewise, the assert_eq above checking operating_saga_id should have fired if the request was in an intermediate state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does eventually get to the requested state.
When I saw the panic, it would be in Allocating state. At least that's what I had written down in an earlier comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in DMs, this was concerning enough to address in a separate PR.
| pub async fn remove_disk_from_snapshot_rop(&self) { | ||
| let disk_url = get_disk_url("disk-from-snapshot"); | ||
|
|
||
| eprintln!("NOW Remove disk from snapshot for disk {:?}", disk_url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These messages are not necessarily needed due to request logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
Various tuning to reduce test flakes during CI.
Decrease the number of tests running at the same time on buildomat runs.
The buildomat AWS instance has 8 cores, so we configure the nextest run to have
two less than that. This reduces overall system load.
Increase some internal timeouts, and add loops to some tests to allow them to finish
even if the system is under heavy load.
Added a few debug messages, but, they may be of limited value.