-
Notifications
You must be signed in to change notification settings - Fork 327
Enforcing correct CI workers for benchmarks #2693
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
Enforcing correct CI workers for benchmarks #2693
Conversation
scripts/jenkins/run-benchmarks.sh
Outdated
| BASE_FREQ="1.9GHz" | ||
| elif [ "${CPU_MODEL}" == "12th Gen Intel(R) Core(TM) i5-12500 " ] | ||
| then | ||
| CORE_INDEX=11 |
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.
Based on the other types, it seems to count the effective threads rather than physical cores, but I am not sure
|
run benchmark tests |
|
Why are the benchmarks suddenly executed on a new hardware? This would make any comparisons with past results meaningless. |
Does this mean we cannot upgrade? Or is it enough to manually compare now and start accumulating new history? |
|
I suppose it doesn't mean that we can never upgrade but we should carefully review when doing so. For example, is the new worker a dedicated bare metal machine like the old one or is it a cloud VM that may have noisy neighbours? Which background processes are running on the new worker that might interfere with the benchmark? |
|
@cachedout do you know why the benchmarks aren't executed on the dedicated bare metal machines anymore? |
I am not a fan of using labels to trigger CI jobs, I preferred it to be intentional using a GitHub comment because these additional jobs usually do not need to run on every commit, and if you do it you will waisting CI time. cc @v1v |
|
Let me look at what's going on |
It's not supported, Lines 57 to 58 in 32ee207
You can enable it by using something like: in Lines 300 to 304 in 32ee207
It uses Unfortunately, the What baremetals can you use?
|
|
@eyalkoren , I just added some changes to this PR so:
Let's see how it goes, I didn't want to change the description of this PR, feel free to do it so 🙇 |
raises hand This is my fault. I requested workers for the prodfiler benchmarking project and had them labeled with a new label but missed the fact that they'd also be put in an existing pool. Apologies. |
Context: benchmarks script failing on new CI processor - https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-java%2Fapm-agent-java-mbp/detail/main/285/pipeline:
Based on Intel's processor's spec.
@kuisathaverat Do you think a
ci:benchmarkslabel to tell CI to run benchmarks for PRs would be useful?