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

github wokflow: use large runner, remove limits #612

Merged
merged 1 commit into from
Nov 20, 2023
Merged

github wokflow: use large runner, remove limits #612

merged 1 commit into from
Nov 20, 2023

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Nov 15, 2023 •
edited
Loading

Remove the resource limits from GOGC, and set the GitHub workflow to run on a large Ubuntu runner.

Fixes #609

@ivanvc ivanvc mentioned this pull request Nov 15, 2023
Closed
@ahrtr
Copy link
Member

ahrtr commented Nov 16, 2023

Please also update

bbolt/.github/workflows/tests_amd64.yaml

Line 10 in 33db274

runs-on: ubuntu-latest

Unfortunately we don't have larger runner for Windows platform, on which the workflows run the longest time.

@ivanvc
Copy link
Member Author

ivanvc commented Nov 16, 2023

Please also update

bbolt/.github/workflows/tests_amd64.yaml

Line 10 in 33db274

runs-on: ubuntu-latest

Unfortunately we don't have larger runner for Windows platform, on which the workflows run the longest time.

Hey @ahrtr, I'm working on these changes. I pushed quickly yesterday just to check if the workflow would run with the large runner. Now I'm tidying up this PR :)

@ivanvc ivanvc force-pushed the use-large-runner-for-test-workflow branch from 3893d26 to 254b089 Compare November 16, 2023 19:43
@ivanvc ivanvc marked this pull request as ready for review November 16, 2023 20:05
@ivanvc
Copy link
Member Author

ivanvc commented Nov 16, 2023 •
edited
Loading

/unhold

@ivanvc
Copy link
Member Author

ivanvc commented Nov 16, 2023

I guess I don't know how to remove that do-not-merge/work-in-progress label 😅

@fuweid, @jmhbnz, @ahrtr, can you take a look at this? Thanks.

@ivanvc ivanvc force-pushed the use-large-runner-for-test-workflow branch from 254b089 to 88aefc6 Compare November 16, 2023 23:40
jmhbnz
jmhbnz approved these changes Nov 16, 2023
View reviewed changes
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM on green - Thanks @ivanvc

fuweid
fuweid approved these changes Nov 17, 2023
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr
Copy link
Member

ahrtr commented Nov 17, 2023

Suggest to run coverage test on larger runner as well, Please see #612 (comment)

@ivanvc
github wokflow: use large runner for race tests
a7cb1ee 
Remove the resource limits from `GOGC`, and set the GitHub workflow to
run on a large runners for race tests (ubuntu 8 cores for amd64 and
coverage tests; actuated 8 cores for arm64).

Signed-off-by: Ivan Valdes <[email protected]>
@ivanvc
Copy link
Member Author

ivanvc commented Nov 17, 2023

Suggest to run coverage test on larger runner as well, Please see #612 (comment)

@ahrtr, you mentioned it, but I forgot to change it. I updated it, and coverage tests use the large runner, too.

@ivanvc ivanvc force-pushed the use-large-runner-for-test-workflow branch from 88aefc6 to a7cb1ee Compare November 17, 2023 20:03
ahrtr
ahrtr approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks.

@ahrtr ahrtr merged commit 947a3f3 into etcd-io:master Nov 20, 2023
@ivanvc ivanvc deleted the use-large-runner-for-test-workflow branch November 20, 2023 18:30
@fuweid fuweid mentioned this pull request Aug 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fuweid fuweid fuweid approved these changes

@ahrtr ahrtr ahrtr approved these changes

@jmhbnz jmhbnz jmhbnz approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use GOMEMLIMIT instead of GOGC to limit GitHub test workflow memory
5 participants
@ivanvc @ahrtr @fuweid @jmhbnz @k8s-ci-robot