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.

Sign up for GitHub

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

Jump to bottom

[17.03.x] Correct CPU usage calculation in presence of offline CPUs and newer Linux #31579 #31802

Merged
merged 1 commit into from
Mar 14, 2017
Merged

[17.03.x] Correct CPU usage calculation in presence of offline CPUs and newer Linux #31579 #31802

merged 1 commit into from
Mar 14, 2017

Conversation

vieux
Copy link
Contributor

@vieux vieux commented Mar 13, 2017

No description provided.

@vieux vieux added this to the 17.03.1 milestone Mar 13, 2017
@vieux vieux self-assigned this Mar 13, 2017
@tiborvass
Copy link
Contributor

LGTM

thaJeztah
thaJeztah approved these changes Mar 13, 2017
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tiborvass
tiborvass requested changes Mar 13, 2017
View reviewed changes
Copy link
Contributor

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

# github.com/docker/docker/daemon
23:03:00 ..\..\daemon\stats_collector.go:118: s.getNumberOnlineCPUs undefined (type *statsCollector has no field or method getNumberOnlineCPUs)
23:03:00 ..\..\daemon\stats_collector_windows.go:17: undefined: Collector

@vieux
Correct CPU usage calculation in presence of offline CPUs and newer L…
9b001c4 
…inux

In torvalds/linux@5ca3726 (released in v4.7-rc1) the
content of the `cpuacct.usage_percpu` file in sysfs was changed to include both
online and offline cpus. This broke the arithmetic in the stats helpers used by
`docker stats`, since it was using the length of the PerCPUUsage array as a
proxy for the number of online CPUs.

Add current number of online CPUs to types.StatsJSON and use it in the
calculation.

Keep a fallback to `len(v.CPUStats.CPUUsage.PercpuUsage)` so this code
continues to work when talking to an older daemon. An old client talking to a
new daemon will ignore the new field and behave as before.

Fixes moby#28941.

Signed-off-by: Ian Campbell <ian.campbell@docker.com>
(cherry picked from commit 115f91d)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@vieux
Copy link
Contributor Author

vieux commented Mar 13, 2017

@tiborvass PTAL

@vieux
Copy link
Contributor Author

vieux commented Mar 14, 2017

exp unrelated.

@vieux vieux merged commit 9a6c8c9 into moby:17.03.x Mar 14, 2017
@vieux vieux deleted the fix_stats branch March 14, 2017 01:09
@thaJeztah thaJeztah mentioned this pull request Mar 22, 2017
Merged
@eesprit eesprit mentioned this pull request Jun 26, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justincormack justincormack justincormack approved these changes

@tiborvass tiborvass tiborvass approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@vieux vieux

Labels
impact/api impact/changelog status/4-merge
Projects
None yet
Milestone
17.03.1
Development

Successfully merging this pull request may close these issues.
5 participants
@vieux @tiborvass @justincormack @thaJeztah @GordonTheTurtle