Skip to content
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

roachtest: use spot VM for all tests #127896

Merged

Conversation

nameisbhaskar
Copy link
Contributor

@nameisbhaskar nameisbhaskar commented Jul 30, 2024

Currently, we are using spot VMs only to run benchmark tests. The reason for the low adoption of spot VM is because of the test failures due to VM preemption.
But, with the recent changes, we have better control on VM preemption changes where if there is a test failure due to preemption, the test is run on an on-demand VM the next time.
Also, the current failed tests that are run on spot VM is 148 out of 1093 = 13.5%. If I consider failure out of total tests run is 148 out of 2508 = 5.9%.

So, this PR removes the condition to use spot VMs only for benchmark tests and changes the probability to 75%.

Fixes: #127236
Epic: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/spot-for-all-tests branch from 2d4aeac to 7ffe9ff Compare July 30, 2024 16:54
@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/spot-for-all-tests branch 5 times, most recently from 76f7f8c to 08bb256 Compare August 19, 2024 05:52
@nameisbhaskar nameisbhaskar marked this pull request as ready for review August 20, 2024 04:54
@nameisbhaskar nameisbhaskar requested a review from a team as a code owner August 20, 2024 04:54
@nameisbhaskar nameisbhaskar requested review from renatolabs and vidit-bhat and removed request for a team August 20, 2024 04:54
@nameisbhaskar
Copy link
Contributor Author

Ran 2 teamcity builds. The failure is ~13%
Test 1 - 28 tests out of 215 tests ran on spot VMs
Test 2 - 27 tests out of 208 tests ran on spot VMs

Currently, we are using spot VMs only to run benchmark tests. The reason
for the low adoption of spot VM is because of the test failures due to
VM preemption.
But, with the recent changes, we have better control on VM preemption
changes where if there is a test failure due to preemption, the test is
run on an on-demand VM the next time.
Also, the current failed tests that are run on spot VM is 148 out of 1093 = 13.5%.
If I consider failure out of total tests run is 148 out of 2508 = 5.9%.

So, this PR removes the condition to use spot VMs only for benchmark tests and changes
the probability to 75%.

Fixes: cockroachdb#127236
Epic: None
@nameisbhaskar nameisbhaskar force-pushed the user/bhaskar/spot-for-all-tests branch from 08bb256 to 69da7de Compare August 20, 2024 04:59
Copy link
Contributor

@shailendra-patel shailendra-patel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nameisbhaskar
Copy link
Contributor Author

Thanks @shailendra-patel and @vidit-bhat for approving the change.

@nameisbhaskar
Copy link
Contributor Author

bors r=@shailendra-patel,@vidit-bhat

@nameisbhaskar nameisbhaskar added backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 labels Aug 20, 2024
@craig craig bot merged commit b42ad64 into cockroachdb:master Aug 20, 2024
23 checks passed
Copy link

blathers-crl bot commented Aug 20, 2024

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.


Issue #127236: branch-release-24.1, branch-release-24.2.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link

blathers-crl bot commented Aug 20, 2024

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 69da7de to blathers/backport-release-24.1-127896: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.1.x failed. See errors above.


error creating merge commit from 69da7de to blathers/backport-release-24.2-127896: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@nameisbhaskar
Copy link
Contributor Author

blathers backport 24.1 24.2

Copy link

blathers-crl bot commented Aug 20, 2024

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 69da7de to blathers/backport-release-24.1-127896: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.1 failed. See errors above.


error creating merge commit from 69da7de to blathers/backport-release-24.2-127896: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.2 failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@nameisbhaskar nameisbhaskar deleted the user/bhaskar/spot-for-all-tests branch August 23, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable spot VMs for tests other than benchmarks
4 participants