-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
update stats.go,Update "docker stats " calculations #13627
Conversation
bug fix moby#13626
Please sign your commits following these rules: $ git clone -b "Liuyanglong-patch-1" git@github.com:Liuyanglong/docker.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f Ammending updates the existing PR. You DO NOT need to open a new one. |
I think |
Agreed; for example, on a 4-core system, cpu usage can be anywhere between 0 and 400%, so it has to be multiplied by the number of cores. I'm going to close this PR, because I don't think this change is correct, but feel free to comment if you think I closed this PR by mistake. |
Do we need to add this to the docs? I've been asked for so many times about the "over 100 percentage of CPU usage of Docker" |
@HuKeping I think it's fairly standard practice to report > 100% for multiple CPUs, but I'm not against adding it to the docs, so feel free to create a PR |
@thaJeztah I have a couple of follow-up questions on this. We are using cpuset-cpus with docker. To get CPU utilization, would the multiplier Another question I have is that even though we are using cpuset-cpus, in the cgroups files, we see more than the set number of CPUs having non-zero usage. For example, on a host of 24 cores and (cpuset-cpus=1,2), more than 2 cores show usage > 0. Why is that so? Shouldn't only two (specific) cores be used for the container in this case? |
Sorry to ask question on this closed PR, I have a question about the algorithm to calculate the cpu usage of a container. As the
|
I am also confused about why need to get |
@thaJeztah Hi, when calculating cpu statistics, from which file the CPU system usage will be read? Is it the cpu value for the system only inside the |
Stats are returned by the containerd API; Line 1393 in 9e7bbdb
moby/libcontainerd/remote/client.go Lines 386 to 395 in 9e7bbdb
Which uses the cgroups module to collect the status (it contains implementations for cgroups v1 and v2); https://github.com/containerd/cgroups I don't know what exactly is used there from the top of my head, but perhaps that code allows you to find the details |
Thanks !! I'll go through the code. Hope, the same approach was taken to monitor the network IO as well |
There's some code in the stats collector for networking; Lines 156 to 158 in 7b9275c
Oh, I realised there's some code related to system CPU in the collector code as well, in case it's relevant to your question; moby/daemon/stats/collector.go Line 123 in 2773f81
|
Yeah, but the problem is which file this |
Looking at I see this declaration
and summation of first 7 |
bug fix #13626
When I read source code about " docker stat", the current is :
cpuPercent = (cpuDelta / systemDelta) * float64(len(v.CpuStats.CpuUsage.PercpuUsage))
https://github.com/docker/docker/blob/0d445685b8d628a938790e50517f3fb949b300e0/api/client/stats.go#L199
I do not understand why " * float64(len(v.CpuStats.CpuUsage.PercpuUsage)) ", (cpuDelta / systemDelta) is Correct。
cpuDelta = float64(v.CpuStats.CpuUsage.TotalUsage - previousCPU)
systemDelta = float64(v.CpuStats.SystemUsage - previousSystem)
so total cpu delta / total system delta is correct , not need to multiplied by CPU kernal count.