Fixes #10824. Adjusted docker stats memory output by Sergeant007 · Pull Request #32777 · moby/moby

Sergeant007 · 2017-04-22T11:29:23Z

Signed-off-by: Sergey Tryuber [email protected]

- What I did

"docker stats" CLI command now outputs memory usage without page cache. Fixes #10824.

- How I did it

Followed to the solution suggested by @crosbymichael in #10824 - "just subtract the cache from the usage value before displaying it to the user".

- How to verify it

#10824 contains the debug steps to prove that "docker stats" used to include page cache in its output. This PR corrects the behavior.

Experiment (Docker version 17.06.0-dev, build 1eec7b5). The steps are done inside development container, you would need to open several terminals to the development container.

Step 0. build the binaries and then start docker daemon listening on tcp

// start with tcp since curl in development container cannot work with file sockets
$dockerd -D -H tcp://0.0.0.0:2375

Step 1. start a container with bash

docker -H localhost:2375 run --name mem_stats_test --interactive --tty ubuntu:14.04 /bin/bash

Step 2. open a new terminal window and ensure that memory consumption in stats is low

$ docker -H localhost:2375 stats mem_stats_test --no-stream
CONTAINER           CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O           PIDS
mem_stats_test      0.00%               1.699MiB / 15.58GiB   0.01%               648B / 0B           0B / 0B             1
$
$curl http://localhost:2375/containers/mem_stats_test/stats?stream=false | python -m json.tool
...
    "memory_stats": {
        "limit": 16724922368,
        "max_usage": 2629632,
        "stats": {
            "active_anon": 507904,
            "active_file": 0,
            "cache": 20480,                  <-- CACHE
            "dirty": 0,
            "hierarchical_memory_limit": 9223372036854771712,
            "inactive_anon": 4096,
            "inactive_file": 0,
            "mapped_file": 0,
            "pgfault": 1377,
            "pgmajfault": 0,
            "pgpgin": 754,
            "pgpgout": 629,
            "rss": 491520,
            "rss_huge": 0,
            "total_active_anon": 507904,
            "total_active_file": 0,
            "total_cache": 20480,
            "total_dirty": 0,
            "total_inactive_anon": 4096,
            "total_inactive_file": 0,
            "total_mapped_file": 0,
            "total_pgfault": 1377,
            "total_pgmajfault": 0,
            "total_pgpgin": 754,
            "total_pgpgout": 629,
            "total_rss": 491520,
            "total_rss_huge": 0,
            "total_unevictable": 0,
            "total_writeback": 0,
            "unevictable": 0,
            "writeback": 0
        },
        "usage": 1802240                     <-- OVERALL USAGE
    },
...

Step 3. Let is work for several hours to ensure that memory doesn't grow significanly (or believe me ans skip this step)

Step 4. Do some heavy file operations that grow page cache (in docker container console from Step 1)

# # Once following operation is finished, page cache will grow, but no additional "real" memory consumption is added
# dd if=/dev/urandom of=/tmp/1GB.bin bs=64M count=16 iflag=fullblock
16+0 records in
16+0 records out
1073741824 bytes (1.1 GB) copied, 10.9137 s, 98.4 MB/s

Step 5. Check docker stats again to see that there is no page cache included in the output

