brumi1024 commented on code in PR #6835:
URL: https://github.com/apache/hadoop/pull/6835#discussion_r1617045570


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsResourceCalculator.java:
##########
@@ -18,338 +18,145 @@
 
 package 
org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.resources;
 
-import org.apache.hadoop.classification.VisibleForTesting;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.apache.hadoop.util.CpuTimeTracker;
-import org.apache.hadoop.util.Shell;
-import org.apache.hadoop.util.SysInfoLinux;
-import org.apache.hadoop.yarn.exceptions.YarnException;
-import org.apache.hadoop.yarn.util.Clock;
-import org.apache.hadoop.yarn.util.ResourceCalculatorProcessTree;
-import org.apache.hadoop.yarn.util.SystemClock;
-
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.math.BigInteger;
-import java.nio.charset.StandardCharsets;
-import java.util.function.Function;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
- * A cgroups file-system based Resource calculator without the process tree
- * features.
+ * A Cgroup version 1 file-system based Resource calculator without the 
process tree features.
  *
- * CGroups has its limitations. It can only be enabled, if both CPU and memory
- * cgroups are enabled with yarn.nodemanager.resource.cpu.enabled and
- * yarn.nodemanager.resource.memory.enabled respectively. This means that
- * memory limits are enforced by default. You can turn this off and keep
- * memory reporting only with yarn.nodemanager.resource.memory.enforced.
+ * Warning!!!
+ * ResourceCalculatorProcessTree can be used with 
mapreduce.job.process-tree.class property.
+ * However, those instances runs in the mapreduce task, and can not access to 
the
+ * ResourceHandlerModule, what is only initialised in the NodeManager process 
not in the container.
+ * So this implementation will not work with the 
mapreduce.job.process-tree.class property.

Review Comment:
   ```suggestion
    * Warning: this implementation will not work properly when configured using 
the mapreduce.job.process-tree.class job property.
    * Theoretically the ResourceCalculatorProcessTree can be configured using 
the mapreduce.job.process-tree.class job property, however it
    * has a dependency on an instantiated ResourceHandlerModule, which is only 
initialised in the NodeManager process 
    * and not in the containers.
   ```



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/CGroupsV2ResourceCalculator.java:
##########
@@ -0,0 +1,135 @@
+/**
+ * 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 java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * A Cgroup version 2 file-system based Resource calculator without the 
process tree features.
+ *
+ * Warning!!!
+ * ResourceCalculatorProcessTree can be used with 
mapreduce.job.process-tree.class property.
+ * However, those instances runs in the mapreduce task, and can not access to 
the
+ * ResourceHandlerModule, what is only initialised in the NodeManager process 
not in the container.
+ * So this implementation will not work with the 
mapreduce.job.process-tree.class property.

Review Comment:
   ```suggestion
    * Warning: this implementation will not work properly when configured using 
the mapreduce.job.process-tree.class job property.
    * Theoretically the ResourceCalculatorProcessTree can be configured using 
the mapreduce.job.process-tree.class job property, however it
    * has a dependency on an instantiated ResourceHandlerModule, which is only 
initialised in the NodeManager process 
    * and not in the containers.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to