Skip to content

throttle.* metrics must be kept for non-CFQ schedulers - #147

Merged
fuweid merged 2 commits into
containerd:masterfrom
kzys:blkio-fix
Mar 8, 2020
Merged

throttle.* metrics must be kept for non-CFQ schedulers#147
fuweid merged 2 commits into
containerd:masterfrom
kzys:blkio-fix

Conversation

@kzys

@kzyskzys commented Mar 5, 2020

Copy link
Copy Markdown
Member

Since c4b9ac5, Stat() ignores
/sys/fs/cgroup/blkio/$cgroup/blkio.throttle.io_serviced and
/sys/fs/cgroup/blkio/$cgroup/blkio.throttle.io_service_bytes when
/sys/fs/cgroup/blkio/$cgroup/blkio.io_serviced_recursive is there.

However the existance of this file only indicates that the kernel is
compiled with the CFQ scheduler. There is a chance that a cgroup uses
devices with a different scheduler such as "none".

Signed-off-by: Kazuyoshi Kato katokazu@amazon.com

Since c4b9ac5, Stat() ignores
/sys/fs/cgroup/blkio/$cgroup/blkio.throttle.io_serviced and
/sys/fs/cgroup/blkio/$cgroup/blkio.throttle.io_service_bytes when
/sys/fs/cgroup/blkio/$cgroup/blkio.io_serviced_recursive is there.
However the existance of this file only indicates that the kernel is
compiled with the CFQ scheduler. There is a chance that a cgroup uses
devices with a different scheduler such as "none".
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@kzys

kzys commented Mar 5, 2020

Copy link
Copy Markdown
MemberAuthor

Looks like the test failure is not related to my change.

Timed out while waiting for the machine to boot. This means that
Vagrant was unable to communicate with the guest machine within
the configured ("config.vm.boot_timeout" value) time period.

@Zyqsempai

Copy link
Copy Markdown
Contributor

@kzys Try to restart CI by pushing empty commit, it happens sometimes.

This change essentially skips the test in Travis though.
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@kzys

kzys commented Mar 6, 2020

Copy link
Copy Markdown
MemberAuthor

@Zyqsempai Thanks! The test is passing, while I have to skip the test in Travis though.

@AkihiroSuda

Copy link
Copy Markdown
Member

@fuweid PTAL

@crosbymichael

Copy link
Copy Markdown
Member

LGTM

@fuweidfuweid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kzys@Zyqsempai@AkihiroSuda@crosbymichael@fuweid