$ # reported memoty DID NOT grew up (significantly)!!!
$ docker -H localhost:2375 stats mem_stats_test --no-stream
CONTAINER           CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O           PIDS
mem_stats_test      0.00%               4.039MiB / 15.58GiB   0.03%               648B / 0B           0B / 0B             1
$curl http://localhost:2375/containers/mem_stats_test/stats?stream=false | python -m json.tool
...
"memory_stats": {
        "limit": 16724922368,
        "max_usage": 1145520128,
        "stats": {
            "active_anon": 520192,
            "active_file": 0,
            "cache": 1073766400,                          <-- OUR PAGE CACHE
            "dirty": 0,
            "hierarchical_memory_limit": 9223372036854771712,
            "inactive_anon": 0,
            "inactive_file": 1073741824,
            "mapped_file": 0,
            "pgfault": 2050,
            "pgmajfault": 0,
            "pgpgin": 263492,
            "pgpgout": 1221,
            "rss": 495616,
            "rss_huge": 0,
            "total_active_anon": 520192,
            "total_active_file": 0,
            "total_cache": 1073766400,
            "total_dirty": 0,
            "total_inactive_anon": 0,
            "total_inactive_file": 1073741824,
            "total_mapped_file": 0,
            "total_pgfault": 2050,
            "total_pgmajfault": 0,
            "total_pgpgin": 263492,
            "total_pgpgout": 1221,
            "total_rss": 495616,
            "total_rss_huge": 0,
            "total_unevictable": 0,
            "total_writeback": 0,
            "unevictable": 0,
            "writeback": 0
        },
        "usage": 1078001664                                  <-- THIS INCLUDES PAGE CACHE !!!!
...

Step 6. Conclusion
With this PR docker stats outputs reasonable amount of used memory (no page cache included)

- Description for the changelog

"docker stats" command outputs memory usage with no page cache memory included.

- A picture of a cute animal (not mandatory but encouraged)

My cat Tom.

boaz0

unit-tests are missing.

GordonTheTurtle · 2017-04-27T17:47:03Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "10824-dockerstats-misleading-mem" [email protected]:Sergeant007/docker.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354520728
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Sergeant007 · 2017-04-27T18:29:30Z

@ripcurld0, @vdemeester
Added unit tests. Please, review.

thaJeztah

left some suggestions :)

thaJeztah · 2017-04-27T22:34:02Z

why the float64(usedNoCache)? (usedNoCache already is a float64)

thaJeztah · 2017-04-27T22:36:03Z

you can pass memLimit here instead of v.MemoryStats.Limit, and change the signature of calculateMemPercentUnixNoCache to;

func calculateMemPercentUnixNoCache(limit float64, usedNoCache float64) float64

thaJeztah · 2017-04-27T23:33:43Z

Perhaps use GoDoc convention here;

// calculateMemUsageUnixNoCache calculates the amount of memory excluding page cache

or something like that 😄

Signed-off-by: Sergey Tryuber <[email protected]>

Sergeant007 · 2017-04-28T04:48:29Z

Hi @thaJeztah
Thanks for good suggestions, I've corrected all of them. Please review.
I wasn't able to find any "godoc documentation convention" - only blog posts. So I did my best there. Please, advise if necessary.

thaJeztah

LGTM, thanks!

boaz0

👍

Sergeant007 · 2017-05-10T15:53:00Z

Hi @stevvooe,
I see that you was assigned to merge the pull request, but now there are merge conflict. Will you be able to resolve them by yourself?

AkihiroSuda · 2017-05-11T01:27:55Z

Please resolve by yourself

AkihiroSuda · 2017-05-11T01:28:35Z

Also, now this needs to be moved to https://github.com/docker/cli

Sergeant007 · 2017-05-13T03:03:05Z

Hi @AkihiroSuda

Please resolve by yourself
Also, now this needs to be moved to https://github.com/docker/cli

This does not "needs to be moved", this is throwing away current and making a brand new pull request and large chunk of "paper work", just because you, guys, are not friendly to 3rd party contributions (or slow, or whatever else - please, find and fire).

Unfortunately I'll have time to do the adjustment only in next several weeks.

AkihiroSuda · 2017-05-13T23:16:30Z

CL is now Docker, Inc.'s product and not under the scope of Moby project. (#32694)

I carried this PR to docker/cli#80

you, guys, are not friendly to 3rd party contributions (or slow, or whatever else - please, find and fire).

Even I'm one of 3rd party contributors, not an employee of Docker, Inc. 😕

thaJeztah · 2017-05-15T08:58:35Z

