[ https://issues.apache.org/jira/browse/YARN-11687?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17845932#comment-17845932 ]
ASF GitHub Bot commented on YARN-11687: --------------------------------------- K0K0V0K commented on code in PR #6780: URL: https://github.com/apache/hadoop/pull/6780#discussion_r1598513673 ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/AbstractCGroupsResourceCalculator.java: ########## @@ -0,0 +1,179 @@ +/** + * 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.math.BigInteger; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.commons.io.FileUtils; +import org.apache.hadoop.classification.VisibleForTesting; +import org.apache.hadoop.util.CpuTimeTracker; +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; + +/** + * Common code base for the CGroupsResourceCalculator implementations. + */ +public abstract class AbstractCGroupsResourceCalculator extends ResourceCalculatorProcessTree { + private static final Logger LOG = LoggerFactory.getLogger(AbstractCGroupsResourceCalculator.class); + protected final String pid; + protected final Clock clock = SystemClock.getInstance(); + protected final Map<String, String> stats = new ConcurrentHashMap<>(); + + @VisibleForTesting + long jiffyLengthMs = SysInfoLinux.JIFFY_LENGTH_IN_MILLIS; + @VisibleForTesting + CpuTimeTracker cpuTimeTracker; + @VisibleForTesting + CGroupsHandler cGroupsHandler; + @VisibleForTesting + String root = "/"; Review Comment: Thanks for the review! I modified this variable to procFs. However, I would rather keep this out of the constructor. - If we create a new constructor with this "VisibleForTesting" annotation we will have a 4 and 5 params constructors, which seems a bit heavy to me. - To use only one method to set param for testing, I should add the other 3 params (jiffyLengthMs, cpuTimeTracker, cGroupsHandler) to the new constructor, so actually we will have a 4 and 8 param constructor. - if we move the PROCFS variable to the ResourceCalculatorProcessTree, it will be present in the WindowsBasedProcessTree which uses a shell command to get this information and the variable can be misleading in there context. > Update CGroupsResourceCalculator to track usages using cgroupv2 > --------------------------------------------------------------- > > Key: YARN-11687 > URL: https://issues.apache.org/jira/browse/YARN-11687 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Benjamin Teke > Assignee: Bence Kosztolnik > Priority: Major > Labels: pull-request-available > > [CGroupsResourceCalculator|https://github.com/apache/hadoop/blob/f609460bda0c2bd87dd3580158e549e2f34f14d5/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] > should also be updated to handle the cgroup v2 changes. -- 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