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

Use uber-go/automaxprocs to default GOMAXPROCS aware of Linux CPU Quotas #10950

Closed
BenTheElder opened this issue May 17, 2023 · 7 comments · Fixed by #10951 or #10952
Closed

Use uber-go/automaxprocs to default GOMAXPROCS aware of Linux CPU Quotas #10950

BenTheElder opened this issue May 17, 2023 · 7 comments · Fixed by #10951 or #10952

Comments

@BenTheElder
Copy link
Contributor

See:

Basically: When using hugo in a container with limited CPU quota, hugo can encounter CPU throttling issues because GOMAXPROCS is defaulted to the underlying hardware core count instead of the allocated CPU limits in the container.

There's something of a defacto standard library for defaulting this in a CPU limit aware way in https://github.com/uber-go/automaxprocs, at least until golang/go#33803 is solved.

Users would still be able to override by setting GOMAXPROCS env, and this is a fairly small MIT licensed dependency.

@bep
Copy link
Member

bep commented May 17, 2023

A note from the sideline, Hugo bases its parallelism on

  1. HUGO_NUMWORKERMULTIPLIER env var if set
  2. runtime.NumCPU()

So, setting HUGO_NUMWORKERMULTIPLIER=1 (or something) could be a workaround.

@BenTheElder
Copy link
Contributor Author

Ack, thanks!

We're getting good results setting GOMAXPROCS=1 in netlify for Kubernetes (it appears free netlify has ~1 core allocated?)

runtime.NumCPU()

I think it might? be better to base it on runtime.GOMAXPROCS(0) (<1 does not set it and only returns the current value).

Even without that though, AIUI the setting GOMAXPROCS will let go internally do better at scheduling within the CPU quota.

I filed a sample PR in #10951 for the latter, we've seen automaxprocs fix performance issues with containerized Go more generally, e.g. in Kubernetes's CI unit tests.

@bep bep added Enhancement and removed Proposal labels May 17, 2023
bep pushed a commit that referenced this issue May 17, 2023
@bep
Copy link
Member

bep commented May 17, 2023

I needed to revert this; a new patch that makes the tests pass is welcomed, but I have not time to investigate this at the moment.

@bep bep reopened this May 17, 2023
@BenTheElder
Copy link
Contributor Author

I'll look into that 👀

Thank you for all your work on hugo!

@bep
Copy link
Member

bep commented May 17, 2023

Great; Ideally I would want no output from that library.

@BenTheElder
Copy link
Contributor Author

BenTheElder commented May 17, 2023

OK, sorry about that, the root automaxprocs just calls maxprocs.Set(macprocs.Logger(log.Printf)), if we call maxprocs.Set() instead there will be no logging this time (per the godocs, checking the implementation, and local testing).

I can repro the failures locally on the last PR from undesired added logging.
go test ./... passes with the updated patch, I should've thought to check before filing the last PR.

Filed #10952

bep pushed a commit that referenced this issue May 18, 2023
@github-actions
Copy link

github-actions bot commented Jun 9, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants