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

#4375 - use memory usage without memory cache #4383

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

julienmathevet
Copy link
Contributor

@julienmathevet julienmathevet commented Jul 5, 2018

closes #4375

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@@ -41,8 +41,8 @@ func calculateCPUPercentWindows(v *types.StatsJSON) float64 {

// calculateMemUsageUnixNoCache calculate memory usage of the container.
// Page cache is intentionally excluded to avoid misinterpretation of the output.
func calculateMemUsageUnixNoCache(mem types.MemoryStats) float64 {
return float64(mem.Usage - mem.Stats["cache"])
func calculateMemUsageUnixNoCache(mem types.MemoryStats) uint64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

@danielnelson may need to weigh in, but I believe changing types isn't supported in certain outputs.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true but in this case I believe the field types are retained: usage is a uint64 and usage_percent is a float64. I think it would be nice to leave this function returning a float64 though so that our code is closer to the upstream code, and then convert to an uint64 just for the usage field.

Gopkg.lock Outdated
@@ -413,6 +413,217 @@
]
revision = "c43482518d410361b6c383d7aebce33d0471d7bc"

[[projects]]
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to this file should be removed.

@danielnelson danielnelson added this to the 1.7.2 milestone Jul 5, 2018
@danielnelson danielnelson added the fix pr to fix corresponding bug label Jul 5, 2018
@julienmathevet julienmathevet force-pushed the 4375-fix_focekr_mem_usage branch from 3e23835 to 4c9603d Compare July 6, 2018 12:51
@glinton glinton merged commit 4c27862 into influxdata:master Jul 17, 2018
glinton pushed a commit that referenced this pull request Jul 17, 2018
@rdxmb
Copy link
Contributor

rdxmb commented Sep 12, 2018

very good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker memory usage include cache, so is misleading
4 participants