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

docker stats: add blkio rate and blkio IOPS #21026

Closed

Conversation

zhangyong-huawei
Copy link

replaces #20741

blkio rate and IOPS is calculated by introducing PreBlkioStats and PreRead in
types.Stats, thus we can get the rate/IOPS by (BlkioStats - PreBlkioStats)/(Read - PreRead)

Now with the patch docker stats shows like below:

    $ docker stats
    CONTAINER           CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BIO(Bytes)          BIO(Rate)           BIO(IOPS)
    1285939c1fd3        0.07%               796 KB / 64 MB        1.21%               788 B / 648 B       3.568 MB / 512 KB   0 B / 0 B           0.00 / 0.00
    9c76f7834ae2        0.07%               2.746 MB / 64 MB      4.29%               1.266 KB / 648 B    12.4 MB / 0 B       0 B / 0 B           0.00 / 0.00
    d1ea048f04e4        0.03%               4.583 MB / 64 MB      6.30%               2.854 KB / 648 B    27.7 MB / 0 B       0 B / 0 B           0.00 / 0.00

Signed-off-by: Zhang Yong [email protected]

@icecrime icecrime added the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 8, 2016
@zhangyong-huawei
Copy link
Author

@thaJeztah code is rebased again.

@thaJeztah
Copy link
Member

Flaky test on win2lin? https://jenkins.dockerproject.org/job/Docker-PRs-Win2Lin/22938/console

12:40:04 PASS: docker_cli_run_test.go:3925: DockerSuite.TestRunNonExecutableCmd 1.010s
12:40:07 
12:40:07 ----------------------------------------------------------------------
12:40:07 FAIL: docker_cli_run_test.go:3936: DockerSuite.TestRunNonExistingCmd
12:40:07 
12:40:07 docker_cli_run_test.go:3942:
12:40:07     c.Fatalf("Run non-existing command should have errored with exit code 127, but we got exit: %d, State.ExitCode: %s", exit, stateExitCode)
12:40:07 ... Error: Run non-existing command should have errored with exit code 127, but we got exit: 125, State.ExitCode: -1 Cannot start container 9feabe3cb5e0c52a83484f8a7a277d284d8de214968c3b3ad84884ad7d579b51: [10] System error: invalid character 'b' looking for beginning of value
12:40:08 ----------------------------------------------------------------------

edit: yes, flaky test: #20462

@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 9, 2016
@thaJeztah
Copy link
Member

ping @cpuguy83 this is a replacement for / rebase of #20741

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 9, 2016
@calavera
Copy link
Contributor

if we ever want to support stats on Windows, I don't think we should add more linux specific fields for now. But I'll leave it to other maintainers.

@zhangyong-huawei
Copy link
Author

Maybe at that time we could have a --format flag on stats. Thus we could provide default
stats format for both linux and windows.

On 2016/3/11 8:14, David Calavera wrote:

if we ever want to support stats on Windows, I don't think we should add more fields to the stats for now. But I'll leave it to other maintainers.


Reply to this email directly or view it on GitHub #21026 (comment).

@coolljt0725
Copy link
Contributor

IMO, this PR only add a linux specific filed PreBlkioStats, but we already have BlkioStats , so it should not become more complicated to support stats on Windows by adding PreBlkioStats :)

@cpuguy83
Copy link
Member

I think is fine with a format flag. New fields don't need to be enabled by default.

blkio rate and IOPS is calculated by introducing PreBlkioStats and PreRead in
types.Stats, thus we can get the rate/IOPS by (BlkioStats - PreBlkioStats)/(Read - PreRead)

Now with the patch docker stats shows like below:
    $ docker stats
    CONTAINER           CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BIO(Bytes)          BIO(Rate)           BIO(IOPS)               PIDS
    1285939c1fd3        0.07%               796 KB / 64 MB        1.21%               788 B / 648 B       3.568 MB / 512 KB   0 B / 0 B           0.00 / 0.00               0
    9c76f7834ae2        0.07%               2.746 MB / 64 MB      4.29%               1.266 KB / 648 B    12.4 MB / 0 B       0 B / 0 B           0.00 / 0.00               0
    d1ea048f04e4        0.03%               4.583 MB / 64 MB      6.30%               2.854 KB / 648 B    27.7 MB / 0 B       0 B / 0 B           0.00 / 0.00               0

Signed-off-by: Zhang Yong <[email protected]>
@thaJeztah
Copy link
Member

I'm going to close this PR, we prefer to have a format flag first, and then reconsider adding more columns

@thaJeztah thaJeztah closed this Apr 14, 2016
@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label May 31, 2016
@thaJeztah
Copy link
Member

ping @zhangyong-huawei a --format flag is now added to docker stats; let us know if you want to work on this again (see #24987)

@zhangyong-huawei
Copy link
Author

Yeah,I’ll rebase my work on –format flag. Thanks for your kind remind.

发件人: Sebastiaan van Stijn [mailto:[email protected]]
发送时间: 2016年11月1日 0:59
收件人: docker/docker [email protected]
抄送: Zhangyong (Euler) [email protected]; Mention [email protected]
主题: Re: [docker/docker] docker stats: add blkio rate and blkio IOPS (#21026)

ping @zhangyong-huaweihttps://github.com/zhangyong-huawei a --format flag is now added to docker stats; let us know if you want to work on this again (see #24987#24987)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/21026#issuecomment-257351725, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AQs1C03Fd_4jN4lsjrQAzUWffbOg7qONks5q5h47gaJpZM4HrqEi.

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

Successfully merging this pull request may close these issues.

7 participants