From f8e36e03b4e65f173bf2330715dceafdb6111190 Mon Sep 17 00:00:00 2001 From: HUAN-PING SU Date: Mon, 25 Nov 2019 14:28:53 +0800 Subject: [PATCH] YARN-9966. Code duplication in UserGroupMappingPlacementRule (#1709) --- .../placement/QueuePlacementRuleUtils.java | 5 +- .../UserGroupMappingPlacementRule.java | 76 +------------------ 2 files changed, 7 insertions(+), 74 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java index 6f2ee33f86b..9463603f81d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/QueuePlacementRuleUtils.java @@ -41,8 +41,9 @@ public final class QueuePlacementRuleUtils { private QueuePlacementRuleUtils() { } - private static void validateQueueMappingUnderParentQueue(CSQueue parentQueue, - String parentQueueName, String leafQueueName) throws IOException { + public static void validateQueueMappingUnderParentQueue( + CSQueue parentQueue, String parentQueueName, + String leafQueueName) throws IOException { if (parentQueue == null) { throw new IOException( "mapping contains invalid or non-leaf queue [" + leafQueueName diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java index 246ade78846..d85ac6d482a 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java @@ -44,8 +44,6 @@ import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.LeafQueue; import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ManagedParentQueue; -import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.DOT; - public class UserGroupMappingPlacementRule extends PlacementRule { private static final Logger LOG = LoggerFactory .getLogger(UserGroupMappingPlacementRule.class); @@ -307,7 +305,8 @@ public boolean initialize(ResourceScheduler scheduler) // check if mappings refer to valid queues for (QueueMapping mapping : queueMappings) { - QueuePath queuePath = extractQueuePath(mapping.getQueue()); + QueuePath queuePath = QueuePlacementRuleUtils + .extractQueuePath(mapping.getQueue()); if (isStaticQueueMapping(mapping)) { //Try getting queue by its leaf queue name // without splitting into parent/leaf queues @@ -411,7 +410,8 @@ private static QueueMapping validateAndGetAutoCreatedQueueMapping( } else if (queuePath.hasParentQueue()) { //if parent queue is specified, // then it should exist and be an instance of ManagedParentQueue - validateParentQueue(queueManager.getQueue(queuePath.getParentQueue()), + QueuePlacementRuleUtils.validateQueueMappingUnderParentQueue( + queueManager.getQueue(queuePath.getParentQueue()), queuePath.getParentQueue(), queuePath.getLeafQueue()); return new QueueMapping(mapping.getType(), mapping.getSource(), queuePath.getLeafQueue(), queuePath.getParentQueue()); @@ -429,74 +429,6 @@ private static boolean isStaticQueueMapping(QueueMapping mapping) { .contains(UserGroupMappingPlacementRule.SECONDARY_GROUP_MAPPING); } - private static class QueuePath { - - public String parentQueue; - public String leafQueue; - - public QueuePath(final String leafQueue) { - this.leafQueue = leafQueue; - } - - public QueuePath(final String parentQueue, final String leafQueue) { - this.parentQueue = parentQueue; - this.leafQueue = leafQueue; - } - - public String getParentQueue() { - return parentQueue; - } - - public String getLeafQueue() { - return leafQueue; - } - - public boolean hasParentQueue() { - return parentQueue != null; - } - - @Override - public String toString() { - return parentQueue + DOT + leafQueue; - } - } - - private static QueuePath extractQueuePath(String queueName) - throws IOException { - int parentQueueNameEndIndex = queueName.lastIndexOf(DOT); - - if (parentQueueNameEndIndex > -1) { - final String parentQueue = queueName.substring(0, parentQueueNameEndIndex) - .trim(); - final String leafQueue = queueName.substring(parentQueueNameEndIndex + 1) - .trim(); - return new QueuePath(parentQueue, leafQueue); - } - - return new QueuePath(queueName); - } - - private static void validateParentQueue(CSQueue parentQueue, - String parentQueueName, String leafQueueName) throws IOException { - if (parentQueue == null) { - throw new IOException( - "mapping contains invalid or non-leaf queue [" + leafQueueName - + "] and invalid parent queue [" + parentQueueName + "]"); - } else if (!(parentQueue instanceof ManagedParentQueue)) { - throw new IOException("mapping contains leaf queue [" + leafQueueName - + "] and invalid parent queue which " - + "does not have auto creation of leaf queues enabled [" - + parentQueueName + "]"); - } else if (!parentQueue.getQueueName().equals(parentQueueName)) { - throw new IOException( - "mapping contains invalid or non-leaf queue [" + leafQueueName - + "] and invalid parent queue " - + "which does not match existing leaf queue's parent : [" - + parentQueueName + "] does not match [ " + parentQueue - .getQueueName() + "]"); - } - } - @VisibleForTesting public List getQueueMappings() { return mappings;