@Sergeant007 please try to keep your comments respectful? We realise there's extra work needed during the transition to a separate Moby project and Docker product, which we're right in the middle of. We do try to review and merge PR's as quickly as possible, but rebases and merge-conflicts cannot be avoided in a fast moving repository, but are happy to help where needed.

[Carry moby/moby#32777] Adjusted docker stats memory output

AkihiroSuda · 2017-05-18T04:20:52Z

merged docker/cli#80, closing

[Carry moby/moby#32777] Adjusted docker stats memory output Upstream-commit: 85c2330dfa4d3845d4cd5f4502709335bce8ee0e Component: cli

GordonTheTurtle added the status/0-triage label Apr 22, 2017

Sergeant007 mentioned this pull request Apr 22, 2017

Docker stats memory usage is misleading #10824

Closed

boaz0 suggested changes Apr 26, 2017

View reviewed changes

vdemeester added status/2-code-review and removed status/0-triage labels Apr 26, 2017

vdemeester requested review from AkihiroSuda, cpuguy83, crosbymichael and thaJeztah April 26, 2017 13:22

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Apr 27, 2017

Sergeant007 force-pushed the 10824-dockerstats-misleading-mem branch from f1e18e6 to 31fe27a Compare April 27, 2017 18:23

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Apr 27, 2017

Sergeant007 force-pushed the 10824-dockerstats-misleading-mem branch from 31fe27a to 4c56d4a Compare April 27, 2017 18:26

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Apr 27, 2017

thaJeztah requested changes Apr 27, 2017

View reviewed changes

Adjusted docker stats memory output

706fc05

Signed-off-by: Sergey Tryuber <[email protected]>

Sergeant007 force-pushed the 10824-dockerstats-misleading-mem branch from 4c56d4a to 706fc05 Compare April 28, 2017 04:44

thaJeztah approved these changes Apr 28, 2017

View reviewed changes

boaz0 approved these changes Apr 30, 2017

View reviewed changes

thaJeztah added the impact/cli label May 5, 2017

GordonTheTurtle assigned stevvooe May 7, 2017

AkihiroSuda mentioned this pull request May 13, 2017

[Carry moby/moby#32777] Adjusted docker stats memory output docker/cli#80

Merged

zoobab mentioned this pull request May 17, 2017

docker stats varies through time julman99/eatmemory#5

Closed

thaJeztah added a commit to docker/cli that referenced this pull request May 17, 2017

Merge pull request #80 from AkihiroSuda/adjust-docker-stats

85c2330

[Carry moby/moby#32777] Adjusted docker stats memory output

AkihiroSuda closed this May 18, 2017

thaJeztah mentioned this pull request Jul 13, 2017

"docker stats" on Linux uses process virtual memory size instead of reporting actually used memory #34073

Closed

thaJeztah mentioned this pull request Sep 27, 2017

Service consumes more than what is limited #33526

Closed

AkihiroSuda mentioned this pull request Mar 25, 2020

Docker stats memory doesn't include tmpfs usage #40727

Closed

thaJeztah mentioned this pull request Jul 6, 2021

Exclude inode, dentry and other slabs from MEM USAGE docker/cli#3171

Open

Uh oh!

Conversation

Sergeant007 commented Apr 22, 2017

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Uh oh!

boaz0 left a comment

Choose a reason for hiding this comment

Uh oh!

GordonTheTurtle commented Apr 27, 2017

Uh oh!

Sergeant007 commented Apr 27, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah Apr 27, 2017

Choose a reason for hiding this comment

Uh oh!

Sergeant007 commented Apr 28, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

boaz0 left a comment

Choose a reason for hiding this comment

Uh oh!

Sergeant007 commented May 10, 2017

Uh oh!

AkihiroSuda commented May 11, 2017

Uh oh!

AkihiroSuda commented May 11, 2017

Uh oh!

Sergeant007 commented May 13, 2017

Uh oh!

AkihiroSuda commented May 13, 2017

Uh oh!

thaJeztah commented May 15, 2017

Uh oh!

AkihiroSuda commented May 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants