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

fix(libcgroups): report CPU throttling stats in 'libcgroups::v2' #2524

Merged
merged 2 commits into from
Nov 17, 2023
Merged

fix(libcgroups): report CPU throttling stats in 'libcgroups::v2' #2524

merged 2 commits into from
Nov 17, 2023

Conversation

xiaoyang-sde
Copy link
Contributor

@xiaoyang-sde xiaoyang-sde commented Nov 12, 2023

fix #2513

This pull request adds logic to read relevant fields from cpu.stat file to populate the CpuThrottling struct for the stats method of libcgroups::v2::Cpu.

Reference: https://www.kernel.org/doc/Documentation/admin-guide/cgroup-v2.rst

CPU Interface Files
~~~~~~~~~~~~~~~~~~~

All time durations are in microseconds.

  cpu.stat
	A read-only flat-keyed file.
	This file exists whether the controller is enabled or not.

	It always reports the following three stats:

	- usage_usec
	- user_usec
	- system_usec

	and the following five when the controller is enabled:

	- nr_periods
	- nr_throttled
	- throttled_usec
	- nr_bursts
	- burst_usec

Signed-off-by: Xiaoyang Liu <siujoeng.lau@gmail.com>
@xiaoyang-sde
Copy link
Contributor Author

It seems that the stats method of libcgroup::v1::Cpu is using parse_flat_keyed_data to parse the cpu.stats file:

https://github.com/containers/youki/blob/7b5e8f238e45497fe041c9f622a9a4e0b3f0e7e4/crates/libcgroups/src/v1/cpu.rs#L66-L90

Should we use parse_flat_keyed_data in libcgroup::v2::Cpu?

@codecov-commenter
Copy link

codecov-commenter commented Nov 13, 2023

Codecov Report

Merging #2524 (c857857) into main (b14c7c3) will increase coverage by 0.03%.
Report is 19 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2524      +/-   ##
==========================================
+ Coverage   65.87%   65.91%   +0.03%     
==========================================
  Files         133      133              
  Lines       16868    16873       +5     
==========================================
+ Hits        11112    11121       +9     
+ Misses       5756     5752       -4     

@utam0k
Copy link
Member

utam0k commented Nov 13, 2023

Thanks for your PR!

Should we use parse_flat_keyed_data in libcgroup::v2::Cpu?

I prefer to use it if possible.

Signed-off-by: Xiaoyang Liu <siujoeng.lau@gmail.com>
@xiaoyang-sde
Copy link
Contributor Author

xiaoyang-sde commented Nov 14, 2023

Thanks for your PR!

Should we use parse_flat_keyed_data in libcgroup::v2::Cpu?

I prefer to use it if possible.

✅No problem!

It turns out that the following macro appears at least 3 times in the code base. This macro makes it easier to extract fields from the parsed flat keyed data. Should we move this common logic to crates/libcgroups/src/stats.rs?

macro_rules! get {
    ($name: expr => $field1:ident.$field2:ident) => {
        stats.$field1.$field2 =
            *stats_table
                .get($name)
                .ok_or_else(|| V2CpuStatsError::MissingField {
                    field: $name,
                    path: stats_path.clone(),
                })?;
    };
}

@xiaoyang-sde xiaoyang-sde changed the title fix(libcgroup): report CPU throttling stats in 'libcgroup::v2' fix(libcgroups): report CPU throttling stats in 'libcgroups::v2' Nov 14, 2023
@utam0k
Copy link
Member

utam0k commented Nov 17, 2023

It turns out that the following macro appears at least 3 times in the code base. This macro makes it easier to extract fields from the parsed flat keyed data. Should we move this common logic to crates/libcgroups/src/stats.rs?

Go ahead 👍 But please send out a new PR about it. When you will do it, please set me as a reviewer.

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

LGTM

@utam0k utam0k merged commit 3c7cc26 into youki-dev:main Nov 17, 2023
14 checks passed
@xiaoyang-sde xiaoyang-sde deleted the cpu-throttling branch November 17, 2023 07:33
This was referenced Dec 17, 2023
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.

libcgroups v2 manager stats doesn't set cpu throttling field
3 participants