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

github workflow: use GOMEMLIMIT to limit mem usage #610

Closed
wants to merge 1 commit into from
Closed

github workflow: use GOMEMLIMIT to limit mem usage #610

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions .github/workflows/tests-template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,7 @@ jobs:
CPU=4 make test
;;
linux-unit-test-4-cpu-race)
# XXX: By default, the Github Action runner will terminate the process
# if it has high resource usage. Try to use GOGC to limit memory and
# cpu usage here to prevent unexpected terminating. It can be replaced
# with GOMEMLIMIT=2048MiB if the go-version is updated to >=1.19.x.
#
# REF: https://github.com/actions/runner-images/issues/6680#issuecomment-1335778010
GOGC=30 CPU=4 ENABLE_RACE=true make test
GOMEMLIMIT=2048MiB CPU=4 ENABLE_RACE=true make test
Copy link
Member

Choose a reason for hiding this comment

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

Based on the Suggested uses in https://tip.golang.org/doc/gc-guide,

  • Don't use the memory limit when deploying to an execution environment you don't control.
  • Don't set a memory limit to avoid out-of-memory conditions when a program is already close to its environment's memory limits.

Also the default value for GOGC is 100, which should be good enough.

What we can do is to try to use larger runner.

So can we just remove the configuration? I mean we don't use GOGC, nor GOMEMLIMIT?

Copy link
Member

Choose a reason for hiding this comment

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

The original issue seems to imply jobs are being killed due to excessive resource usage: actions/runner-images#6680 and based on discussion / linked issues it appears to be ongoing.

However I don't actually know if we are still impacted by this, based on the comments we were historically.

Agree let's try large runner for longer jobs and see if we can just remove it altogether. We can always reinstate the limit if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

These changes would change the approach for this PR. I pushed them to #612.

Copy link
Member

Choose a reason for hiding this comment

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

The GOGC was used to prevent from unexpected kill. I think the issue is still here if we don't use larger size runner.
Yeah, I prefer to abort this one and use #612.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would using the large runner just for the race test case make sense? And leave the others with the regular runner? I can change the implementation if that sounds good.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM - 4cpu race (amd64 +arm64) and windows tests are the ones that need extra resources.

;;
*)
echo "Failed to find target"
Expand Down
Loading