Browse Source

YARN-9966. Code duplication in UserGroupMappingPlacementRule (#1709)

HUAN-PING SU 5 years ago
parent
commit
f8e36e03b4

+ 3 - 2
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 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) {
     if (parentQueue == null) {
       throw new IOException(
       throw new IOException(
           "mapping contains invalid or non-leaf queue [" + leafQueueName
           "mapping contains invalid or non-leaf queue [" + leafQueueName

+ 4 - 72
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.Capacity
 import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.LeafQueue;
 import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.LeafQueue;
 import org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ManagedParentQueue;
 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 {
 public class UserGroupMappingPlacementRule extends PlacementRule {
   private static final Logger LOG = LoggerFactory
   private static final Logger LOG = LoggerFactory
       .getLogger(UserGroupMappingPlacementRule.class);
       .getLogger(UserGroupMappingPlacementRule.class);
@@ -307,7 +305,8 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
     // check if mappings refer to valid queues
     // check if mappings refer to valid queues
     for (QueueMapping mapping : queueMappings) {
     for (QueueMapping mapping : queueMappings) {
 
 
-      QueuePath queuePath = extractQueuePath(mapping.getQueue());
+      QueuePath queuePath = QueuePlacementRuleUtils
+              .extractQueuePath(mapping.getQueue());
       if (isStaticQueueMapping(mapping)) {
       if (isStaticQueueMapping(mapping)) {
         //Try getting queue by its leaf queue name
         //Try getting queue by its leaf queue name
         // without splitting into parent/leaf queues
         // without splitting into parent/leaf queues
@@ -411,7 +410,8 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
     } else if (queuePath.hasParentQueue()) {
     } else if (queuePath.hasParentQueue()) {
       //if parent queue is specified,
       //if parent queue is specified,
       // then it should exist and be an instance of ManagedParentQueue
       // 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());
           queuePath.getParentQueue(), queuePath.getLeafQueue());
       return new QueueMapping(mapping.getType(), mapping.getSource(),
       return new QueueMapping(mapping.getType(), mapping.getSource(),
           queuePath.getLeafQueue(), queuePath.getParentQueue());
           queuePath.getLeafQueue(), queuePath.getParentQueue());
@@ -429,74 +429,6 @@ public class UserGroupMappingPlacementRule extends PlacementRule {
             .contains(UserGroupMappingPlacementRule.SECONDARY_GROUP_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
   @VisibleForTesting
   public List<QueueMapping> getQueueMappings() {
   public List<QueueMapping> getQueueMappings() {
     return mappings;
     return mappings;