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

Automatically set GOMAXPROCS to match Linux container CPU quota #1827

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alphastorm
Copy link
Contributor

See https://github.com/uber-go/automaxprocs for details about the library.

See prysmaticlabs/prysm#2770 for an example PR of this getting merged to the Prysmatic Labs ETH2 beacon chain and validator clients.

@Shadowfiend
Copy link
Contributor

Compiling in an external package that has unlimited access to our memory just to set an environment variable seems odd… I'm interested in finding a better way to get this functionality that more clearly separates the environment concern from the client. Maybe we can pull this lib into a separate executable that can spit out the relevant value rather than interfere in-memory, and package that into the container as something that can be run during startup.

@alphastorm
Copy link
Contributor Author

Compiling in an external package that has unlimited access to our memory just to set an environment variable seems odd… I'm interested in finding a better way to get this functionality that more clearly separates the environment concern from the client.

Importing the package triggers init() which shells out to maxprocs.Set(), which calculates the optimal GOMAXPROCS considering cgroups / calls runtime.GOMAXPROCS() / returns. I wouldn't call this unlimited access since the library's lifecycle is fairly limited.

Maybe we can pull this lib into a separate executable that can spit out the relevant value rather than interfere in-memory, and package that into the container as something that can be run during startup.

That seems needlessly complex, as it would require either using a wrapper script or process manager like supervisord (see https://docs.docker.com/config/containers/multi-service_container/). This library has been OSS since August 2017 and is pretty widely used. Longer term the hope is to add mainline support (see golang/go#33803).

Whatever you choose to do, the current default behavior can cause (and is likely causing) performance degradation in affected configurations (from uber-go/automaxprocs#12):

If there's misalignment between CFS quota, the language runtime tuning (in this case GOMAXPROCS) and the workload is mostly CPU bound (e.g. spawning a lot of active goroutines, calculations, etc.) then this could cause performance degradation.

There are numbers from synthetic and real-world benchmarks in the linked issue. The guy quoted above ran a synthetic benchmark calculating prime numbers 😄

Useful thread with a lot more references: https://threadreaderapp.com/thread/1149654812595646471.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants