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

Support changes in the number of cpus given by cpu.Counts #640

Open
danielnelson opened this issue Feb 21, 2019 · 4 comments
Open

Support changes in the number of cpus given by cpu.Counts #640

danielnelson opened this issue Feb 21, 2019 · 4 comments

Comments

@danielnelson
Copy link
Contributor

Is your feature request related to a problem? Please describe.

In Telegraf, it has been suggested that we report changes to the number of cpus without restarting the process. In KVM, and many other hypervisors the number of cpus can be hotplugged on a running VM.

Currently we are calling runtime.NumCPU() in Telegraf directly, but I would like to move this to use cpu.Counts. However, the implementation on Linux is the same runtime.NumCPU() call.

Describe the solution you'd like

Instead of using the cached number of CPU from Go, each call to cpu.Counts() should attempt to discover the number of CPUs again.

Describe alternatives you've considered

None

Additional context

influxdata/telegraf#5451

@Lomanic
Copy link
Collaborator

Lomanic commented Feb 21, 2019

As raised in #628 we should have OS-specific functions to gather the number of cores, runtime.NumCPU() is not sufficient (it returns the number available cores for the process, which can change afterwards on VMs indeed and we can't distinguish between logical and physical cores).

@mznet
Copy link
Contributor

mznet commented Feb 21, 2019

@Lomanic Refer to the document about runtime.NumCPU(), this method shows only the number of CPUs at process startup and can't reflect the changes after startup. To reflect the change of CPU properly, we should not use runtime.NumCPU so that need to find an alternative way, IMHO.
What about using SC_NPROCESSORS_ONLN or /proc/cpuinfo to get the number of logical or physical CPUs like original psutil implementation

Update: I didn't consider that those two ways are only available on linux. I take it back.

@danielnelson
Copy link
Contributor Author

What do you think about just pulling up the implementations from Go without the caching? Every platform would of course need a different implementation.

@Lomanic
Copy link
Collaborator

Lomanic commented Feb 23, 2019

I think we shouldn't reuse runtime.NumCPU code with caching removed, we will still have to port psutil code to implement logical/physical cores count (the logical argument) afterwards.

Also almost all the platforms implement processor affinity (how many cores the process can run on) in runtime.NumCPU, which is not exactly the number of physical or logical cores on the machine. Plus I'm not even sure it gets updated after process startup and at least one implementation relies on assembly.

Lomanic added a commit to Lomanic/gopsutil that referenced this issue Mar 3, 2019
Lomanic added a commit that referenced this issue Mar 13, 2019
[cpu][linux] Add support for logical arg in Counts #640 #628
shirou added a commit that referenced this issue Aug 16, 2019
[cpu][windows] Add support for logical arg in Counts #640 #628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants