[ 
https://issues.apache.org/jira/browse/YARN-11672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17838167#comment-17838167
 ] 

ASF GitHub Bot commented on YARN-11672:
---------------------------------------

tomicooler commented on code in PR #6734:
URL: https://github.com/apache/hadoop/pull/6734#discussion_r1568394772


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Support for interacting with various CGroup v2 subsystems. Thread-safe.
+ */
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+class CGroupsV2HandlerImpl extends AbstractCGroupsHandler {
+  private static final Logger LOG =
+          LoggerFactory.getLogger(CGroupsV2HandlerImpl.class);
+
+  private static final String CGROUP2_FSTYPE = "cgroup2";
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @param mtab mount file location
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor, String mtab)
+          throws ResourceHandlerException {
+    super(conf, privilegedOperationExecutor, mtab);
+  }
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor) throws ResourceHandlerException {
+    this(conf, privilegedOperationExecutor, MTAB_FILE);
+  }
+
+  @Override
+  protected Map<String, Set<String>> parsePreConfiguredMountPath() throws 
IOException {
+    Map<String, Set<String>> controllerMappings = new HashMap<>();
+    String controllerPath = this.cGroupsMountConfig.getMountPath() + 
Path.SEPARATOR + this.cGroupPrefix;
+    controllerMappings.put(this.cGroupsMountConfig.getMountPath(), 
parseControllersFile(controllerPath));
+    return controllerMappings;
+  }
+
+  @Override
+  protected Set<String> handleMtabEntry(String path, String type, String 
options) throws IOException {
+    if (type.equals(CGROUP2_FSTYPE)) {
+      return parseControllersFile(path);
+    }
+
+    return null;
+  }
+
+  @Override
+  protected void mountCGroupController(CGroupController controller) {
+    throw new UnsupportedOperationException("Mounting cgroup controllers is 
not supported in cgroup v2");
+  }
+
+  /**
+   * Parse the cgroup v2 controllers file to check the enabled controllers.
+   * @param cgroupPath path to the cgroup directory
+   * @return set of enabled and YARN supported controllers.
+   * @throws IOException if the file is not found or cannot be read
+   */
+  public Set<String> parseControllersFile(String cgroupPath) throws 
IOException {
+    File cgroupControllersFile = new File(cgroupPath + Path.SEPARATOR + 
CGROUP_CONTROLLERS_FILE);
+    if (!cgroupControllersFile.exists()) {
+      throw new IOException("No cgroup controllers file found in the directory 
specified: " +
+              cgroupPath);
+    }
+
+    String enabledControllers = 
FileUtils.readFileToString(cgroupControllersFile, StandardCharsets.UTF_8);
+    Set<String> validCGroups =
+            CGroupsHandler.CGroupController.getValidCGroups();
+    Set<String> controllerSet =
+            new HashSet<>(Arrays.asList(enabledControllers.split(" ")));
+    // Collect the valid subsystem names
+    controllerSet.retainAll(validCGroups);
+    if (controllerSet.isEmpty()) {
+      LOG.warn("The following cgroup directory doesn't contain any supported 
controllers: " +
+              cgroupPath);
+    }
+
+    return controllerSet;
+  }
+
+  /**
+   * Update the subtree_control file to enable subsequent container based 
cgroups to use the same controllers.
+   * @param yarnHierarchy path to the yarn cgroup under which the container 
cgroups will be created
+   * @throws ResourceHandlerException if the controllers file cannot be updated
+   */
+  @Override
+  protected void updateEnabledControllersInHierarchy(File yarnHierarchy) 
throws ResourceHandlerException {
+    PrintWriter pw = null;
+    try {
+      Set<String> enabledControllers = 
parseControllersFile(yarnHierarchy.getAbsolutePath());
+      if (enabledControllers.isEmpty()) {
+        throw new ResourceHandlerException("No valid controllers found in the 
cgroup hierarchy: " +
+                yarnHierarchy.getAbsolutePath());
+      }
+
+      File subtreeControlFile = new File(yarnHierarchy.getAbsolutePath()
+          + Path.SEPARATOR + CGROUP_SUBTREE_CONTROL_FILE);
+
+      Writer w = new 
OutputStreamWriter(Files.newOutputStream(subtreeControlFile.toPath()), 
StandardCharsets.UTF_8);
+      pw = new PrintWriter(w);
+      pw.write(String.join(" ", enabledControllers));
+
+    } catch (IOException e) {
+      throw new ResourceHandlerException("Failed to update the controllers 
file in the cgroup hierarchy: " +
+              yarnHierarchy.getAbsolutePath(), e);
+    } finally {
+      if (pw != null) {
+        pw.close();
+      }
+    }

Review Comment:
   ```suggestion
       try {
         Set<String> enabledControllers = 
parseControllersFile(yarnHierarchy.getAbsolutePath());
         if (enabledControllers.isEmpty()) {
           throw new ResourceHandlerException("No valid controllers found in 
the cgroup hierarchy: " +
                   yarnHierarchy.getAbsolutePath());
         }
   
         File subtreeControlFile = new File(yarnHierarchy.getAbsolutePath()
             + Path.SEPARATOR + CGROUP_SUBTREE_CONTROL_FILE);
   
         Writer w = new 
OutputStreamWriter(Files.newOutputStream(subtreeControlFile.toPath()), 
StandardCharsets.UTF_8);
         try (PrintWriter pw = new PrintWriter(w)) {
           pw.write(String.join(" ", enabledControllers));
         }
       } catch (IOException e) {
         throw new ResourceHandlerException("Failed to update the controllers 
file in the cgroup hierarchy: " +
                 yarnHierarchy.getAbsolutePath(), e);
       }
   ```
   
   try with resources simplifies this code



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsHandler.java:
##########
@@ -36,17 +36,23 @@
 public interface CGroupsHandler {
 
   /**
-   * List of supported cgroup subsystem types.
+   * List of supported cgroup v1 and v2 subsystem types.
    */
   enum CGroupController {
-    CPU("cpu"),
+    // v1 specific
     NET_CLS("net_cls"),
     BLKIO("blkio"),
-    MEMORY("memory"),
     CPUACCT("cpuacct"),
-    CPUSET("cpuset"),
     FREEZER("freezer"),
-    DEVICES("devices");
+    DEVICES("devices"),
+
+    // v2 specific

Review Comment:
   Extending this enum means that the `getValidCGroups` will return the union 
of v1 and v2 as valid values. This changes the existing behaviour (e.g. 
`parseConfiguredCGroupPath`). And it allows to have malformed subtree_control 
files in v2, etc.
   
   Shouldn't we keep valid cgroups separate for each implementation?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Support for interacting with various CGroup v2 subsystems. Thread-safe.
+ */
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+class CGroupsV2HandlerImpl extends AbstractCGroupsHandler {
+  private static final Logger LOG =
+          LoggerFactory.getLogger(CGroupsV2HandlerImpl.class);
+
+  private static final String CGROUP2_FSTYPE = "cgroup2";
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @param mtab mount file location
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor, String mtab)
+          throws ResourceHandlerException {
+    super(conf, privilegedOperationExecutor, mtab);
+  }
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor) throws ResourceHandlerException {
+    this(conf, privilegedOperationExecutor, MTAB_FILE);
+  }
+
+  @Override
+  protected Map<String, Set<String>> parsePreConfiguredMountPath() throws 
IOException {
+    Map<String, Set<String>> controllerMappings = new HashMap<>();
+    String controllerPath = this.cGroupsMountConfig.getMountPath() + 
Path.SEPARATOR + this.cGroupPrefix;
+    controllerMappings.put(this.cGroupsMountConfig.getMountPath(), 
readControllersFile(controllerPath));
+    return controllerMappings;
+  }
+
+  @Override
+  protected Set<String> handleMtabEntry(String path, String type, String 
options) throws IOException {
+    if (type.equals(CGROUP2_FSTYPE)) {
+      return readControllersFile(path);
+    }
+
+    return null;
+  }
+
+  @Override
+  protected void mountCGroupController(CGroupController controller) {
+    throw new UnsupportedOperationException("Mounting cgroup controllers is 
not supported in cgroup v2");
+  }
+
+  /**
+   * Parse the cgroup v2 controllers file to check the enabled controllers.
+   * @param cgroupPath path to the cgroup directory
+   * @return set of enabled and YARN supported controllers.
+   * @throws IOException if the file is not found or cannot be read
+   */
+  public Set<String> readControllersFile(String cgroupPath) throws IOException 
{
+    File cgroupControllersFile = new File(cgroupPath + Path.SEPARATOR + 
CGROUP_CONTROLLERS_FILE);
+    if (!cgroupControllersFile.exists()) {
+      throw new IOException("No cgroup controllers file found in the directory 
specified: " +
+              cgroupPath);
+    }
+
+    String enabledControllers = 
FileUtils.readFileToString(cgroupControllersFile, StandardCharsets.UTF_8);
+    Set<String> validCGroups =
+            CGroupsHandler.CGroupController.getValidCGroups();
+    Set<String> controllerSet =
+            new HashSet<>(Arrays.asList(enabledControllers.split(" ")));
+    // Collect the valid subsystem names
+    controllerSet.retainAll(validCGroups);
+    if (controllerSet.isEmpty()) {
+      LOG.warn("The following cgroup directory doesn't contain any supported 
controllers: " +
+              cgroupPath);
+    }
+
+    return controllerSet;
+  }
+
+  /**
+   * Update the subtree_control file to enable subsequent container based 
cgroups to use the same controllers.
+   * @param yarnHierarchy path to the yarn cgroup under which the container 
cgroups will be created
+   * @throws ResourceHandlerException if the controllers file cannot be updated
+   */
+  @Override
+  protected void updateEnabledControllersInHierarchy(File yarnHierarchy) 
throws ResourceHandlerException {
+    PrintWriter pw = null;
+    try {
+      Set<String> enabledControllers = 
readControllersFile(yarnHierarchy.getAbsolutePath());
+      if (enabledControllers.isEmpty()) {
+        throw new ResourceHandlerException("No valid controllers found in the 
cgroup hierarchy: " +
+                yarnHierarchy.getAbsolutePath());
+      }
+
+      File subtreeControlFile = new File(yarnHierarchy.getAbsolutePath()
+          + Path.SEPARATOR + CGROUP_SUBTREE_CONTROL_FILE);
+      if (!subtreeControlFile.exists()) {
+        throw new ResourceHandlerException("No subtree control file found in 
the cgroup hierarchy: " +
+                yarnHierarchy.getAbsolutePath());
+      }
+
+      String subtreeControllers = 
FileUtils.readFileToString(subtreeControlFile, StandardCharsets.UTF_8);
+      Set<String> subtreeControllerSet = new 
HashSet<>(Arrays.asList(subtreeControllers.split(" ")));
+      
subtreeControllerSet.retainAll(CGroupsHandler.CGroupController.getValidCGroups());
+
+      if (subtreeControllerSet.containsAll(enabledControllers)) {
+        return;
+      }
+      Writer w = new 
OutputStreamWriter(Files.newOutputStream(subtreeControlFile.toPath()), 
StandardCharsets.UTF_8);
+      pw = new PrintWriter(w);

Review Comment:
   If we are OK with the overwrite, at least log a warning about it.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Tests for the CGroups handler implementation.
+ */
+public class TestCGroupsV2HandlerImpl extends TestCGroupsHandlerBase {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestCGroupsV2HandlerImpl.class);
+
+  // Create a controller file in the unified hierarchy of cgroup v2
+  @Override
+  protected String getControllerFilePath(String controllerName) {
+    return new File(tmpPath, hierarchy).getAbsolutePath();
+  }
+
+  public File createPremountedCgroups(File parentDir)
+          throws IOException {
+    // Format:
+    // cgroup2 /sys/fs/cgroup cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0
+    String baseCgroup2Line =
+            "cgroup2 " + parentDir.getAbsolutePath()
+                    + " cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0\n";
+
+    File mockMtab = new File(parentDir, UUID.randomUUID().toString());
+    if (!mockMtab.exists()) {
+      if (!mockMtab.createNewFile()) {
+        String message = "Could not create file " + mockMtab.getAbsolutePath();
+        throw new IOException(message);
+      }
+    }
+    FileWriter mtabWriter = new FileWriter(mockMtab.getAbsoluteFile());
+    mtabWriter.write(baseCgroup2Line);
+    mtabWriter.close();
+    mockMtab.deleteOnExit();
+
+    String enabledControllers = "cpuset cpu io memory hugetlb pids rdma 
misc\n";
+
+    File controllersFile = new File(parentDir, 
CGroupsHandler.CGROUP_CONTROLLERS_FILE);
+    FileWriter controllerWriter = new 
FileWriter(controllersFile.getAbsoluteFile());
+    controllerWriter.write(enabledControllers);
+    controllerWriter.close();
+    controllersFile.deleteOnExit();
+
+    File subtreeControlFile = new File(parentDir, 
CGroupsHandler.CGROUP_SUBTREE_CONTROL_FILE);
+    Assert.assertTrue("empty subtree_control file should be created", 
subtreeControlFile.createNewFile());
+
+    File hierarchyDir = new File(parentDir, hierarchy);
+    if (!hierarchyDir.mkdirs()) {
+      String message = "Could not create directory " + 
hierarchyDir.getAbsolutePath();
+      throw new IOException(message);
+    }
+    hierarchyDir.deleteOnExit();
+    FileUtils.copyFile(controllersFile, new File(hierarchyDir, 
CGroupsHandler.CGROUP_CONTROLLERS_FILE));
+    FileUtils.copyFile(subtreeControlFile, new File(hierarchyDir, 
CGroupsHandler.CGROUP_SUBTREE_CONTROL_FILE));
+
+    return mockMtab;
+  }
+
+  @Test
+  public void testCGroupPaths() throws IOException {
+    verifyZeroInteractions(privilegedOperationExecutorMock);
+    CGroupsHandler cGroupsHandler = null;
+    File parentDir = new File(tmpPath);
+    File mtab = createPremountedCgroups(parentDir);
+    assertTrue("Sample subsystem should be created",
+        new File(controllerPath).exists());
+
+    try {
+      cGroupsHandler = new 
CGroupsV2HandlerImpl(createNoMountConfiguration(hierarchy),
+              privilegedOperationExecutorMock, mtab.getAbsolutePath());
+      cGroupsHandler.initializeCGroupController(controller);
+    } catch (ResourceHandlerException e) {
+      LOG.error("Caught exception: " + e);
+      fail("Unexpected ResourceHandlerException when initializing 
controller!");
+    }
+
+    String testCGroup = "container_01";
+    String expectedPath =
+        controllerPath + Path.SEPARATOR + testCGroup;
+    String path = cGroupsHandler.getPathForCGroup(controller, testCGroup);
+    Assert.assertEquals(expectedPath, path);
+
+    String expectedPathTasks = expectedPath + Path.SEPARATOR
+        + CGroupsHandler.CGROUP_PROCS_FILE;
+    path = cGroupsHandler.getPathForCGroupTasks(controller, testCGroup);
+    Assert.assertEquals(expectedPathTasks, path);
+
+    String param = CGroupsHandler.CGROUP_PARAM_CLASSID;
+    String expectedPathParam = expectedPath + Path.SEPARATOR
+        + controller.getName() + "." + param;
+    path = cGroupsHandler.getPathForCGroupParam(controller, testCGroup, param);
+    Assert.assertEquals(expectedPathParam, path);
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testUnsupportedMountConfiguration() throws Exception {
+    //As per junit behavior, we expect a new mock object to be available
+    //in this test.
+    verifyZeroInteractions(privilegedOperationExecutorMock);
+    CGroupsHandler cGroupsHandler;
+    File mtab = createEmptyCgroups();
+
+    assertTrue("Sample subsystem should be created",
+            new File(controllerPath).mkdirs());
+
+    cGroupsHandler = new CGroupsV2HandlerImpl(createMountConfiguration(),
+            privilegedOperationExecutorMock, mtab.getAbsolutePath());
+    cGroupsHandler.initializeCGroupController(controller);
+    }
+
+  @Test
+  public void testCGroupOperations() throws IOException {
+    verifyZeroInteractions(privilegedOperationExecutorMock);
+    CGroupsHandler cGroupsHandler = null;
+    File parentDir = new File(tmpPath);
+    File mtab = createPremountedCgroups(parentDir);
+    assertTrue("Sample subsystem should be created",
+            new File(controllerPath).exists());
+
+    try {
+      cGroupsHandler = new 
CGroupsV2HandlerImpl(createNoMountConfiguration(hierarchy),
+          privilegedOperationExecutorMock, mtab.getAbsolutePath());
+      cGroupsHandler.initializeCGroupController(controller);
+    } catch (ResourceHandlerException e) {

Review Comment:
   Same as above, no need to catch.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Support for interacting with various CGroup v2 subsystems. Thread-safe.
+ */
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+class CGroupsV2HandlerImpl extends AbstractCGroupsHandler {
+  private static final Logger LOG =
+          LoggerFactory.getLogger(CGroupsV2HandlerImpl.class);
+
+  private static final String CGROUP2_FSTYPE = "cgroup2";
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @param mtab mount file location
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor, String mtab)
+          throws ResourceHandlerException {
+    super(conf, privilegedOperationExecutor, mtab);
+  }
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor) throws ResourceHandlerException {
+    this(conf, privilegedOperationExecutor, MTAB_FILE);
+  }
+
+  @Override
+  protected Map<String, Set<String>> parsePreConfiguredMountPath() throws 
IOException {
+    Map<String, Set<String>> controllerMappings = new HashMap<>();
+    String controllerPath = this.cGroupsMountConfig.getMountPath() + 
Path.SEPARATOR + this.cGroupPrefix;
+    controllerMappings.put(this.cGroupsMountConfig.getMountPath(), 
parseControllersFile(controllerPath));
+    return controllerMappings;
+  }
+
+  @Override
+  protected Set<String> handleMtabEntry(String path, String type, String 
options) throws IOException {
+    if (type.equals(CGROUP2_FSTYPE)) {
+      return parseControllersFile(path);
+    }
+
+    return null;
+  }
+
+  @Override
+  protected void mountCGroupController(CGroupController controller) {
+    throw new UnsupportedOperationException("Mounting cgroup controllers is 
not supported in cgroup v2");
+  }
+
+  /**
+   * Parse the cgroup v2 controllers file to check the enabled controllers.

Review Comment:
   ```suggestion
      * Parse the cgroup v2 controllers file (cgroup.controllers) to check the 
enabled controllers.
   ```
   
   It's easier to google the file syntax. Maybe we could also add an example in 
the javadoc to help understand the parsing.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Tests for the CGroups handler implementation.
+ */
+public class TestCGroupsV2HandlerImpl extends TestCGroupsHandlerBase {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestCGroupsV2HandlerImpl.class);
+
+  // Create a controller file in the unified hierarchy of cgroup v2
+  @Override
+  protected String getControllerFilePath(String controllerName) {
+    return new File(tmpPath, hierarchy).getAbsolutePath();
+  }
+
+  public File createPremountedCgroups(File parentDir)

Review Comment:
   A javadoc with an ascii output (e.g. with `tree`) would be really useful.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Support for interacting with various CGroup v2 subsystems. Thread-safe.
+ */
+
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+class CGroupsV2HandlerImpl extends AbstractCGroupsHandler {
+  private static final Logger LOG =
+          LoggerFactory.getLogger(CGroupsV2HandlerImpl.class);
+
+  private static final String CGROUP2_FSTYPE = "cgroup2";
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @param mtab mount file location
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor, String mtab)
+          throws ResourceHandlerException {
+    super(conf, privilegedOperationExecutor, mtab);
+  }
+
+  /**
+   * Create cgroup v2 handler object.
+   * @param conf configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @throws ResourceHandlerException if initialization failed
+   */
+  CGroupsV2HandlerImpl(Configuration conf, PrivilegedOperationExecutor
+          privilegedOperationExecutor) throws ResourceHandlerException {
+    this(conf, privilegedOperationExecutor, MTAB_FILE);
+  }
+
+  @Override
+  protected Map<String, Set<String>> parsePreConfiguredMountPath() throws 
IOException {
+    Map<String, Set<String>> controllerMappings = new HashMap<>();
+    String controllerPath = this.cGroupsMountConfig.getMountPath() + 
Path.SEPARATOR + this.cGroupPrefix;
+    controllerMappings.put(this.cGroupsMountConfig.getMountPath(), 
readControllersFile(controllerPath));
+    return controllerMappings;
+  }
+
+  @Override
+  protected Set<String> handleMtabEntry(String path, String type, String 
options) throws IOException {
+    if (type.equals(CGROUP2_FSTYPE)) {
+      return readControllersFile(path);
+    }
+
+    return null;
+  }
+
+  @Override
+  protected void mountCGroupController(CGroupController controller) {
+    throw new UnsupportedOperationException("Mounting cgroup controllers is 
not supported in cgroup v2");
+  }
+
+  /**
+   * Parse the cgroup v2 controllers file to check the enabled controllers.
+   * @param cgroupPath path to the cgroup directory
+   * @return set of enabled and YARN supported controllers.
+   * @throws IOException if the file is not found or cannot be read
+   */
+  public Set<String> readControllersFile(String cgroupPath) throws IOException 
{
+    File cgroupControllersFile = new File(cgroupPath + Path.SEPARATOR + 
CGROUP_CONTROLLERS_FILE);
+    if (!cgroupControllersFile.exists()) {
+      throw new IOException("No cgroup controllers file found in the directory 
specified: " +
+              cgroupPath);
+    }
+
+    String enabledControllers = 
FileUtils.readFileToString(cgroupControllersFile, StandardCharsets.UTF_8);
+    Set<String> validCGroups =
+            CGroupsHandler.CGroupController.getValidCGroups();
+    Set<String> controllerSet =
+            new HashSet<>(Arrays.asList(enabledControllers.split(" ")));
+    // Collect the valid subsystem names
+    controllerSet.retainAll(validCGroups);
+    if (controllerSet.isEmpty()) {
+      LOG.warn("The following cgroup directory doesn't contain any supported 
controllers: " +
+              cgroupPath);
+    }
+
+    return controllerSet;
+  }
+
+  /**
+   * Update the subtree_control file to enable subsequent container based 
cgroups to use the same controllers.
+   * @param yarnHierarchy path to the yarn cgroup under which the container 
cgroups will be created
+   * @throws ResourceHandlerException if the controllers file cannot be updated
+   */
+  @Override
+  protected void updateEnabledControllersInHierarchy(File yarnHierarchy) 
throws ResourceHandlerException {
+    PrintWriter pw = null;
+    try {
+      Set<String> enabledControllers = 
readControllersFile(yarnHierarchy.getAbsolutePath());
+      if (enabledControllers.isEmpty()) {
+        throw new ResourceHandlerException("No valid controllers found in the 
cgroup hierarchy: " +
+                yarnHierarchy.getAbsolutePath());
+      }
+
+      File subtreeControlFile = new File(yarnHierarchy.getAbsolutePath()
+          + Path.SEPARATOR + CGROUP_SUBTREE_CONTROL_FILE);
+      if (!subtreeControlFile.exists()) {
+        throw new ResourceHandlerException("No subtree control file found in 
the cgroup hierarchy: " +
+                yarnHierarchy.getAbsolutePath());
+      }
+
+      String subtreeControllers = 
FileUtils.readFileToString(subtreeControlFile, StandardCharsets.UTF_8);
+      Set<String> subtreeControllerSet = new 
HashSet<>(Arrays.asList(subtreeControllers.split(" ")));
+      
subtreeControllerSet.retainAll(CGroupsHandler.CGroupController.getValidCGroups());
+
+      if (subtreeControllerSet.containsAll(enabledControllers)) {
+        return;
+      }
+      Writer w = new 
OutputStreamWriter(Files.newOutputStream(subtreeControlFile.toPath()), 
StandardCharsets.UTF_8);
+      pw = new PrintWriter(w);
+      pw.write(String.join(" ", enabledControllers));
+
+    } catch (IOException e) {
+      throw new ResourceHandlerException("Failed to update the controllers 
file in the cgroup hierarchy: " +
+              yarnHierarchy.getAbsolutePath(), e);
+    } finally {
+      if (pw != null) {
+        pw.close();
+      }
+    }

Review Comment:
   ```suggestion
       try {
         Set<String> enabledControllers = 
readControllersFile(yarnHierarchy.getAbsolutePath());
         if (enabledControllers.isEmpty()) {
           throw new ResourceHandlerException("No valid controllers found in 
the cgroup hierarchy: " +
                   yarnHierarchy.getAbsolutePath());
         }
   
         File subtreeControlFile = new File(yarnHierarchy.getAbsolutePath()
             + Path.SEPARATOR + CGROUP_SUBTREE_CONTROL_FILE);
         if (!subtreeControlFile.exists()) {
           throw new ResourceHandlerException("No subtree control file found in 
the cgroup hierarchy: " +
                   yarnHierarchy.getAbsolutePath());
         }
   
         String subtreeControllers = 
FileUtils.readFileToString(subtreeControlFile, StandardCharsets.UTF_8);
         Set<String> subtreeControllerSet = new 
HashSet<>(Arrays.asList(subtreeControllers.split(" ")));
         
subtreeControllerSet.retainAll(CGroupsHandler.CGroupController.getValidCGroups());
   
         if (subtreeControllerSet.containsAll(enabledControllers)) {
           return;
         }
         Writer w = new 
OutputStreamWriter(Files.newOutputStream(subtreeControlFile.toPath()), 
StandardCharsets.UTF_8);
         try(PrintWriter pw = new PrintWriter(w)) {
           pw.write(String.join(" ", enabledControllers));
         }
       } catch (IOException e) {
         throw new ResourceHandlerException("Failed to update the controllers 
file in the cgroup hierarchy: " +
                 yarnHierarchy.getAbsolutePath(), e);
       }
   ```
   
   try with resources simplifies this code a little bit



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Tests for the CGroups handler implementation.
+ */
+public class TestCGroupsV2HandlerImpl extends TestCGroupsHandlerBase {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestCGroupsV2HandlerImpl.class);
+
+  // Create a controller file in the unified hierarchy of cgroup v2
+  @Override
+  protected String getControllerFilePath(String controllerName) {
+    return new File(tmpPath, hierarchy).getAbsolutePath();
+  }
+
+  public File createPremountedCgroups(File parentDir)
+          throws IOException {
+    // Format:
+    // cgroup2 /sys/fs/cgroup cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0
+    String baseCgroup2Line =
+            "cgroup2 " + parentDir.getAbsolutePath()
+                    + " cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0\n";
+
+    File mockMtab = new File(parentDir, UUID.randomUUID().toString());
+    if (!mockMtab.exists()) {
+      if (!mockMtab.createNewFile()) {
+        String message = "Could not create file " + mockMtab.getAbsolutePath();
+        throw new IOException(message);
+      }
+    }
+    FileWriter mtabWriter = new FileWriter(mockMtab.getAbsoluteFile());
+    mtabWriter.write(baseCgroup2Line);
+    mtabWriter.close();
+    mockMtab.deleteOnExit();
+
+    String enabledControllers = "cpuset cpu io memory hugetlb pids rdma 
misc\n";
+
+    File controllersFile = new File(parentDir, 
CGroupsHandler.CGROUP_CONTROLLERS_FILE);
+    FileWriter controllerWriter = new 
FileWriter(controllersFile.getAbsoluteFile());
+    controllerWriter.write(enabledControllers);
+    controllerWriter.close();
+    controllersFile.deleteOnExit();
+
+    File subtreeControlFile = new File(parentDir, 
CGroupsHandler.CGROUP_SUBTREE_CONTROL_FILE);
+    Assert.assertTrue("empty subtree_control file should be created", 
subtreeControlFile.createNewFile());
+
+    File hierarchyDir = new File(parentDir, hierarchy);
+    if (!hierarchyDir.mkdirs()) {
+      String message = "Could not create directory " + 
hierarchyDir.getAbsolutePath();
+      throw new IOException(message);
+    }
+    hierarchyDir.deleteOnExit();
+    FileUtils.copyFile(controllersFile, new File(hierarchyDir, 
CGroupsHandler.CGROUP_CONTROLLERS_FILE));
+    FileUtils.copyFile(subtreeControlFile, new File(hierarchyDir, 
CGroupsHandler.CGROUP_SUBTREE_CONTROL_FILE));
+
+    return mockMtab;
+  }
+
+  @Test
+  public void testCGroupPaths() throws IOException {
+    verifyZeroInteractions(privilegedOperationExecutorMock);
+    CGroupsHandler cGroupsHandler = null;
+    File parentDir = new File(tmpPath);
+    File mtab = createPremountedCgroups(parentDir);
+    assertTrue("Sample subsystem should be created",
+        new File(controllerPath).exists());
+
+    try {
+      cGroupsHandler = new 
CGroupsV2HandlerImpl(createNoMountConfiguration(hierarchy),
+              privilegedOperationExecutorMock, mtab.getAbsolutePath());
+      cGroupsHandler.initializeCGroupController(controller);
+    } catch (ResourceHandlerException e) {

Review Comment:
   No need to catch the exception, let it throw and the we will see the whole 
stactrace, better for debug and less code.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/TestCGroupsV2HandlerImpl.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Tests for the CGroups handler implementation.
+ */
+public class TestCGroupsV2HandlerImpl extends TestCGroupsHandlerBase {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestCGroupsV2HandlerImpl.class);
+
+  // Create a controller file in the unified hierarchy of cgroup v2
+  @Override
+  protected String getControllerFilePath(String controllerName) {
+    return new File(tmpPath, hierarchy).getAbsolutePath();
+  }
+
+  public File createPremountedCgroups(File parentDir)
+          throws IOException {
+    // Format:
+    // cgroup2 /sys/fs/cgroup cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0
+    String baseCgroup2Line =
+            "cgroup2 " + parentDir.getAbsolutePath()
+                    + " cgroup2 
rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0\n";
+
+    File mockMtab = new File(parentDir, UUID.randomUUID().toString());
+    if (!mockMtab.exists()) {
+      if (!mockMtab.createNewFile()) {
+        String message = "Could not create file " + mockMtab.getAbsolutePath();
+        throw new IOException(message);
+      }
+    }
+    FileWriter mtabWriter = new FileWriter(mockMtab.getAbsoluteFile());

Review Comment:
   Create a helper function that creates the file with the name, writes the 
text out and deleteOnExit(), it can be reused for this and the controllersFile.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/AbstractCGroupsHandler.java:
##########
@@ -0,0 +1,567 @@
+/*
+ * *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements. See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership. The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License. You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ * /
+ */
+
+package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileUtil;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.io.IOUtils;
+import org.apache.hadoop.util.Shell;
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor;
+import org.apache.hadoop.yarn.util.Clock;
+import org.apache.hadoop.yarn.util.SystemClock;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public abstract class AbstractCGroupsHandler implements CGroupsHandler {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(AbstractCGroupsHandler.class);
+  protected static final String MTAB_FILE = "/proc/mounts";
+
+  private final long deleteCGroupTimeout;
+  private final long deleteCGroupDelay;
+  private final Clock clock;
+
+  protected final String mtabFile;
+  protected final CGroupsMountConfig cGroupsMountConfig;
+  protected final ReadWriteLock rwLock;
+  protected Map<CGroupController, String> controllerPaths;
+  protected Map<String, Set<String>> parsedMtab;
+  protected final PrivilegedOperationExecutor privilegedOperationExecutor;
+  protected final String cGroupPrefix;
+
+  /**
+   * Create cgroup handler object.
+   *
+   * @param conf                        configuration
+   * @param privilegedOperationExecutor provides mechanisms to execute
+   *                                    PrivilegedContainerOperations
+   * @param mtab                        mount file location
+   * @throws ResourceHandlerException if initialization failed
+   */
+  AbstractCGroupsHandler(Configuration conf, PrivilegedOperationExecutor
+      privilegedOperationExecutor, String mtab)
+      throws ResourceHandlerException {
+    // Remove leading and trialing slash(es)
+    this.cGroupPrefix = conf.get(YarnConfiguration.
+            NM_LINUX_CONTAINER_CGROUPS_HIERARCHY, "/hadoop-yarn")
+        .replaceAll("^/+", "").replaceAll("/+$", "");
+    this.cGroupsMountConfig = new CGroupsMountConfig(conf);
+    this.deleteCGroupTimeout = conf.getLong(
+        YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_DELETE_TIMEOUT,
+        YarnConfiguration.DEFAULT_NM_LINUX_CONTAINER_CGROUPS_DELETE_TIMEOUT) +
+        conf.getLong(YarnConfiguration.NM_SLEEP_DELAY_BEFORE_SIGKILL_MS,
+            YarnConfiguration.DEFAULT_NM_SLEEP_DELAY_BEFORE_SIGKILL_MS) + 1000;
+    this.deleteCGroupDelay =
+        conf.getLong(YarnConfiguration.NM_LINUX_CONTAINER_CGROUPS_DELETE_DELAY,
+            YarnConfiguration.DEFAULT_NM_LINUX_CONTAINER_CGROUPS_DELETE_DELAY);
+    this.controllerPaths = new HashMap<>();
+    this.parsedMtab = new HashMap<>();
+    this.rwLock = new ReentrantReadWriteLock();
+    this.privilegedOperationExecutor = privilegedOperationExecutor;
+    this.clock = SystemClock.getInstance();
+    mtabFile = mtab;
+    init();
+  }
+
+  protected void init() throws ResourceHandlerException {
+    initializeControllerPaths();
+  }
+
+  @Override
+  public String getControllerPath(CGroupController controller) {
+    rwLock.readLock().lock();
+    try {
+      return controllerPaths.get(controller);
+    } finally {
+      rwLock.readLock().unlock();
+    }
+  }
+
+  private void initializeControllerPaths() throws ResourceHandlerException {
+    // Cluster admins may have some subsystems mounted in specific locations
+    // We'll attempt to figure out mount points. We do this even if we plan
+    // to mount cgroups into our own tree to control the path permissions or
+    // to mount subsystems that are not mounted previously.
+    // The subsystems for new and existing mount points have to match, and
+    // the same hierarchy will be mounted at each mount point with the same
+    // subsystem set.
+
+    Map<String, Set<String>> newMtab = null;
+    Map<CGroupController, String> cPaths;
+    try {
+      if (this.cGroupsMountConfig.mountDisabledButMountPathDefined()) {
+        newMtab = parsePreConfiguredMountPath();
+      }
+
+      if (newMtab == null) {
+        // parse mtab
+        newMtab = parseMtab(mtabFile);
+      }
+
+      // find cgroup controller paths
+      cPaths = initializeControllerPathsFromMtab(newMtab);
+    } catch (IOException e) {
+      LOG.warn("Failed to initialize controller paths! Exception: " + e);
+      throw new ResourceHandlerException(
+          "Failed to initialize controller paths!");
+    }
+
+    // we want to do a bulk update without the paths changing concurrently
+    rwLock.writeLock().lock();
+    try {
+      controllerPaths = cPaths;
+      parsedMtab = newMtab;
+    } finally {
+      rwLock.writeLock().unlock();
+    }
+  }
+
+  protected abstract Map<String, Set<String>> parsePreConfiguredMountPath() 
throws IOException;
+
+  protected Map<CGroupController, String> initializeControllerPathsFromMtab(
+      Map<String, Set<String>> parsedMtab) throws ResourceHandlerException {
+    Map<CGroupController, String> ret = new HashMap<>();
+
+    for (CGroupController controller : CGroupController.values()) {
+      String subsystemName = controller.getName();
+      String controllerPath = findControllerInMtab(subsystemName, parsedMtab);
+
+      if (controllerPath != null) {
+        ret.put(controller, controllerPath);
+      }
+    }
+    return ret;
+  }
+
+  /* We are looking for entries of the form:
+   * none /cgroup/path/mem cgroup rw,memory 0 0
+   *
+   * Use a simple pattern that splits on the five spaces, and
+   * grabs the 2, 3, and 4th fields.
+   */
+
+  private static final Pattern MTAB_FILE_FORMAT = Pattern.compile(
+      "^[^\\s]+\\s([^\\s]+)\\s([^\\s]+)\\s([^\\s]+)\\s[^\\s]+\\s[^\\s]+$");
+
+  /*
+   * Returns a map: path -> mount options
+   * for mounts with type "cgroup". Cgroup controllers will
+   * appear in the list of options for a path.
+   */
+  protected Map<String, Set<String>> parseMtab(String mtab)
+      throws IOException {
+    Map<String, Set<String>> ret = new HashMap<>();
+    BufferedReader in = null;
+
+    try {
+      FileInputStream fis = new FileInputStream(mtab);
+      in = new BufferedReader(new InputStreamReader(fis, 
StandardCharsets.UTF_8));
+
+      for (String str = in.readLine(); str != null;
+           str = in.readLine()) {
+        Matcher m = MTAB_FILE_FORMAT.matcher(str);
+        boolean mat = m.find();
+        if (mat) {
+          String path = m.group(1);
+          String type = m.group(2);
+          String options = m.group(3);
+
+          Set<String> controllerSet = handleMtabEntry(path, type, options);
+          if (controllerSet != null) {
+            ret.put(path, controllerSet);
+          }
+        }
+      }
+    } catch (IOException e) {
+      if (Shell.LINUX) {
+        throw new IOException("Error while reading " + mtab, e);
+      } else {
+        // Ignore the error, if we are running on an os other than Linux
+        LOG.warn("Error while reading " + mtab, e);
+      }
+    } finally {
+      IOUtils.cleanupWithLogger(LOG, in);
+    }
+
+    return ret;
+  }
+
+  protected abstract Set<String> handleMtabEntry(String path, String type, 
String options) throws IOException;
+
+  /**
+   * Find the hierarchy of the subsystem.
+   * The kernel ensures that a subsystem can only be part of a single 
hierarchy.
+   * The subsystem can be part of multiple mount points, if they belong to the
+   * same hierarchy.
+   *
+   * @param controller subsystem like cpu, cpuset, etc...
+   * @param entries    map of paths to mount options
+   * @return the first mount path that has the requested subsystem
+   */
+  protected String findControllerInMtab(String controller,
+                                        Map<String, Set<String>> entries) {
+    for (Map.Entry<String, Set<String>> e : entries.entrySet()) {
+      if (e.getValue().contains(controller)) {
+        if (new File(e.getKey()).canRead()) {
+          return e.getKey();
+        } else {
+          LOG.warn(String.format(
+              "Skipping inaccessible cgroup mount point %s", e.getKey()));
+        }
+      }
+    }
+
+    return null;
+  }
+
+  protected abstract void mountCGroupController(CGroupController controller) 
throws ResourceHandlerException;
+
+  @Override
+  public String getRelativePathForCGroup(String cGroupId) {
+    return cGroupPrefix + Path.SEPARATOR + cGroupId;
+  }
+
+  @Override
+  public String getPathForCGroup(CGroupController controller, String cGroupId) 
{
+    return getControllerPath(controller) + Path.SEPARATOR + cGroupPrefix
+        + Path.SEPARATOR + cGroupId;
+  }
+
+  @Override
+  public String getPathForCGroupTasks(CGroupController controller,
+                                      String cGroupId) {
+    return getPathForCGroup(controller, cGroupId)
+        + Path.SEPARATOR + CGROUP_PROCS_FILE;
+  }
+
+  @Override
+  public String getPathForCGroupParam(CGroupController controller,
+                                      String cGroupId, String param) {
+    return getPathForCGroup(controller, cGroupId)
+        + Path.SEPARATOR + controller.getName()
+        + "." + param;
+  }
+
+  /**
+   * Mount cgroup or use existing mount point based on configuration.
+   *
+   * @param controller - the controller being initialized
+   * @throws ResourceHandlerException yarn hierarchy cannot be created or
+   *                                  accessed for any reason
+   */
+  @Override
+  public void initializeCGroupController(CGroupController controller) throws
+      ResourceHandlerException {
+    if (this.cGroupsMountConfig.isMountEnabled() &&
+        cGroupsMountConfig.ensureMountPathIsDefined()) {
+      // We have a controller that needs to be mounted
+      mountCGroupController(controller);
+    }
+
+    // We are working with a pre-mounted contoller
+    // Make sure that YARN cgroup hierarchy path exists
+    initializePreMountedCGroupController(controller);
+  }
+
+  /**
+   * This function is called when the administrator opted
+   * to use a pre-mounted cgroup controller.
+   * There are two options.
+   * 1. YARN hierarchy already exists. We verify, whether we have write access
+   * in this case.
+   * 2. YARN hierarchy does not exist, yet. We create it in this case.

Review Comment:
   `updateEnabledControllersInHierarchy` should be documented too even if it 
applies to v2 only.
   
   This function checks if the hierarchy exists and writeable which is applied 
in both v1 and v2. For v2 we also write/overwrite the subtree control file. Is 
that intended, is there a use case where that file is created manually and not 
by YARN? (might be good for more flexible configuration, enabling/disabling 
controllers on the fly)
   
   https://www.kernel.org/doc/Documentation/admin-guide/cgroup-v2.rst
   ```
     cgroup.subtree_control
        A read-write space separated values file which exists on all
        cgroups.  Starts out empty.
   
        When read, it shows space separated list of the controllers
        which are enabled to control resource distribution from the
        cgroup to its children.
   
        Space separated list of controllers prefixed with '+' or '-'
        can be written to enable or disable controllers.  A controller
        name prefixed with '+' enables the controller and '-'
        disables.  If a controller appears more than once on the list,
        the last one is effective.  When multiple enable and disable
        operations are specified, either all succeed or all fail.
   ```





> Create a CgroupHandler implementation for cgroup v2
> ---------------------------------------------------
>
>                 Key: YARN-11672
>                 URL: https://issues.apache.org/jira/browse/YARN-11672
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Benjamin Teke
>            Assignee: Benjamin Teke
>            Priority: Major
>              Labels: pull-request-available
>
> [CGroupsHandler's|https://github.com/apache/hadoop/blob/69b328943edf2f61c8fc139934420e3f10bf3813/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsHandler.java#L36]
>  current implementation holds the functionality to mount and setup the YARN 
> specific cgroup v1 functionality. A similar v2 implementation should be 
> created that allows initialising the v2 structure.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to