Add mon_group support for resctrl. #2793

Creatone · 2021-01-28T16:22:29Z

On a system with RDT monitoring features, the root directory, and other top levels
directories contain a folder named "mon_groups" in which additional
"MON" groups can be created to monitor subsets of tasks from a "CTRL_MON" group that is their ancestor.

There is a limitation of the number of "CTRL_MON" groups.
So in case when there are so many containers and as a user, you only want to use monitoring features, it's easy to reach this limit.
That's why "MON" groups are introduced.

This PR adds monitoring RDT features by mon_groups.
Signed-off-by: Paweł Szulik pawel.szulik@intel.com

Creatone · 2021-01-28T16:36:48Z

/ok-to-test

iwankgb

Some general remarks:

We do not support any Golang version lower than 1.15 so you can safely use %w instead of %v when wrapping errors.

iwankgb · 2021-03-04T15:30:44Z

cmd/cadvisor.go

@@ -77,6 +77,8 @@ var rawCgroupPrefixWhiteList = flag.String("raw_cgroup_prefix_whitelist", "", "A

 var perfEvents = flag.String("perf_events_config", "", "Path to a JSON file containing configuration of perf events to measure. Empty value disabled perf events measuring.")

+var resctrlInterval = flag.Duration("resctrl_interval", 0, "Resctrl mon groups updating interval. Zero value disables updating mon groups.")


What do you mean by "updating mon groups"? Are you going to add and/or remove pids from them?

Why do you think that --housekeeping_interval is not enough and that we need another flag? We still can exclude restctrl metrics using disable_metrics.

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:

... I0315 12:54:47.268474 22054 container.go:593] [/some_cgroup] Housekeeping took 331.572µs I0315 12:54:47.268948 22054 container.go:593] [/some_cgroup] Housekeeping took 429.79µs I0315 12:54:47.269348 22054 container.go:593] [/some_cgroup] Housekeeping took 356.188µs I0315 12:54:47.269363 22054 container.go:593] [/machine.slice/libpod-conmon-e361822894402b35f7230ccd133725e8d52e97c8eddd3ba77cd0dc3fc9ee46ad.scope] Housekeeping took 1.506114ms I0315 12:54:47.269385 22054 container.go:593] [/system.slice/thermos.service] Housekeeping took 1.48471ms I0315 12:54:47.269919 22054 container.go:593] [/some_cgroup] Housekeeping took 528.066µs I0315 12:54:47.270333 22054 container.go:593] [/some_cgroup] Housekeeping took 371.664µs I0315 12:54:47.270611 22054 container.go:593] [/some_cgroup] Housekeeping took 220.11µs I0315 12:54:47.270855 22054 container.go:593] [/some_cgroup] Housekeeping took 188.26µs I0315 12:54:47.271074 22054 container.go:593] [/machine.slice/libpod-conmon-df33bd5e05ec42afd4c39cc0a1417c227553de2bd7fc510de00fb7341d59d263.scope] Housekeeping took 1.682142ms I0315 12:54:47.271163 22054 container.go:593] [/some_cgroup] Housekeeping took 250.002µs I0315 12:54:47.271407 22054 container.go:593] [/some_cgroup] Housekeeping took 185.349µs ...

Could you elaborate more about

Setting --resctrl_interval to 0, disables updating mon groups.
This is good behavior for users that want to use only monitoring features of Resctrl.

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.

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).

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.

resctrl/collector.go

resctrl/utils.go

iwankgb · 2021-03-07T21:25:03Z

@Creatone do not force-push, please. Ability to browse history of your changes will be appreciated.

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone · 2021-03-15T13:50:19Z

@Creatone do not force-push, please. Ability to browse history of your changes will be appreciated.

Only for rebase.

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone · 2021-03-15T15:26:05Z

/retest

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone · 2021-03-17T17:38:45Z

/retest

Creatone · 2021-03-17T17:40:47Z

There is something with k8s test infrastructure.

@Creatone: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-cadvisor-e2e 75fabd7 link /test pull-cadvisor-e2e
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Creatone · 2021-03-18T07:17:46Z

/retest

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

resctrl/collector.go

resctrl/utils.go

iwankgb

Take a look at my comments, please. Sorry for long delay.

Creatone · 2021-08-16T13:55:07Z

/retest

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone · 2021-08-20T08:25:16Z

@iwankgb PTAL

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

deploy/Dockerfile

