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.

Sign up for GitHub

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

Jump to bottom

CI: split i686-msvc job #135483

Closed
wants to merge 0 commits into from
Closed

CI: split i686-msvc job #135483

wants to merge 0 commits into from

Conversation

marcoieni
Copy link
Member

@marcoieni marcoieni commented Jan 14, 2025 •
edited
Loading

Split the i686-msvc job in two to move it to free runners and reduce the use of large runners in CI. Related to rust-lang/infra-team#189

This PR is similar to #133632 however the job i686-msvc fails less often than x86_64-msvc in CI, so it can be split more safely.

In the last month, this job failed only 3 times:

image

Datadog link for the infra team.

r? @Kobzol

try-job: i686-msvc-1
try-job: i686-msvc-2

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 14, 2025
@marcoieni
Copy link
Member Author

@bors try

@marcoieni marcoieni mentioned this pull request Jan 14, 2025
Open
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
@bors
Auto merge of rust-lang#135483 - marcoieni:split-i686-msvc, r=<try>
a206d6d 
CI: split i686-msvc job

try-job: i686-msvc-1
try-job: i686-msvc-2
@bors
Copy link
Contributor

bors commented Jan 14, 2025

⌛ Trying commit 3bb38a0 with merge a206d6d...

@jieyouxu
Copy link
Member

(@marcoieni btw, your PR says i686-msvc but your screenshot says i686-mingw, I think)

@marcoieni
Copy link
Member Author

marcoieni commented Jan 14, 2025 •
edited
Loading

omg, sorry, I'm fixing this 😅
Thanks!

@marcoieni
Copy link
Member Author

marcoieni commented Jan 14, 2025 •
edited
Loading

Fixed. I was lucky. The number of failures during the last month is the same (3). 👍

@marcoieni
Copy link
Member Author

image
too slow. Let's see if we hit some cache by running the try job again.

@marcoieni
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Jan 14, 2025

⌛ Trying commit 3bb38a0 with merge 52e3e61...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
@bors
Auto merge of rust-lang#135483 - marcoieni:split-i686-msvc, r=<try>
52e3e61 
CI: split i686-msvc job

try-job: i686-msvc-1
try-job: i686-msvc-2
@bors
Copy link
Contributor

bors commented Jan 14, 2025

☀️ Try build successful - checks-actions
Build commit: 52e3e61 (52e3e61e4f39446fc514027288e0829dce60dff2)

@marcoieni
Copy link
Member Author

marcoieni commented Jan 15, 2025 •
edited
Loading

image

Timing looks ok.
If needed, in the future I can try splitting the first job into two.

@marcoieni marcoieni marked this pull request as ready for review January 15, 2025 07:26
@Kobzol
Copy link
Contributor

Kobzol commented Jan 15, 2025

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jan 15, 2025

📌 Commit 3bb38a0 has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 16, 2025
@bors
Auto merge of rust-lang#135483 - marcoieni:split-i686-msvc, r=Kobzol
6f332a1 
CI: split i686-msvc job

try-job: i686-msvc-1
try-job: i686-msvc-2
@bors
Copy link
Contributor

bors commented Jan 16, 2025

⌛ Testing commit 3bb38a0 with merge 6f332a1...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 16, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 16, 2025
@marcoieni
Copy link
Member Author

another job (different from this one) failed in the auto build, so it should still be fine to merge this

@Kobzol
Copy link
Contributor

Kobzol commented Jan 16, 2025

Well, this PR has actually changed what gets executed in the CI job that has failed 😅 But the failure is indeed spurious :)

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2025
@bors
Copy link
Contributor

bors commented Jan 16, 2025

⌛ Testing commit 3bb38a0 with merge f726e60...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 16, 2025
@bors
Auto merge of rust-lang#135483 - marcoieni:split-i686-msvc, r=Kobzol
f726e60 
CI: split i686-msvc job

try-job: i686-msvc-1
try-job: i686-msvc-2
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 16, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 16, 2025
@Kobzol
Copy link
Contributor

Kobzol commented Jan 16, 2025

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2025
@bors
Auto merge of rust-lang#135483 - marcoieni:split-i686-msvc, r=Kobzol
a0ba075 
CI: split i686-msvc job

try-job: i686-msvc-1
try-job: i686-msvc-2
@bors
Copy link
Contributor

bors commented Jan 17, 2025

⌛ Testing commit 3bb38a0 with merge a0ba075...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jan 17, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 17, 2025
@Kobzol
Copy link
Contributor

Kobzol commented Jan 17, 2025

Hmm, it looks like the MSVC change maybe increases the spurious failures? Maybe we should revert that change.

@ChrisDenton
Copy link
Member

Wait, this only changes i686-msvc right? So why is x86_64-msvc being affected?

@marcoieni marcoieni closed this Jan 17, 2025
@marcoieni
Copy link
Member Author

I force pushed my changes but github did this 😅
Probably I did something wrong, I don't know
image

@marcoieni
Copy link
Member Author

Luckily github is lying and the PR wasn't merged

@marcoieni
Copy link
Member Author

anyway, I'll open a new PR 👍

@marcoieni marcoieni mentioned this pull request Jan 17, 2025
Merged
@marcoieni
Copy link
Member Author

opened #135616 👍
Anyway, my guess for the failures is that splitting the job in that way caused the x86_64-msvc to be more flaky. Good to know before merging.
In the new PR I created to new make tasks so that the x86_64-msvc in unchanged

@marcoieni marcoieni mentioned this pull request Jan 19, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Kobzol Kobzol

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@marcoieni @bors @jieyouxu @Kobzol @rust-log-analyzer @ChrisDenton @rustbot