You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/09/10 13:50:10 UTC
[GitHub] [hadoop] tomicooler commented on a change in pull request #3402: YARN-10914. Simplify duplicated code for tracking ResourceUsage in Ab…
tomicooler commented on a change in pull request #3402:
URL: https://github.com/apache/hadoop/pull/3402#discussion_r706198172
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
##########
@@ -1189,84 +1190,72 @@ boolean canAssignToThisQueue(Resource clusterResource,
}
+ private static String ensurePartition(String partition) {
+ return Optional.ofNullable(partition).orElse(RMNodeLabelsManager.NO_LABEL);
+ }
+
+ @FunctionalInterface
+ interface Counter {
+ void count(String partition, Resource reservedRes);
+ }
+
+ private void count(String partition, Resource resource, Counter counter, Counter parentCounter) {
+ final String partitionChecked = ensurePartition(partition);
Review comment:
Done.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
##########
@@ -1189,84 +1190,72 @@ boolean canAssignToThisQueue(Resource clusterResource,
}
+ private static String ensurePartition(String partition) {
+ return Optional.ofNullable(partition).orElse(RMNodeLabelsManager.NO_LABEL);
+ }
+
+ @FunctionalInterface
+ interface Counter {
+ void count(String partition, Resource reservedRes);
+ }
+
+ private void count(String partition, Resource resource, Counter counter, Counter parentCounter) {
+ final String partitionChecked = ensurePartition(partition);
+ counter.count(partitionChecked, resource);
+ Optional.ofNullable(parentCounter).ifPresent(c -> c.count(partitionChecked, resource));
+ }
+
@Override
public void incReservedResource(String partition, Resource reservedRes) {
- if (partition == null) {
- partition = RMNodeLabelsManager.NO_LABEL;
- }
-
- queueUsage.incReserved(partition, reservedRes);
- if(null != parent){
- parent.incReservedResource(partition, reservedRes);
- }
+ count(partition, reservedRes, queueUsage::incReserved,
+ parent == null ? null : parent::incReservedResource);
}
@Override
public void decReservedResource(String partition, Resource reservedRes) {
- if (partition == null) {
- partition = RMNodeLabelsManager.NO_LABEL;
- }
-
- queueUsage.decReserved(partition, reservedRes);
- if(null != parent){
- parent.decReservedResource(partition, reservedRes);
- }
+ count(partition, reservedRes, queueUsage::decReserved,
+ parent == null ? null : parent::decReservedResource);
}
@Override
public void incPendingResource(String nodeLabel, Resource resourceToInc) {
- if (nodeLabel == null) {
- nodeLabel = RMNodeLabelsManager.NO_LABEL;
- }
- // ResourceUsage has its own lock, no addition lock needs here.
- queueUsage.incPending(nodeLabel, resourceToInc);
- if (null != parent) {
- parent.incPendingResource(nodeLabel, resourceToInc);
- }
+ count(nodeLabel, resourceToInc, queueUsage::incPending,
+ parent == null ? null : parent::incPendingResource);
}
@Override
public void decPendingResource(String nodeLabel, Resource resourceToDec) {
- if (nodeLabel == null) {
- nodeLabel = RMNodeLabelsManager.NO_LABEL;
- }
- // ResourceUsage has its own lock, no addition lock needs here.
- queueUsage.decPending(nodeLabel, resourceToDec);
- if (null != parent) {
- parent.decPendingResource(nodeLabel, resourceToDec);
- }
+ count(nodeLabel, resourceToDec, queueUsage::decPending,
+ parent == null ? null : parent::decPendingResource);
+ }
+
+ @FunctionalInterface
+ interface CounterWithApp {
+ void count(String partition, Resource reservedRes, SchedulerApplicationAttempt application);
+ }
+
+ private void countAndUpdate(String partition, Resource resource,
+ Counter counter, CounterWithApp parentCounter) {
+ final String partitionChecked = ensurePartition(partition);
+ counter.count(partitionChecked, resource);
Review comment:
Done.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
##########
@@ -1189,84 +1190,72 @@ boolean canAssignToThisQueue(Resource clusterResource,
}
+ private static String ensurePartition(String partition) {
+ return Optional.ofNullable(partition).orElse(RMNodeLabelsManager.NO_LABEL);
+ }
+
+ @FunctionalInterface
+ interface Counter {
+ void count(String partition, Resource reservedRes);
Review comment:
Renamed to resource.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/AbstractCSQueue.java
##########
@@ -1189,84 +1190,72 @@ boolean canAssignToThisQueue(Resource clusterResource,
}
+ private static String ensurePartition(String partition) {
+ return Optional.ofNullable(partition).orElse(RMNodeLabelsManager.NO_LABEL);
+ }
+
+ @FunctionalInterface
+ interface Counter {
+ void count(String partition, Resource reservedRes);
+ }
+
+ private void count(String partition, Resource resource, Counter counter, Counter parentCounter) {
+ final String partitionChecked = ensurePartition(partition);
+ counter.count(partitionChecked, resource);
+ Optional.ofNullable(parentCounter).ifPresent(c -> c.count(partitionChecked, resource));
+ }
+
@Override
public void incReservedResource(String partition, Resource reservedRes) {
- if (partition == null) {
- partition = RMNodeLabelsManager.NO_LABEL;
- }
-
- queueUsage.incReserved(partition, reservedRes);
- if(null != parent){
- parent.incReservedResource(partition, reservedRes);
- }
+ count(partition, reservedRes, queueUsage::incReserved,
+ parent == null ? null : parent::incReservedResource);
}
@Override
public void decReservedResource(String partition, Resource reservedRes) {
- if (partition == null) {
- partition = RMNodeLabelsManager.NO_LABEL;
- }
-
- queueUsage.decReserved(partition, reservedRes);
- if(null != parent){
- parent.decReservedResource(partition, reservedRes);
- }
+ count(partition, reservedRes, queueUsage::decReserved,
+ parent == null ? null : parent::decReservedResource);
}
@Override
public void incPendingResource(String nodeLabel, Resource resourceToInc) {
- if (nodeLabel == null) {
- nodeLabel = RMNodeLabelsManager.NO_LABEL;
- }
- // ResourceUsage has its own lock, no addition lock needs here.
- queueUsage.incPending(nodeLabel, resourceToInc);
- if (null != parent) {
- parent.incPendingResource(nodeLabel, resourceToInc);
- }
+ count(nodeLabel, resourceToInc, queueUsage::incPending,
+ parent == null ? null : parent::incPendingResource);
}
@Override
public void decPendingResource(String nodeLabel, Resource resourceToDec) {
- if (nodeLabel == null) {
- nodeLabel = RMNodeLabelsManager.NO_LABEL;
- }
- // ResourceUsage has its own lock, no addition lock needs here.
- queueUsage.decPending(nodeLabel, resourceToDec);
- if (null != parent) {
- parent.decPendingResource(nodeLabel, resourceToDec);
- }
+ count(nodeLabel, resourceToDec, queueUsage::decPending,
+ parent == null ? null : parent::decPendingResource);
+ }
+
+ @FunctionalInterface
+ interface CounterWithApp {
+ void count(String partition, Resource reservedRes, SchedulerApplicationAttempt application);
+ }
+
+ private void countAndUpdate(String partition, Resource resource,
Review comment:
Rearranged it.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org