-
Notifications
You must be signed in to change notification settings - Fork 770
[CI] Enable v2 adapter testing on BMG on linux #18357
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
Conversation
9f63093
to
6f011d1
Compare
6f011d1
to
fc25582
Compare
fc25582
to
ec8a3f4
Compare
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.
@sarnex , @uditagarwal97 , do we have enough runners capacity in CI for that?
Here's the time taken by E2E jobs before and after this PR: Overhead on PVC seems significant to me. IIRC, we also had one PVC runner also used for build. @sarnex have you seen PVC being a bottleneck recently? I think we should be good with BMG, although we have only one Linux BMG runner, I haven't experienced BMG being a bottleneck recently. |
PVC GPU hasn't been a blocker recently, but I agree the more than doubling of the job time is worrying. |
That seems to be due to the timeout, so not representative. That said, timeout in itself is a problem. |
Ah sorry, it would be nice to see some results we think are representative of normal runs. |
Yes, I'm investigating the timeouts right now - it seems to be related to copy offload (so likely a driver issue). For now, perhaps we can enable the testing just for BMG? |
👍 |
@uditagarwal97 @sarnex @aelovikov-intel I changed this PR to only enable testing on BMG. Here's a separate PR for PVC: #18909 Interestingely, on PVC 'Run E2E tests' takes almost exactly the same amount of time with v2 and without it (there's only a few seconds difference which seems pretty random) once I disable the time-outing tests. |
is gonna be disabled right? change lgtm otherwise |
Actually, I only enabled the v2 testing on linux so this failure seems unrelated - I think I saw it failing on other PRs as well. I updated the PR title to make it clear it's only on linux for now. |
@aelovikov-intel any other comments/concerns? If no, I think this should be ready for merge |
No description provided.