[SAMZA-2800] A new Control Group Metric for Samza #1699

li-afaris · 2024-06-03T19:30:30Z

Introduction

Hadoop clusters have the ability to restrict CPU usage for Samza applications by utilizing Control Groups, (Cgroups).
Before enabling CPU enforcement on Hadoop clusters, application owners must have a way of knowing when their application is being throttled by Cgroups. This PR will add a new Cgroup metric that makes application owners aware if containers CPU usage is being throttled by control groups & whether the application needs to request additional resources.

Implementation

The Linux kernel reports when applications within a Cgroup has been throttled by writing values to a file named cpu.stat. cpu.stat contains two fields named nr_periods & nr_throttled. nr_periods represents the number of enforcement periods that elapsed. nr_thorttled represents the number of times the group has been throttled. We can treat these fields as a ratio that shows the number of times applications has been throttled over a number of enforcement periods. The proposal is to have the running container locate the cpu.stat file by reading property values from Hadoop's YARN config.

Implementation details

To limit high cardinality in the metrics storage layer, instead of using the Hadoop YARN container id, the metric will emit the Samza container ID as the hostname, (ie: Container 3). This is already supported by the existing metrics framework within Samza.
The container will emit a float value between zero and 1 as a gauge metric. A zero value means the Cgroup was not throttled for that period of time. A value of 1 means the Cgroup was unable to complete any work as it was persistently throttled.
To stay consistent with existing metrics, a negative value (-1) will be emitted if an exception is thrown when reading the cpu.stat file. Exceptions when reading cpu.stat will be logged to the container logs.
This implementation will be specific to Samza on Hadoop. The reasoning is the application itself should emit this metric, not the embedded library.

Considered Alternatives

I’m unaware of alternatives but reading values from cpu.stat is a pattern which appears in the Runc project. Runc is the underlying library for ContainerD which is used by both Docker & Kubernetes.

The metric needs to be emitted from the Samza container itself. Using a system daemon or sidecar application complicates deployments & creates data consistency issues when the sidecar process isn’t running.

External references

Linux kernel documentation on the cpu.stat file
cpu.stat references from the Open Container Initiative runc project.

bringhurst · 2024-06-21T19:41:43Z

gradle/dependency-versions.gradle

@@ -47,7 +47,8 @@
  scalaTestVersion = "3.0.1"
  snappyVersion = "1.1.8.4"
  slf4jVersion = "1.7.7"
-  yarnVersion = "2.10.1"


Is it possible to split up the Hadoop version bump (and the related hadoop zookeeper gradle exclude) to a separate PR? It feels like it may be significant enough to stand alone.

Updated in the next push

bringhurst · 2024-06-21T19:45:43Z

samza-core/src/main/java/org/apache/samza/container/host/LinuxCgroupStatistics.java

+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    org.apache.samza.container.host.LinuxCgroupStatistics that = (org.apache.samza.container.host.LinuxCgroupStatistics) o;


Does this need to be fully qualified here?

Updated in the next push

bringhurst · 2024-06-21T19:46:04Z

samza-core/src/main/java/org/apache/samza/container/host/LinuxCgroupStatisticsGetter.java

+ * under the License.
+ */
+
+package org.apache.samza.container.host;


Line break after package please.

Updated in the next push

bringhurst · 2024-06-21T19:51:16Z

samza-core/src/main/java/org/apache/samza/container/host/LinuxCgroupStatisticsGetter.java

+  }
+
+  private double getCPUStat() {
+    if (this.containerID.equals("NOT_DETECTED")) {


Where is NOT_DETECTED defined? Should this be a new const?

Updated in the next push

bringhurst · 2024-06-21T19:52:50Z

samza-core/src/main/java/org/apache/samza/container/host/LinuxCgroupStatisticsGetter.java

+      // return a sentinel value to signal this is not running on Hadoop
+      return -2.0;
+    }
+    String[] controllers = {"cpu", "cpuacct", "cpu,cpuacct" };


The order isn't guaranteed here. Please add "cpuacct,cpu".

Good catch. I didn't think about directory name ordering. Updated in the next push

bringhurst · 2024-06-21T19:55:57Z

samza-core/src/test/java/org/apache/samza/container/host/TestLinuxCgroupStatistics.java

+package org.apache.samza.container.host;
+
+import org.junit.Test;
+import static org.junit.Assert.*;


Please don't wildcard import here.

I need to figure out the IDE setting that does this. Updated in the next push.

bringhurst · 2024-06-21T19:56:12Z

samza-core/src/test/java/org/apache/samza/container/host/TestLinuxCgroupStatisticsGetter.java

+package org.apache.samza.container.host;
+
+
+import static org.junit.Assert.*;


Please don't wildcard import here.

I need to figure out the IDE setting that does this. Updated in the next push.

li-afaris · 2024-07-12T16:38:55Z

I'm not happy with this approach after reviewing the changes. As these are YARN metrics, they should be part of the samza-yarn subproject and not part of samza-core. I'm going to close this PR and open a new one to simplify the change list.

li-afaris added 6 commits May 31, 2024 13:48

New metric stubs

206c4b1

fix tests

abf4393

Nightly checkin

fbb33e8

Add unit tests

e9ebd8a

New test for LinuxCgroupStatistics

53f5f6e

Updated exception handling

da25a21

li-afaris marked this pull request as ready for review June 7, 2024 00:12

li-afaris changed the title ~~A new Control Group Metric for Samza~~ [SAMZA-2800] A new Control Group Metric for Samza Jun 8, 2024

bringhurst suggested changes Jun 21, 2024

View reviewed changes

li-afaris closed this Jul 12, 2024

li-afaris deleted the cgroup_metrics branch July 12, 2024 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SAMZA-2800] A new Control Group Metric for Samza #1699

[SAMZA-2800] A new Control Group Metric for Samza #1699

li-afaris commented Jun 3, 2024 •

edited

Loading

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

bringhurst Jun 21, 2024

li-afaris Jun 26, 2024

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

bringhurst Jun 21, 2024

li-afaris Jun 25, 2024

li-afaris commented Jul 12, 2024

		package org.apache.samza.container.host;


		import static org.junit.Assert.*;

[SAMZA-2800] A new Control Group Metric for Samza #1699

[SAMZA-2800] A new Control Group Metric for Samza #1699

Conversation

li-afaris commented Jun 3, 2024 • edited Loading

Introduction

Implementation

Implementation details

Considered Alternatives

External references

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

li-afaris commented Jul 12, 2024

li-afaris commented Jun 3, 2024 •

edited

Loading