Skip to content

[CI] Cancel previous run when new commit pushed #11941

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
Merged
merged 11 commits into from
Dec 1, 2023
Merged

[CI] Cancel previous run when new commit pushed #11941

merged 11 commits into from
Dec 1, 2023

Conversation

jsji
Copy link
Contributor

@jsji jsji commented Nov 18, 2023 •
edited
Loading

In quite some PRs, we may need to push commits when the run is still in progress.
Currently we will run all the workflow to the end by default, which is somehow a waste of machine resource.

This PR use https://docs.github.com/en/actions/using-jobs/using-concurrency to group tests for Linux and Windows pre-commit tests, and cancel-in-progress if new commits are pushed.

@jsji jsji self-assigned this Nov 18, 2023
@jsji jsji closed this Nov 20, 2023
@jsji jsji reopened this Nov 20, 2023
@jsji jsji marked this pull request as ready for review November 20, 2023 13:42
@jsji jsji requested a review from a team as a code owner November 20, 2023 13:42
@aelovikov-intel
Copy link
Contributor

Last time I tried it, it didn't work and the whole CI just died. We also have an ongoing issue with Win CI, so I'm not going to merge it right now.

This can only work in default branch, so the testing/validation can only be done when it is merged

Is it about CUDA only or this PR in general?

@jsji
Copy link
Contributor Author

jsji commented Nov 20, 2023 •
edited
Loading

Last time I tried it, it didn't work and the whole CI just died. We also have an ongoing issue with Win CI, so I'm not going to merge it right now.

This can only work in default branch, so the testing/validation can only be done when it is merged

Is it about CUDA only or this PR in general?

CUDA only. I can remove the CUDA part first for now. Windows/Linux pre-commit using concurrency tested in this PR.

@jsji
Copy link
Contributor Author

jsji commented Nov 30, 2023

Ping.. I think this is general good for CI, may help with the windows CI too.

aelovikov-intel
aelovikov-intel approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

The change is good, but someone has to commit to keep closely monitoring CI after this is merged, possibly for a couple of days.

@jsji
Copy link
Contributor Author

jsji commented Dec 1, 2023

The change is good, but someone has to commit to keep closely monitoring CI after this is merged, possibly for a couple of days.

@intel/llvm-gatekeepers I think this is ready for merge. I will keep an eye in CI for days after it is merged.

@aelovikov-intel aelovikov-intel merged commit f715119 into intel:sycl Dec 1, 2023
@aelovikov-intel
Copy link
Contributor

@intel/llvm-gatekeepers I think this is ready for merge. I will keep an eye in CI for days after it is merged.

Merged.

@jsji
Copy link
Contributor Author

jsji commented Dec 7, 2023

In case others are interested:

12-1 to 12-6, CI cancelled 20 of 154 run for windows, 15 of 154 runs for Linux.
Assuming the cancelled run would take the same amount of time of the success one if we don’t cancel it,
we saved around 748 minutes in windows run (10% of total runtime), 547 minutes in linux run (8% of total runtime).

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jan 5, 2024
@aelovikov-intel
[CI] Remove old lint task
8451655 
We've enabled the upstream community workflow in
intel#12184 after discussion in
intel#12085. It's been doing just
fine, so the old can be removed now.

We now start the build/tests before format checks are finished, but that
shouldn't be a problem thanks to intel#11941.
@aelovikov-intel aelovikov-intel mentioned this pull request Jan 5, 2024
Merged
aelovikov-intel added a commit that referenced this pull request Jan 5, 2024
@aelovikov-intel
[CI] Remove old lint task (#12308)
cff54c8 
We've enabled the upstream community workflow in
#12184 after discussion in
#12085. It's been doing just
fine, so the old can be removed now.

We now start the build/tests before format checks are finished, but that
shouldn't be a problem thanks to
#11941.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aelovikov-intel aelovikov-intel aelovikov-intel approved these changes

Assignees

@jsji jsji

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsji @aelovikov-intel