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

automaxprocs? #111

Closed
uhthomas opened this issue Mar 5, 2021 · 6 comments
Closed

automaxprocs? #111

uhthomas opened this issue Mar 5, 2021 · 6 comments

Comments

@uhthomas
Copy link

uhthomas commented Mar 5, 2021

Would it be possible to include https://github.com/uber-go/automaxprocs with all BuildBarn services? Running BuildBarn in Kubernetes (which uses cgroups) does not correctly use all available resources.

@uhthomas
Copy link
Author

uhthomas commented Mar 5, 2021

golang/go#33803 is the relevant Go issue.

@EdSchouten
Copy link
Member

Hey!

We could add this, but are you aware of Kubernetes’ CPU manager policies?

https://kubernetes.io/docs/tasks/administer-cluster/cpu-management-policies/

Setting that to use the static policy will cause Kubernetes to place your containers in cpusets, making Go do the right thing out of the box.

Doing this would be preferable anyway for bb_runner containers, so that build actions also use proper CPU counts.

@uhthomas
Copy link
Author

uhthomas commented Mar 5, 2021

Thanks for the quick follow-up. From what I understand, this requires configuring the kubelets, which I cannot do.

I'm more than happy to open some PRs if you're not opposed to adding the package.

@EdSchouten
Copy link
Member

At this point I'm not really convinced. You can already change this by setting GOMAXPROCS, which I think is sufficient for environments where using CPU sets is completely out of the question.

I think someone should go ahead and file a ticket against Kubernetes, asking them to simply make the static CPU manager policy the default. I can't think of a logical reason why anyone would want to run a cluster without it.

@uhthomas
Copy link
Author

I guess GOMAXPROCS might work okay, but it's not very intuitive, and requiring knowledge of language runtime features just to get the application to scale CPUs correctly isn't nice.

There are lots of scenarios where it's not possible to touch the kubelet config. i.e a managed cluster.

It would be nice, as we see all BuildBarn components getting CPU throttled even with sufficient resources.

Is there any specific reason you would be opposed to this change?

@EdSchouten
Copy link
Member

Is there any specific reason you would be opposed to this change?

Yes. I think that this is an issue that is outside the scope of the Buildbarn project. This is something for Kubernetes/Docker/... and Go to sort out together. Users of the Go programming language shouldn't need to deal with this kind of stuff.

Sure, the problem can be solved by adding a single import to our code, but that only solves it on our end. You should also file a ticket at Prometheus to request the same, because it's also written in Go. The same for Traefik, CoreDNS, etc. etc. etc.

Basically requiring that everybody slaps import statements into their code to make essential stuff like this work is a perfect example of cargo cult programming.

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

No branches or pull requests

2 participants