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

Feature request: os: os.availableProcessors() #28855

Closed
NicolasSchwarzer opened this issue Jul 25, 2019 · 17 comments
Closed

Feature request: os: os.availableProcessors() #28855

NicolasSchwarzer opened this issue Jul 25, 2019 · 17 comments
Labels
feature request Issues that request new features to be added to Node.js. os Issues and PRs related to the os subsystem.

Comments

@NicolasSchwarzer
Copy link

Is your feature request related to a problem? Please describe.
It's unable to get actual available amount of processors in a docker container running on image node:10.16.0, of which cpus is limited by docker container resource constraints. For detail information, please have a look at issue #28762.

Describe the solution you'd like
Provide os.availableProcessors() api to get the actual available amount of processors.

Describe alternatives you've considered
If possible, it's better to fix the output of os.cpus().length, e.g. by using Proxy to change the value of length property. Because most node modules use os.cpus().length to decide how many processes it should fork, which may exceed the limitation of cpus in the docker container.

@silverwind
Copy link
Contributor

silverwind commented Jul 25, 2019

I would first start investigating if Docker/Moby or their upstream projects can fix it.

Both/proc/cpuinfoand getconf _NPROCESSORS_ONLN are heavily used methods to obtain number of CPU cores but for some reason both do not reflect Docker's CPU limits.

@silverwind silverwind added the feature request Issues that request new features to be added to Node.js. label Jul 25, 2019
@juanarbol
Copy link
Member

juanarbol commented Jul 26, 2019

I think this issue happens also with memory, using os.totalmem(), does not take container resources, It'll take the Docker engine resources, I guess

@NicolasSchwarzer
Copy link
Author

NicolasSchwarzer commented Jul 26, 2019

@silverwind Okay, thank you, waiting for your good news!

@NicolasSchwarzer
Copy link
Author

@juanarbol I've did some investigation, it seems work with memory,please see below:

$ docker container run --rm --memory="3072m" -it node:10.16.0 bash
root@e39346d06455:/# node
> const os = require('os');
undefined
> console.log(os.freemem());
76783616
undefined
> console.log(os.totalmem());
2096164864
undefined
> .exit
root@e39346d06455:/# exit
exit

@juanarbol
Copy link
Member

@NicolasSchwarzer I got the same is total memory as you did, but I just gave 100mb as maximum memory usage, I think we're getting the Docker engine total memory also

$ docker container run --rm --memory="100m" -it node:10.16.0 bash
root@fe57c5bcdc46:/# node
> const os = require('os');
undefined
> console.log(os.freemem())
119603200
undefined
> console.log(os.totalmem())
2096164864 
undefined
> .exit
root@fe57c5bcdc46:/# exit

Also free memory is around 119.6mb, that's greater than my given 100mb to that container, and total memory is more than 1gb of ram

@NicolasSchwarzer
Copy link
Author

@juanarbol You're right, I've tried and it shows exactly as what you described.

@silverwind Please also have a look at the same kind bug of os. freemem() & os. totalmem().

@Fishrock123 Fishrock123 added the os Issues and PRs related to the os subsystem. label Jul 29, 2019
@silverwind
Copy link
Contributor

silverwind commented Jul 31, 2019

Guess you misunderstood me, I don't indent to chase this down to the correct project. I found this which might point someone in the right directions (kernel or runc). Out of curiosity, I did check in nginx's source and found it relies on _NPROCESSORS_ONLN which is also affected by this issue.

Anyways, if we want to do something about this in node, it should probably interface with /sys/fs/cgroup but the issue remains that this is something very Linux-specific and Node.js aims to be cross-platform, so it's a rather bad candidate for inclusion in core I think.

@NicolasSchwarzer
Copy link
Author

Okay, I get it. So is there a confirmed conclusion about whether to support in node? Indeed this is a platform-specific issue, I could raise an issue to docker if you confirmed not acceptable.

@silverwind
Copy link
Contributor

I'd say its pretty certain that we wouldn't accept something only relevant to one platform. There have been various proposals for Linux-specific features in the past and most of them have been rejected.

@mhdawson
Copy link
Member

mhdawson commented Aug 1, 2019

@NicolasSchwarzer most of the platform specific code is in https://github.com/libuv/libuv. You might look there to see if you can find the code Node.js uses in libuv and have a discussion in the libuv repo if that should reflect the limits withing a docker container. I personally think being able to find the actual number of processors available is useful.

@NicolasSchwarzer
Copy link
Author

@silverwind Thanks for your answering, I will close this issue.

@mhdawson Okay, I will investigate libuv later, thanks for your advice.

@jdmarshall
Copy link

jdmarshall commented Mar 6, 2020

@NicolasSchwarzer @silverwind is it really a “Platform specific issue” if it’s the platform most production Node runs on, almost all build jobs and quite a lot of dev loops also use?

OS X and I believe Windows run Docker on a Linux VM. so you’re still dealing with Linux cgroup issues even when you are developing on a Mac.

And my Build agents pretty much only run on Windows or OS X if I need some UI testing and still have a small test matrix.

@harish2704
Copy link

Update: It seems like , cputset related issue inside docker container can be solved using lxcfs ( even without patching docker project ) . See https://github.com/lxc/lxcfs#using-with-docker

@stringang
Copy link

stringang commented May 12, 2022

With k8s becoming increasingly popular and nodejs running in k8s, this request is necessary.

@bnoordhuis
Copy link
Member

It's coming but it's blocked on upgrading libuv, see #42340.

@jdmarshall
Copy link

The Q&A on the associated PR says that it does not fulfill the spirit of this request. If that's so I'm going to re-file this issue because Docker is where almost all Node applications run in production and still having to handle these problems out-of-process is a disservice to the community.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jan 7, 2023

I commented on your comment in the linked PR but to recap/expound: as long as you use docker's cpuset functionality - not its exquisitely poorly named --cpus=... flag - things should Just Work(TM).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. os Issues and PRs related to the os subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants