From 2cac82bb3f52f66d443e0ff3ad35592f1902e432 Mon Sep 17 00:00:00 2001 From: Gergely Pollak Date: Thu, 30 Apr 2020 17:12:36 +0200 Subject: [PATCH 2/2] YARN-10254. CapacityScheduler incorrect User Group Mapping after leaf queue change Change-Id: I0bce6297c366d97904c80ae7da9712cfe18a9ca1 --- .../UserGroupMappingPlacementRule.java | 133 ++++++++++-------- 1 file changed, 74 insertions(+), 59 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/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 391fb34c5d6..44fd3f38382 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 @@ -115,9 +115,9 @@ private ApplicationPlacementContext getPlacementForUser(String user) } else if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) { return getPlacementContext(mapping, user); } else if (mapping.getQueue().equals(PRIMARY_GROUP_MAPPING)) { - return getContextForPrimaryGroup(user, mapping); + return getContextForGroup(getPrimaryGroup(user), mapping); } else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) { - return getContextForSecondaryGroup(user, mapping); + return getContextForGroup(getSecondaryGroup(user), mapping); } else { return getPlacementContext(mapping); } @@ -152,49 +152,25 @@ private ApplicationPlacementContext getPlacementForUser(String user) return null; } + private QueueMapping alterMapping( + QueueMapping mapping, String parentPath, String leafName) { + return QueueMappingBuilder.create() + .type(mapping.getType()) + .source(mapping.getSource()) + .queue(leafName) + .parentQueue(parentPath) + .build(); + } + // invoked for mappings: // u:%user:[parent].%primary_group // u:%user:%primary_group - private ApplicationPlacementContext getContextForPrimaryGroup( - String user, - QueueMapping mapping) throws IOException { - String group = - CapacitySchedulerConfiguration.ROOT + "." + getPrimaryGroup(user); - - String parent = mapping.getParentQueue(); - CSQueue groupQueue = queueManager.getQueue(group); - - if (parent != null) { - CSQueue parentQueue = queueManager.getQueue(parent); - - if (parentQueue instanceof ManagedParentQueue) { - return getPlacementContext(mapping, group); - } else { - return groupQueue == null ? null : getPlacementContext(mapping, group); - } - } else { - return groupQueue == null ? null : getPlacementContext(mapping, group); - } - } - - // invoked for mappings - // u:%user:%secondary_group // u:%user:[parent].%secondary_group - private ApplicationPlacementContext getContextForSecondaryGroup( - String user, + // u:%user:%secondary_group + private ApplicationPlacementContext getContextForGroup( + String group, QueueMapping mapping) throws IOException { - String secondaryGroup = getSecondaryGroup(user); - - if (secondaryGroup != null) { - CSQueue queue = this.queueManager.getQueue(secondaryGroup); - if ( queue != null) { - return getPlacementContext(mapping, queue.getQueuePath()); - } else { - return null; - } - } else { - return null; - } + return getPlacementContext(mapping, group); } // invoked for mappings: @@ -205,17 +181,13 @@ private ApplicationPlacementContext getContextForGroupParent( QueueMapping mapping, String group) throws IOException { - if (this.queueManager.getQueue(group) != null) { + CSQueue groupQueue = this.queueManager.getQueue(group); + if ( groupQueue != null) { // replace the group string - QueueMapping resolvedGroupMapping = - QueueMappingBuilder.create() - .type(mapping.getType()) - .source(mapping.getSource()) - .queue(user) - .parentQueue( - CapacitySchedulerConfiguration.ROOT + "." + - group) - .build(); + QueueMapping resolvedGroupMapping = alterMapping( + mapping, + groupQueue.getQueuePath(), + user); validateQueueMapping(resolvedGroupMapping); return getPlacementContext(resolvedGroupMapping, user); } else { @@ -247,7 +219,7 @@ public ApplicationPlacementContext getPlacementForApp( } catch (IOException ioex) { String message = "Failed to submit application " + applicationId + " submitted by user " + user + " reason: " + ioex.getMessage(); - throw new YarnException(message); + throw new YarnException(message, ioex); } } return null; @@ -258,6 +230,48 @@ private ApplicationPlacementContext getPlacementContext( return getPlacementContext(mapping, mapping.getQueue()); } + private ApplicationPlacementContext getPlacementContextWithParent( + QueueMapping mapping, + String leafQueueName) { + CSQueue parent = queueManager.getQueue(mapping.getParentQueue()); + //we don't find the specified parent, so the placement rule is invalid + //for this case + if (parent == null) { + return null; + } + + //if we have a parent which is not a managed parent, we check if the leaf + //queue exists under this parent + if (!(parent instanceof ManagedParentQueue)) { + CSQueue queue = queueManager.getQueue( + mapping.getParentQueue() + "." + leafQueueName); + //if the queue doesn't exit we return null + if (queue == null) { + return null; + } + } + //at this point we either have a managed parent or the queue actually + //exists so we have a placement context, returning it + return new ApplicationPlacementContext(leafQueueName, + parent.getQueuePath()); + } + + private ApplicationPlacementContext getPlacementContextNoParent( + String leafQueueName) { + //in this case we don't have a parent specified so we expect the queue to + //exist, otherwise the mapping will not be valid for this case + CSQueue queue = queueManager.getQueue(leafQueueName); + if (queue == null) { + return null; + } + + //getting parent path to make sure if the leaf name would become ambiguous + //the placement context stays valid. + CSQueue parent = queueManager.getQueue(leafQueueName).getParent(); + return new ApplicationPlacementContext( + leafQueueName, parent.getQueuePath()); + } + private ApplicationPlacementContext getPlacementContext(QueueMapping mapping, String leafQueueName) throws IOException { @@ -268,10 +282,9 @@ private ApplicationPlacementContext getPlacementContext(QueueMapping mapping, } if (!StringUtils.isEmpty(mapping.getParentQueue())) { - return new ApplicationPlacementContext(leafQueueName, - mapping.getParentQueue()); - } else{ - return new ApplicationPlacementContext(leafQueueName); + return getPlacementContextWithParent(mapping, leafQueueName); + } else { + return getPlacementContextNoParent(leafQueueName); } } @@ -449,10 +462,12 @@ private void validateQueueMapping(QueueMapping queueMapping) //as mapping.getQueue() if (leafQueue == null && queueManager.isAmbiguous(leafQueueFullName)) { throw new IOException("mapping contains ambiguous leaf queue name: " - + leafQueueFullName); - } else { - throw new IOException("mapping contains invalid or non-leaf queue : " - + leafQueueFullName); + + leafQueueFullName); + } else if (parentQueue == null || + (!(parentQueue instanceof ManagedParentQueue))) { + throw new IOException("mapping contains invalid or non-leaf queue " + + " and no managed parent is found: " + + leafQueueFullName); } } else if (parentQueue == null || (!(parentQueue instanceof ParentQueue))) { throw new IOException( -- 2.26.2