-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add mon_group support for resctrl. #2793
Merged
Merged
Changes from 14 commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
0f93f1a
Add mon_group support for resctrl.
3452cf3
Do not try to setup the root container.
18d297d
Update klog version to avoid errors from golangci-lint.
3feb264
Update klog version in cmd to avoid errors from golangci-lint.
7aaaff7
Fix go.sum
58a2e01
Check if container moved between control groups only if its running.
5561bd9
Get NUMA nodes from MachineInfo.Topology.
9a1e6cb
Make code thread safe again.
873cc47
Fix typo.
d07084c
Refactor resctrl collector setup.
801e883
Refactor resctrl utilies.
75fabd7
Better name vars.
a1619a7
Add missing python3 in Dockerfile.
0f274e4
Add missing procps in Dockerfile.
73c71b6
Merge branch 'master' of github.com:google/cadvisor into creatone/res…
03f8571
Use const instead of magic value.
29e1e19
Delete an unnecessary setting of c.running to false.
fc3b8ce
Do not wrap the error from cAdvisor.
ecb0156
Use path in error message.
e1f5e9b
Avoid goroutine looping.
3951953
Do not use fscommon package from runc/libcontainer.
7d36305
Fix const ASCII names.
ee42de2
Use same operator in func.
7c1eeb2
Introduce const variables.
a1ce724
Merge branch 'master' of github.com:google/cadvisor into creatone/res…
3ffe422
Introduce vendor_id in MachineInfo.
ab3ee30
Extend files which should be omitted when searching control group.
a986999
Add info about possible bug when reading resctrl values on AMD.
88b6b7c
Use empty struct map instead of boolean.
8bf947a
Move reading file logic.
c54e007
Use scanner to read tasks file.
dbb54d2
Change the way of searching for the control group.
731606d
Add comments. Use const value.
15be02e
Comment function.
d24ece6
Fix typo.
c9da6fb
Refactor getAllProcessThreads.
8162197
Refactor GetVendorID.
9c675e4
Rename VendorID.
a76478c
Resctrl collector should be aware of existing mon groups.
852f755
Optimization for finding control/monitoring group.
97987d8
Avoid having ugly errors.
c675d56
Merge branch 'master' of github.com:google/cadvisor into creatone/res…
e7628e7
Use strings.HasPrefix().
1db6ca2
Add comments.
fa9b5db
Rename variables.
b3f311b
Fix test.
bbde636
Use string map instead of int.
ea39458
Now there is no need to use procps in Dockerfile.
0ec4da2
Merge branch 'master' of github.com:google/cadvisor into creatone/res…
b76ee15
Update to go 1.17.
8c734a0
Add information about possible race condition.
fb4b9c8
Add warning when docker_only is not set.
00b42cf
Fix typo.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "updating mon groups"? Are you going to add and/or remove pids from them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you think that
--housekeeping_interval
is not enough and that we need another flag? We still can exclude restctrl metrics usingdisable_metrics
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, watching mon groups and making sure that have correct pids.
Thanks to that flag we can monitor containers that changed their control group during the cAdvisor run.
Setting
--resctrl_interval
to 0, disables updating mon groups.This is good behavior for users that want to use only monitoring features of Resctrl.
We cannot depend on
--housekeeping_interval
because cAdvisor will be updating containers instantly:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate more about
I'm afraid that it won't be enough in case where other components (cri-rm, runc, manually) manage ctr/mon groups manually. At least this comment suggest that "enabling rdt" + restctrl_intreval=0 is enough, which is not true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to consider the necessity of --restrcl_interval flag, because it makes another cmdline options that is hard to understand but gives a chance to limit overhead in case that there is no other manager of resctrl hierarchy. I'm not yet full convinced it is worh. Pawel can you provide rough estimation of overhead - I mean e.g. number of syscalls (listdir,readfile, cpuusage) in typical setup (e.g. 20 pods + system cgroups).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If something will change the control group, the cAdvisor will continue measuring in the new group.
cAdvisor manipulates only mon groups. Now it's aware of existing groups, even own prepared have "cadvisor" prefix,
e.g.
cadvisor-system.slice-docker-0f07bb8ed38c0f40c9424875f2cf417d19b27ca93b631640a2039b3478ee5858.scope
So will manipulate only the groups with that prefix.