ext cpu throttling scenarios #2581

nagworld9 · 2022-05-04T01:11:45Z

Description

Cgroup disable on following cases:-

Agent throttled - reset agent cpu quota.

Extra process in Agent cgroup - Disable All (includes extension and its services). reset quotas and reset monitors.

Systemd-run failure: disable all, reset quotas and reset monitors.

Also have changes for logging throttled time on extension timeout case.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
If applicable, the PR references the bug/issue that it fixes in the description.
New Unit tests were added for the changes made

Quality of Code and Contribution Guidelines

I have read the contribution guidelines.

codecov · 2022-05-04T01:15:52Z

Codecov Report

Merging #2581 (e174e27) into develop (e4ec4c9) will decrease coverage by 0.00%.
The diff coverage is 72.72%.

@@             Coverage Diff             @@
##           develop    #2581      +/-   ##
===========================================
- Coverage    71.96%   71.95%   -0.01%     
===========================================
  Files          102      102              
  Lines        15397    15460      +63     
  Branches      2446     2459      +13     
===========================================
+ Hits         11080    11125      +45     
- Misses        3824     3836      +12     
- Partials       493      499       +6

Impacted Files	Coverage Δ
azurelinuxagent/common/cgroupconfigurator.py	`73.52% <63.93%> (-0.72%)`	⬇️
...urelinuxagent/common/utils/extensionprocessutil.py	`96.96% <88.23%> (-3.04%)`	⬇️
azurelinuxagent/common/cgroup.py	`91.08% <100.00%> (+0.11%)`	⬆️
azurelinuxagent/common/cgroupapi.py	`81.60% <100.00%> (+0.21%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4ec4c9...e174e27. Read the comment docs.

nagworld9 · 2022-05-04T01:14:22Z

azurelinuxagent/common/cgroupconfigurator.py

@@ -873,6 +913,31 @@ def start_tracking_extension_services_cgroups(self, services_list):
                    if service_name is not None:
                        self.start_tracking_unit_cgroups(service_name)

+        @staticmethod
+        def get_extension_services_list():


retrieve the manifest files of installed extensions to reset quotas when we disable cgroups for all.

nagworld9 · 2022-05-04T01:15:48Z

azurelinuxagent/common/utils/extensionprocessutil.py

    process_output = read_output(stdout, stderr)

    if timed_out:
+        if cpu_cgroup is not None:# Report CPUThrottledTime when timeout happens


Report the Throttled time in the exception msg when timeout case.

instead of passing the cpu_cgroup as parameter down the call stack, can we handle the exception in the cgroup configurator and report the throttle time there? (you can create a new exception ExtensionTimeout derived from ExtensionError)

yes, I would like to do that way but when timeout happens, we kill the process before returns ExtensionTimeout exception. That would remove the cgroups so I had to pass cpu_cgroup down the lane to compute before kill.

nagworld9 · 2022-05-04T01:20:11Z

azurelinuxagent/common/cgroupconfigurator.py

            elif disable_cgroups == DisableCgroups.AGENT:  # disable agent
                self._agent_cgroups_enabled = False
                self.__reset_agent_cpu_quota()
                CGroupsTelemetry.stop_tracking(CpuCgroup(AGENT_NAME_TELEMETRY, self._agent_cpu_cgroup_path))
-            elif disable_cgroups == DisableCgroups.EXTENSIONS:  # disable extensions
-                self._extensions_cgroups_enabled = False


not needed as we don't disable particular extension

nagworld9 · 2022-05-04T01:30:45Z

azurelinuxagent/common/cgroupconfigurator.py

@@ -519,7 +529,6 @@ def __reset_agent_cpu_quota():
            """
            logger.info("Resetting agent's CPUQuota")
            if CGroupConfigurator._Impl.__try_set_cpu_quota(''):  # setting an empty value resets to the default (infinity)
-                CGroupsTelemetry.set_track_throttled_time(False)


Not needed as we are already stop tracking the cgroup and also this would set to false for extensions too which we don't want to do it on agent throttling case

narrieta · 2022-05-10T21:24:34Z

azurelinuxagent/common/cgroup.py

@@ -138,6 +139,7 @@ def __init__(self, name, cgroup_path):
        self._current_system_cpu = None
        self._previous_throttled_time = None
        self._current_throttled_time = None
+        self._throttled_time_lock = threading.RLock()  # Protect the get_throttled_time which is called from Monitor thread and cgroupconfigurator.


it doesn't seem like we need to protect concurrent access to this getter, do we?

I think we need in the case of long running extensions which are running more than 5mins or so. In those cases, monitor thread and update handler will have race condition.

narrieta · 2022-05-10T23:36:49Z

azurelinuxagent/common/utils/extensionprocessutil.py

    process_output = read_output(stdout, stderr)

    if timed_out:
+        if cpu_cgroup is not None:# Report CPUThrottledTime when timeout happens


instead of passing the cpu_cgroup as parameter down the call stack, can we handle the exception in the cgroup configurator and report the throttle time there? (you can create a new exception ExtensionTimeout derived from ExtensionError)

narrieta · 2022-05-10T23:38:35Z

azurelinuxagent/common/utils/extensionprocessutil.py

+    throttled_time = 0
+    if cpu_cgroup is not None:
+        try:
+            throttled_time = float(cpu_cgroup.get_throttled_time() / 1E9)


we do this conversion in a couple of places; we should abstract it in the CpuCgroup class

azurelinuxagent/common/cgroupconfigurator.py

narrieta · 2022-05-11T18:20:40Z

azurelinuxagent/common/cgroupconfigurator.py

+                                files_to_create.append((drop_in_file_cpu_quota, cpu_quota_contents))
+                            self.__create_all_files(files_to_create)
+            except Exception as exception:
+                _log_cgroup_warning('Failed to set CPUQuota: {0}', ustr(exception))


let's add the extension/service name to the error message

narrieta · 2022-05-11T18:23:20Z

azurelinuxagent/common/cgroupconfigurator.py

+            read the extension services if ResourceLimits are present.
+            """
+            extensions_services = {}
+            for manifest_path in glob.iglob(os.path.join(conf.get_lib_dir(), "*/HandlerManifest.json")):


can we abstract the extension traversal in ExtensionsHandlerHandler? that class already has some code to do this kind of stuff, and takes into account, for example, extensions that failed to install (you probably want to skip those)

The challenge was getting around cyclic dependency if I have to move this to extensionhandler

tests/common/test_cgroupconfigurator.py

…o TASK-14179340

ext cpu throttling scenarios

3ed3a12

nagworld9 requested review from narrieta, larohra, kevinclark19a, ZhidongPeng and dhivyaganesan as code owners May 4, 2022 01:11

nagworld9 commented May 4, 2022

View reviewed changes

Merge branch 'develop' into TASK-14179340

8044329

narrieta reviewed May 11, 2022

View reviewed changes

nagworld9 and others added 4 commits May 12, 2022 18:13

address comments

8e20135

Merge branch 'TASK-14179340' of github.com:nagworld9/WALinuxAgent int…

63d63e2

…o TASK-14179340

remove unused import

ee69499

Merge branch 'develop' into TASK-14179340

9adc7be

narrieta approved these changes May 16, 2022

View reviewed changes

kevinclark19a approved these changes May 16, 2022

View reviewed changes

Merge branch 'develop' into TASK-14179340

e174e27

nagworld9 merged commit aa6d82c into Azure:develop May 17, 2022

nagworld9 deleted the TASK-14179340 branch May 17, 2022 19:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ext cpu throttling scenarios #2581

ext cpu throttling scenarios #2581

nagworld9 commented May 4, 2022

codecov bot commented May 4, 2022 •

edited

Loading

nagworld9 May 4, 2022

nagworld9 May 4, 2022

narrieta May 10, 2022

nagworld9 May 12, 2022

nagworld9 May 4, 2022

nagworld9 May 4, 2022

narrieta May 10, 2022

nagworld9 May 12, 2022

narrieta May 10, 2022

narrieta May 10, 2022

nagworld9 May 13, 2022

narrieta May 11, 2022

nagworld9 May 13, 2022

narrieta May 11, 2022

nagworld9 May 12, 2022

ext cpu throttling scenarios #2581

ext cpu throttling scenarios #2581

Conversation

nagworld9 commented May 4, 2022

Description

PR information

Quality of Code and Contribution Guidelines

codecov bot commented May 4, 2022 • edited Loading

Codecov Report

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

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

Choose a reason for hiding this comment

codecov bot commented May 4, 2022 •

edited

Loading