-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Reserve more CPU cores for integration tests with high rate of flakes #27257
Reserve more CPU cores for integration tests with high rate of flakes #27257
Conversation
Signed-off-by: Yan Avlasov <yavlasov@google.com>
@@ -40,6 +40,9 @@ envoy_extension_cc_test( | |||
size = "large", | |||
srcs = ["local_ratelimit_integration_test.cc"], | |||
extension_names = ["envoy.filters.http.local_ratelimit"], | |||
tags = [ | |||
"cpu:3", |
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.
i experimentally added something similar here https://github.com/envoyproxy/envoy/pull/27243/files#diff-3ca343c3f15cbe6a9354d5c44647e85b92d118a4a01123b470c7866cc5c088adR62
i wasnt 100% clear how this works but iiuc its indicative to bazel - bazel schedules less local jobs according to their cpu reservation
i think in the RBE case it doesnt help - we get mostly 2-core workers (ive seen sometimes 16-core also but infrequently) - but i think this flag doesnt come into to play in that case
also ~related #27243 (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.
It does help in RBE case as well. It will prevent scheduling two integration test processes from running on the same two core VM. Reduce CPU over subscription by half.
Signed-off-by: Yan Avlasov <yavlasov@google.com>
/assign @phlax |
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.
lgtm, thanks @yanavlasov
…envoyproxy#27257) Signed-off-by: Yan Avlasov <yavlasov@google.com>
…#27257) Signed-off-by: Yan Avlasov <yavlasov@google.com>
…envoyproxy#27257) Signed-off-by: Yan Avlasov <yavlasov@google.com> Signed-off-by: Ryan Eskin <ryan.eskin89@protonmail.com>
Additional Description:
This should reduce the rate of timing related flakes (which it does in local runs). However to take full advantage of CPU reservation all integration tests need to be configured with more CPU cores.
Risk Level: Low
Testing: Unit Tests
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A