resctrl/collector.go

resctrl/utils.go

ppalucki · 2021-08-24T10:04:18Z

resctrl/utils.go

 		}

-		processThreads, err := getAllProcessThreads(threadFiles)
+		properContainerName := fmt.Sprintf("%s-%s", monGroupPrefix, strings.Replace(containerName, "/", "-", -1))
+		monGroupPath = filepath.Join(controlGroupPath, monitoringGroupDir, properContainerName)


is there a risk that e.g.

kubepods/pod-x

kubepods/pod-x/container-y
will be created and then you have an race condition, that only of those will contain pids (other will be stealed)

8c734a0 will be sufficient for you?

ppalucki · 2021-08-24T10:06:24Z

resctrl/utils.go

+			}
+			for _, thread := range processThreads {
+				err = intelrdt.WriteIntelRdtTasks(monGroupPath, thread)
+				if err != nil {


what if just of the pids disapperead but rest are ok? why are you removing all the group

That will be a race condition which --resctrl_interval fix.

I think that we shouldn't monitor something that cannot be set by us.

resctrl/utils.go

…ctrl

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

…ctrl

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone · 2021-09-09T15:23:19Z

@ppalucki @iwankgb PTAL

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

iwankgb

@Creatone thank you for your patience!

iwankgb · 2021-09-21T08:23:27Z

@Creatone one last request: squash the commits to some reasonable amount and content 🙇

Creatone · 2021-09-21T08:24:24Z

Can be a single one? :)

* Add mon_group support for resctrl. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Do not try to setup the root container. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Update klog version to avoid errors from golangci-lint. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Update klog version in cmd to avoid errors from golangci-lint. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix go.sum Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Check if container moved between control groups only if its running. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Get NUMA nodes from MachineInfo.Topology. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Make code thread safe again. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix typo. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Refactor resctrl collector setup. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Refactor resctrl utilies. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Better name vars. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add missing python3 in Dockerfile. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add missing procps in Dockerfile. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use const instead of magic value. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Delete an unnecessary setting of c.running to false. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Do not wrap the error from cAdvisor. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use path in error message. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Avoid goroutine looping. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Do not use fscommon package from runc/libcontainer. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix const ASCII names. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use same operator in func. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Introduce const variables. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Introduce vendor_id in MachineInfo. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Extend files which should be omitted when searching control group. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add info about possible bug when reading resctrl values on AMD. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use empty struct map instead of boolean. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Move reading file logic. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use scanner to read tasks file. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Change the way of searching for the control group. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add comments. Use const value. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Comment function. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix typo. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Refactor getAllProcessThreads. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Refactor GetVendorID. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Rename VendorID. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Resctrl collector should be aware of existing mon groups. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Optimization for finding control/monitoring group. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Avoid having ugly errors. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use strings.HasPrefix(). Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add comments. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Rename variables. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix test. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Use string map instead of int. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Now there is no need to use procps in Dockerfile. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Update to go 1.17. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add information about possible race condition. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Add warning when docker_only is not set. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com> * Fix typo. Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

google-cla bot added the cla: yes label Jan 28, 2021

k8s-ci-robot added the ok-to-test label Jan 28, 2021

Creatone force-pushed the creatone/resctrl branch 2 times, most recently from bead82b to 1a4f1f6 Compare January 30, 2021 08:39

Creatone mentioned this pull request Feb 4, 2021

How to collect memory/cache bandwidth usage using cAdvisor? #2797

Closed

iwankgb requested changes Mar 7, 2021

View reviewed changes

Paweł Szulik added 2 commits March 15, 2021 14:45

Add mon_group support for resctrl.

0f93f1a

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Do not try to setup the root container.

3452cf3

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Creatone force-pushed the creatone/resctrl branch from df6d638 to 3452cf3 Compare March 15, 2021 13:45

Paweł Szulik added 3 commits March 15, 2021 15:39

Update klog version to avoid errors from golangci-lint.

18d297d

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Update klog version in cmd to avoid errors from golangci-lint.

3feb264

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Fix go.sum

7aaaff7

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Paweł Szulik added 7 commits March 15, 2021 16:27

Check if container moved between control groups only if its running.

58a2e01

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Get NUMA nodes from MachineInfo.Topology.

5561bd9

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Make code thread safe again.

9a1e6cb

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Fix typo.

873cc47

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Refactor resctrl collector setup.

d07084c

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Refactor resctrl utilies.

801e883

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Better name vars.

75fabd7

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Paweł Szulik added 2 commits March 18, 2021 09:12

Add missing python3 in Dockerfile.

a1619a7

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Add missing procps in Dockerfile.

0f274e4

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

iwankgb reviewed May 7, 2021

View reviewed changes

iwankgb requested changes May 7, 2021

View reviewed changes

Paweł Szulik added 7 commits August 16, 2021 15:57

Add comments. Use const value.

731606d

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Comment function.

15be02e

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Fix typo.

d24ece6

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Refactor getAllProcessThreads.

c9da6fb

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Refactor GetVendorID.

8162197

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Rename VendorID.

9c675e4

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Resctrl collector should be aware of existing mon groups.

a76478c

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Paweł Szulik added 2 commits August 20, 2021 15:36

Optimization for finding control/monitoring group.

852f755

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Avoid having ugly errors.

97987d8

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

ppalucki reviewed Aug 24, 2021

View reviewed changes

Paweł Szulik added 10 commits August 24, 2021 17:02

Merge branch 'master' of github.com:google/cadvisor into creatone/res…

c675d56

…ctrl

Use strings.HasPrefix().

e7628e7

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Add comments.

1db6ca2

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Rename variables.

fa9b5db

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Fix test.

b3f311b

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Use string map instead of int.

bbde636

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Now there is no need to use procps in Dockerfile.

ea39458

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Merge branch 'master' of github.com:google/cadvisor into creatone/res…

0ec4da2

…ctrl

Update to go 1.17.

b76ee15

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Add information about possible race condition.

8c734a0

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Paweł Szulik added 2 commits September 10, 2021 13:07

Add warning when docker_only is not set.

fb4b9c8

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

Fix typo.

00b42cf

Signed-off-by: Paweł Szulik <pawel.szulik@intel.com>

iwankgb approved these changes Sep 21, 2021

View reviewed changes

Creatone merged commit 80e6574 into google:master Sep 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mon_group support for resctrl. #2793

Add mon_group support for resctrl. #2793

Creatone commented Jan 28, 2021

Creatone commented Jan 28, 2021

iwankgb left a comment

iwankgb Mar 4, 2021

iwankgb Mar 4, 2021

Creatone Mar 15, 2021

ppalucki Aug 12, 2021

ppalucki Aug 12, 2021

Creatone Aug 19, 2021

iwankgb commented Mar 7, 2021

Creatone commented Mar 15, 2021

Creatone commented Mar 15, 2021

Creatone commented Mar 17, 2021

Creatone commented Mar 17, 2021

Creatone commented Mar 18, 2021

iwankgb left a comment

Creatone commented Aug 16, 2021 •

edited

Loading

Creatone commented Aug 20, 2021

ppalucki Aug 24, 2021

Creatone Sep 9, 2021

ppalucki Aug 24, 2021

Creatone Sep 9, 2021

Creatone Sep 9, 2021

Creatone commented Sep 9, 2021

iwankgb left a comment

iwankgb commented Sep 21, 2021

Creatone commented Sep 21, 2021

		@@ -77,6 +77,8 @@ var rawCgroupPrefixWhiteList = flag.String("raw_cgroup_prefix_whitelist", "", "A

		var perfEvents = flag.String("perf_events_config", "", "Path to a JSON file containing configuration of perf events to measure. Empty value disabled perf events measuring.")

		var resctrlInterval = flag.Duration("resctrl_interval", 0, "Resctrl mon groups updating interval. Zero value disables updating mon groups.")

Add mon_group support for resctrl. #2793

Add mon_group support for resctrl. #2793

Conversation

Creatone commented Jan 28, 2021

Creatone commented Jan 28, 2021

iwankgb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iwankgb commented Mar 7, 2021

Creatone commented Mar 15, 2021

Creatone commented Mar 15, 2021

Creatone commented Mar 17, 2021

Creatone commented Mar 17, 2021

Creatone commented Mar 18, 2021

iwankgb left a comment

Choose a reason for hiding this comment

Creatone commented Aug 16, 2021 • edited Loading

Creatone commented Aug 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creatone commented Sep 9, 2021

iwankgb left a comment

Choose a reason for hiding this comment

iwankgb commented Sep 21, 2021

Creatone commented Sep 21, 2021

Creatone commented Aug 16, 2021 •

edited

Loading