Black Freezes in Containers on Nodes with a Large os.cpu_count()

[FHTMitchell]

Describe the bug

Black attempts to run with too many executed threads when running in a container (e.g. in kubernetes).

It is well understood that containers do not mask the number of physical cores available, despite the fact that the container itself might be limited by the amount of CPU it can use at one time. This means that using the result of os.cpu_count() to spin up that many threads can cause a huge number of threads to be created. The memory overhead of this can cause the program to freeze or crash. See how this has impacted other projects:

• https://bugs.python.org/issue36054
• multiprocessing.cpu_count() is dangerous in containers benoitc/gunicorn#2028
• https://medium.com/@chipiga86/python-kubernetes-cpu-pinning-eeaeee977c2e
• https://stackoverflow.com/questions/47989418/multiprocessing-python-program-inside-docker

We use black in our CI (via precommit) stack which runs on Kubernetes and are currently unable to run any builds using black (and so have had to turn it off for now).

To Reproduce

1. Simply log into your nearest 1000 core machine
2. Start a container on the machine with reasonable resource limits (e.g. 2 CPU & 1GB RAM)
3. Run black file1.py file2.py (must be more than 1 file)
4. Watch as black grinds to a halt trying to start 1000 threads

Expected behavior

Black does not grind to a halt...

Environment:

• Version: 21.9b0
• OS and Python version: Linux/[3.7|3.8|3.9]

Does this bug also happen on main?

Yes, you can see the offending code on HEAD here.

Resolution

I would suggest the easiest solution would be to add a --workers argument to black to optionally set the number of threads. I have a PR ready to go.

[cooperlees]

We try to limit the number of CLI switches. Are there generic APIs black could possibly query to:

• Workout if we're in a container
  • If so, maybe generate to 1 vCPU?
• Workout how many CPUs a container has?

that work in the majority of Linux containers?

Just asking and playing devils advocate here to see if we could do saner auto detection rather than add yet another a CLI switch (and I'm not ruling it out just wondering here).

[FHTMitchell]

Thank you for the quick reply @cooperlees.

We did talk about that here, and the issues I mentioned above have looked into it.

• One option is using len(os.sched_getaffinity(0)). This is the available CPUs but that requires that you set the cgroup on your container, which isn't yet supported by many container managers. I know our version of K8s doesn't yet.
• Another would be simply setting the maximum to be some reasonable number. This is what we do in win32 here. Do people really need to run on more that 32 (or even 16 or 8!) threads?
• Figuring out if we are in containers is not as simple as it sounds. There are ways for docker/k8s but this wouldn't be generic. Other container types would be ignored. A generic solution for how many CPUs a container is limited to is more or less impossible.

I'll submit my PR but happy for it to be rejected if a better solution is found. I'd rather get this solved sooner than later though, a lot of projects are currently blackless where I work :(

I understand why black attempts to reduce the number of CLI switches, and I support it, but I think this is a genuine use case, changing how the program runs rather than what it does.

[felix-hilden]

I think our opposition to new configuration values is more important in the context of formatting preferences, not as much in this case. But I think setting a maximum would also be fine! 8-16 seems reasonable, although I couldn't argue why not 32 or more.

[FHTMitchell]

Yeah I'd obviously be happy with just limiting the number of threads. It's up to you lot if you're worried about breaking people's workflow or not. Also happy to implement that PR.

[JelleZijlstra]

I think it's fine to add a new --workers flag. As Felix said, a new flag affecting how Black schedules formatting is not nearly as bad as one that affects the output.

[felix-hilden]

My money's on the flag too, because then we don't have to set an arbitrary limit to how much Black can be scaled for potentially huge workloads. Not that it's very likely to be an issue for more than 10 people in the world, but anyways.