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.

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

ci(check-urls): use a matrix to speed-up execution #7060

Conversation

davorpa
Copy link
Member

@davorpa davorpa commented Sep 7, 2022

What does this PR do?

Improve repo

For resources

Description

The matrix strategy creates 10 max-parallel workers with a disabled fail-fast parameter

Examples

Running the workflow when -langs.md or -subject.md files are involved could exhaust the runner execution max-time like sometime happens in the past (https://github.com/EbookFoundation/free-programming-books/actions/runs/2924594027 max is 360min).

image

Using this implementation... the elapsed time is of the longest execution time, normally no more than 30min on the worst of cases.

Moreover, we can meter how much time awesome bot consumes in each check.

Checklist:

Follow-up

  • Check the status of GitHub Actions and resolve any reported warnings!

The matrix strategy creates 10 max-parallel workers with a disabled fail-fast parameter

Examples
- Before: https://github.com/davorpa/free-programming-books/actions/runs/2999590952
- After: https://github.com/davorpa/free-programming-books/actions/runs/3007199364

Running the workflow when -langs or -subject files are involved could exhaust the runner execution max-time.

Using this implementation... the elapsed time is of the longest execution time, normally no more than 30min on the worst of cases
@davorpa davorpa added urlchecker Checker workflow monitoring the links of resources has found some warnings. See logs and fix URLs. 🤖 automation Automated tasks done by workflows or bots labels Sep 7, 2022
@davorpa davorpa self-assigned this Sep 7, 2022
@davorpa davorpa marked this pull request as ready for review September 7, 2022 14:21
@eshellman
Copy link
Collaborator

there must be some limit on how many threads the repo can use as a whole. Worrying about load in October

@davorpa davorpa added the 👀 Needs Review Is this really a good resource? Reviews requested. label Sep 7, 2022
@davorpa
Copy link
Member Author

davorpa commented Sep 8, 2022

there must be some limit on how many threads the repo can use as a whole. Worrying about load in October

About max-parallel matrix strategy param:

The maximum number of jobs that can run simultaneously when using a matrix job strategy. By default, GitHub will maximize the number of jobs run in parallel depending on the available runners on GitHub-hosted virtual machines

Is it 10 well or better leave GitHub to decide?

Moreover... setting max-parallel: 1 it will the same what we have now 😉 but with a bit penalization of 5s (I'm not sure at all) wasting on warm up the job container.

@davorpa

This comment was marked as outdated.

@davorpa davorpa added the waiting for changes PR has been reviewed and changes/suggestions requested label Sep 8, 2022
@davorpa davorpa marked this pull request as draft September 8, 2022 08:45
@davorpa davorpa marked this pull request as ready for review September 11, 2022 12:03
@davorpa

This comment was marked as outdated.

@davorpa davorpa removed the waiting for changes PR has been reviewed and changes/suggestions requested label Sep 11, 2022
@eshellman
Copy link
Collaborator

Reviewers welcome!

@davorpa davorpa added the conflicts Conflict(s) need to be resolved label Sep 21, 2022
@davorpa

This comment was marked as outdated.

@davorpa davorpa removed the conflicts Conflict(s) need to be resolved label Sep 21, 2022
@davorpa davorpa force-pushed the gh-actions/check-urls/using-parallel-matrix branch from 7c69152 to 05273d9 Compare September 24, 2022 13:50
@davorpa davorpa added the conflicts Conflict(s) need to be resolved label Sep 24, 2022
…l-matrix

Update tj-actions/changed-files@v29.0.7 -> v30.0.0
@davorpa davorpa removed the conflicts Conflict(s) need to be resolved label Sep 24, 2022
@davorpa davorpa added the help wanted Needs help solving a blocked / stucked item label Feb 22, 2023
@github-actions github-actions bot added the conflicts Conflict(s) need to be resolved label Feb 23, 2023
@github-actions
Copy link

Oh no 😟! Conflicts have been found.

Please 🙏, take a moment and address the merge conflicts of your pull request before we can evaluate it again.

Thanks in advance for your effort and patience ❤️!

@github-actions github-actions bot removed the conflicts Conflict(s) need to be resolved label Feb 23, 2023
@davorpa davorpa requested a review from Thenlie February 23, 2023 16:36
::set-output --> GITHUB_OUTPUT env file
@Thenlie
Copy link
Contributor

Thenlie commented Feb 23, 2023

This seems like it could be a really nice improvement. I like that you are not failing fast so a user can see all issues right away. In looking at the workflow results I am a bit confused though.

Original Workflow

image

Matrix workflow

image

As you can see, the new workflow shows a total time that is nearly 3 times as long as the original. With that said, the individual times add up to 30s so maybe this is misleading on GitHubs part. Additionally, this is only running a single test, so not really seeing the full benefits of the matrix.

As far as I can tell, this is, at the very least, working as expected. Would it be worth making some test PR's that trigger as many workflows as possible to get a better gauge what improvement to execution time this makes? Admittedly, I have not used this matrix feature before, so let me know if there is anything else I should look into/test on my end.

@eshellman
Copy link
Collaborator

is this ready?

Copy link
Contributor

@Thenlie Thenlie left a comment

Choose a reason for hiding this comment

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

Since the individual execution is the same speed, I am giving this my approval. There might be slightly more setup/tear down time but this seems to be worth the improved performance when multiple jobs are run.

@davorpa
Copy link
Member Author

davorpa commented Feb 24, 2023

is this ready?

Yes, developing is ready 😉

@eshellman eshellman merged commit 62d466d into EbookFoundation:main Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 automation Automated tasks done by workflows or bots 👀 Needs Review Is this really a good resource? Reviews requested. help wanted Needs help solving a blocked / stucked item urlchecker Checker workflow monitoring the links of resources has found some warnings. See logs and fix URLs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants