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 391fb34c5d..643fc5015d 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 @@ -105,35 +105,76 @@ private ApplicationPlacementContext getPlacementForUser(String user) if (mapping.getParentQueue() != null && mapping.getParentQueue().equals(PRIMARY_GROUP_MAPPING) && mapping.getQueue().equals(CURRENT_USER_MAPPING)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "primary group current user mapping", user); + } return getContextForGroupParent(user, mapping, getPrimaryGroup(user)); } else if (mapping.getParentQueue() != null && mapping.getParentQueue().equals(SECONDARY_GROUP_MAPPING) && mapping.getQueue().equals(CURRENT_USER_MAPPING)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "secondary group current user mapping", user); + } return getContextForGroupParent(user, mapping, getSecondaryGroup(user)); } else if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "current user mapping", user); + } return getPlacementContext(mapping, user); } else if (mapping.getQueue().equals(PRIMARY_GROUP_MAPPING)) { - return getContextForPrimaryGroup(user, mapping); + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "primary group mapping", user); + } + return getPlacementContext(mapping, getPrimaryGroup(user)); } else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) { - return getContextForSecondaryGroup(user, mapping); + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "secondary group mapping", user); + } + return getPlacementContext(mapping, getSecondaryGroup(user)); } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static user static mapping", user); + } return getPlacementContext(mapping); } } if (user.equals(mapping.getSource())) { if (mapping.getQueue().equals(PRIMARY_GROUP_MAPPING)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static user primary group mapping", user); + } return getPlacementContext(mapping, getPrimaryGroup(user)); } else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) { String secondaryGroup = getSecondaryGroup(user); if (secondaryGroup != null) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static user secondary group mapping", user); + } return getPlacementContext(mapping, secondaryGroup); } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Wanted to create placement context for user {}" + + " using static user secondary group mapping," + + " but user has no secondary group!", user); + } return null; } } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static user static mapping", user); + } return getPlacementContext(mapping); } } @@ -142,8 +183,16 @@ private ApplicationPlacementContext getPlacementForUser(String user) for (String userGroups : groups.getGroups(user)) { if (userGroups.equals(mapping.getSource())) { if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) { + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static group current user mapping", user); + } return getPlacementContext(mapping, user); } + if (LOG.isDebugEnabled()) { + LOG.debug("Creating placement context for user {} using " + + "static group static mapping", user); + } return getPlacementContext(mapping); } } @@ -152,49 +201,23 @@ private ApplicationPlacementContext getPlacementForUser(String user) return null; } - // 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, - 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; - } + /** + * This convenience method allows to change the parent path or a leafName in + * a mapping object, by creating a new one, using the builder and copying the + * rest of the parameters. + * @param mapping The mapping to be changed + * @param parentPath The new parentPath of the mapping + * @param leafName The new leafQueueName of the mapping + * @return The updated NEW mapping + */ + 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: @@ -205,20 +228,23 @@ 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 { + if (queueManager.isAmbiguous(group)) { + LOG.info("Queue mapping rule expect group queue to exist with name {}" + + " but the reference is ambiguous!", group); + } else { + LOG.info("Queue mapping rule expect group queue to exist with name {}" + + " but it does not exist!", group); + } return null; } } @@ -247,7 +273,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; @@ -260,7 +286,6 @@ private ApplicationPlacementContext getPlacementContext( private ApplicationPlacementContext getPlacementContext(QueueMapping mapping, String leafQueueName) throws IOException { - //leafQueue name no longer identifies a queue uniquely checking ambiguity if (!mapping.hasParentQueue() && queueManager.isAmbiguous(leafQueueName)) { throw new IOException("mapping contains ambiguous leaf queue reference " + @@ -268,13 +293,71 @@ 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); } } + 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) { + if (queueManager.isAmbiguous(mapping.getParentQueue())) { + LOG.warn("Placement rule specified a parent queue {}, but it is" + + "ambiguous.", mapping.getParentQueue()); + } else { + LOG.warn("Placement rule specified a parent queue {}, but it does" + + "not exist.", mapping.getParentQueue()); + } + return null; + } + + String parentPath = parent.getQueuePath(); + + //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(parentPath + "." + leafQueueName); + //if the queue doesn't exit we return null + if (queue == null) { + LOG.warn("Placement rule specified a parent queue {}, but it is" + + " not a managed parent queue, and no queue exists with name {} " + + "under it.", mapping.getParentQueue(), leafQueueName); + 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, parentPath); + } + + 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) { + if (queueManager.isAmbiguous(leafQueueName)) { + LOG.warn("Queue {} specified in placement rule is ambiguous", + leafQueueName); + } else { + LOG.warn("Queue {} specified in placement rule does not exist", + leafQueueName); + } + 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()); + } + @VisibleForTesting @Override public boolean initialize(ResourceScheduler scheduler) @@ -449,10 +532,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( diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestUserGroupMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestUserGroupMappingPlacementRule.java index 1d7b6b7ff7..5d2aaa17a5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestUserGroupMappingPlacementRule.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/placement/TestUserGroupMappingPlacementRule.java @@ -21,6 +21,7 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.isNull; import java.util.Arrays; @@ -70,13 +71,18 @@ private void verifyQueueMapping(QueueMappingTestData queueMappingTestData) CapacitySchedulerQueueManager queueManager = mock(CapacitySchedulerQueueManager.class); + ParentQueue root = mock(ParentQueue.class); + when(root.getQueuePath()).thenReturn("root"); + ParentQueue agroup = mock(ParentQueue.class); when(agroup.getQueuePath()).thenReturn("root.agroup"); ParentQueue bsubgroup2 = mock(ParentQueue.class); when(bsubgroup2.getQueuePath()).thenReturn("root.bsubgroup2"); + when(bsubgroup2.getParent()).thenReturn(root); ManagedParentQueue managedParent = mock(ManagedParentQueue.class); - when(managedParent.getQueueName()).thenReturn("root.managedParent"); + when(managedParent.getQueueName()).thenReturn("managedParent"); + when(managedParent.getQueuePath()).thenReturn("root.managedParent"); LeafQueue a = mock(LeafQueue.class); when(a.getQueuePath()).thenReturn("root.agroup.a"); @@ -86,15 +92,23 @@ private void verifyQueueMapping(QueueMappingTestData queueMappingTestData) when(b.getParent()).thenReturn(bsubgroup2); LeafQueue asubgroup2 = mock(LeafQueue.class); when(asubgroup2.getQueuePath()).thenReturn("root.asubgroup2"); + when(asubgroup2.getParent()).thenReturn(root); + when(queueManager.getQueue(isNull())).thenReturn(null); when(queueManager.getQueue("a")).thenReturn(a); + when(a.getParent()).thenReturn(agroup); when(queueManager.getQueue("b")).thenReturn(b); + when(b.getParent()).thenReturn(bsubgroup2); when(queueManager.getQueue("agroup")).thenReturn(agroup); + when(agroup.getParent()).thenReturn(root); when(queueManager.getQueue("bsubgroup2")).thenReturn(bsubgroup2); + when(bsubgroup2.getParent()).thenReturn(root); when(queueManager.getQueue("asubgroup2")).thenReturn(asubgroup2); + when(asubgroup2.getParent()).thenReturn(root); when(queueManager.getQueue("managedParent")).thenReturn(managedParent); - when(queueManager.getQueue(null)).thenThrow(new NullPointerException()); + when(managedParent.getParent()).thenReturn(root); + when(queueManager.getQueue("root")).thenReturn(root); when(queueManager.getQueue("root.agroup")).thenReturn(agroup); when(queueManager.getQueue("root.bsubgroup2")).thenReturn(bsubgroup2); when(queueManager.getQueue("root.asubgroup2")).thenReturn(asubgroup2); @@ -135,7 +149,8 @@ public void testSecondaryGroupMapping() throws YarnException { .source("%user") .queue("%secondary_group").build()) .inputUser("a") - .expectedQueue("root.asubgroup2") + .expectedQueue("asubgroup2") + .expectedParentQueue("root") .build()); // PrimaryGroupMapping.class returns only primary group, no secondary groups @@ -176,35 +191,37 @@ public void testNullGroupMapping() throws YarnException { @Test public void testMapping() throws YarnException { + //if a mapping rule defines no parent, we cannot expect auto creation, + // so we must provide already existing queues verifyQueueMapping( QueueMappingTestDataBuilder.create() .queueMapping(QueueMappingBuilder.create() .type(MappingType.USER) .source("a") - .queue("q1") + .queue("a") .build()) .inputUser("a") - .expectedQueue("q1") + .expectedQueue("a") .build()); verifyQueueMapping( QueueMappingTestDataBuilder.create() .queueMapping(QueueMappingBuilder.create() .type(MappingType.GROUP) .source("agroup") - .queue("q1") + .queue("a") .build()) .inputUser("a") - .expectedQueue("q1") + .expectedQueue("a") .build()); verifyQueueMapping( QueueMappingTestDataBuilder.create() .queueMapping(QueueMappingBuilder.create() .type(MappingType.USER) .source("%user") - .queue("q2") + .queue("b") .build()) .inputUser("a") - .expectedQueue("q2") + .expectedQueue("b") .build()); verifyQueueMapping( QueueMappingTestDataBuilder.create() @@ -224,7 +241,8 @@ public void testMapping() throws YarnException { .queue("%primary_group") .build()) .inputUser("a") - .expectedQueue("root.agroup") + .expectedQueue("agroup") + .expectedParentQueue("root") .build()); verifyQueueMapping( QueueMappingTestDataBuilder.create() @@ -255,10 +273,10 @@ public void testMapping() throws YarnException { .queueMapping(QueueMappingBuilder.create() .type(MappingType.GROUP) .source("asubgroup1") - .queue("q1") + .queue("a") .build()) .inputUser("a") - .expectedQueue("q1") + .expectedQueue("a") .build()); // "agroup" queue exists @@ -268,10 +286,11 @@ public void testMapping() throws YarnException { .type(MappingType.USER) .source("%user") .queue("%primary_group") - .parentQueue("bsubgroup2") + .parentQueue("root") .build()) .inputUser("a") - .expectedQueue("root.agroup") + .expectedQueue("agroup") + .expectedParentQueue("root") .build()); // "abcgroup" queue doesn't exist, %primary_group queue, not managed parent @@ -297,7 +316,8 @@ public void testMapping() throws YarnException { .parentQueue("managedParent") .build()) .inputUser("abc") - .expectedQueue("root.abcgroup") + .expectedQueue("abcgroup") + .expectedParentQueue("root.managedParent") .build()); // "abcgroup" queue doesn't exist, %secondary_group queue @@ -320,10 +340,11 @@ public void testMapping() throws YarnException { .type(MappingType.USER) .source("%user") .queue("%secondary_group") - .parentQueue("bsubgroup2") + .parentQueue("root") .build()) .inputUser("a") - .expectedQueue("root.asubgroup2") + .expectedQueue("asubgroup2") + .expectedParentQueue("root") .build()); // specify overwritten, and see if user specified a queue, and it will be @@ -333,11 +354,11 @@ public void testMapping() throws YarnException { .queueMapping(QueueMappingBuilder.create() .type(MappingType.USER) .source("user") - .queue("q1") + .queue("a") .build()) .inputUser("user") - .inputQueue("q2") - .expectedQueue("q1") + .inputQueue("b") + .expectedQueue("a") .overwrite(true) .build()); @@ -347,11 +368,11 @@ public void testMapping() throws YarnException { .queueMapping(QueueMappingBuilder.create() .type(MappingType.USER) .source("user") - .queue("q1") + .queue("a") .build()) .inputUser("user") - .inputQueue("q2") - .expectedQueue("q2") + .inputQueue("b") + .expectedQueue("b") .build()); // if overwritten not specified, it should be which user specified @@ -364,8 +385,8 @@ public void testMapping() throws YarnException { .parentQueue("usergroup") .build()) .inputUser("user") - .inputQueue("default") - .expectedQueue("user") + .inputQueue("a") + .expectedQueue("a") .build()); // if overwritten not specified, it should be which user specified @@ -374,12 +395,12 @@ public void testMapping() throws YarnException { .queueMapping(QueueMappingBuilder.create() .type(MappingType.GROUP) .source("usergroup") - .queue("%user") - .parentQueue("usergroup") + .queue("b") + .parentQueue("root.bsubgroup2") .build()) .inputUser("user") - .inputQueue("agroup") - .expectedQueue("user") + .inputQueue("a") + .expectedQueue("b") .overwrite(true) .build()); @@ -391,7 +412,7 @@ public void testMapping() throws YarnException { .type(MappingType.GROUP) .source("agroup") .queue("%user") - .parentQueue("parent1") + .parentQueue("root.agroup") .build()) .inputUser("a") .expectedQueue("a") diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerQueueMappingFactory.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerQueueMappingFactory.java index adad396864..eedc6aecde 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerQueueMappingFactory.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerQueueMappingFactory.java @@ -206,7 +206,7 @@ public void testNestedUserQueueWithStaticParentQueue() throws Exception { ApplicationPlacementContext ctx2 = r.getPlacementForApp(asc, "user2"); assertEquals("Queue", "user2", ctx2.getQueue()); - assertEquals("Queue", "c", ctx2.getParentQueue()); + assertEquals("Queue", "root.c", ctx2.getParentQueue()); } finally { if(mockRM != null) { mockRM.close(); @@ -398,7 +398,7 @@ public void testDynamicPrimaryGroupQueue() throws Exception { // u:user2:%primary_group QueueMapping userQueueMapping2 = QueueMappingBuilder.create() .type(QueueMapping.MappingType.USER) - .source("user2") + .source("a1") .queue("%primary_group") .build(); @@ -430,8 +430,8 @@ public void testDynamicPrimaryGroupQueue() throws Exception { ApplicationPlacementContext ctx = r.getPlacementForApp(asc, "user1"); assertEquals("Queue", "b1", ctx.getQueue()); - ApplicationPlacementContext ctx1 = r.getPlacementForApp(asc, "user2"); - assertEquals("Queue", "user2group", ctx1.getQueue()); + ApplicationPlacementContext ctx1 = r.getPlacementForApp(asc, "a1"); + assertEquals("Queue", "a1group", ctx1.getQueue()); } finally { if (mockRM != null) { mockRM.close(); @@ -467,14 +467,14 @@ public void testFixedUserWithDynamicGroupQueue() throws Exception { // u:user2:%primary_group QueueMapping userQueueMapping2 = QueueMappingBuilder.create() .type(QueueMapping.MappingType.USER) - .source("user2") + .source("a1") .queue("%primary_group") .build(); // u:b4:%secondary_group QueueMapping userQueueMapping3 = QueueMappingBuilder.create() .type(QueueMapping.MappingType.USER) - .source("b4") + .source("e") .queue("%secondary_group") .build(); @@ -507,11 +507,11 @@ public void testFixedUserWithDynamicGroupQueue() throws Exception { ApplicationPlacementContext ctx = r.getPlacementForApp(asc, "user1"); assertEquals("Queue", "b1", ctx.getQueue()); - ApplicationPlacementContext ctx1 = r.getPlacementForApp(asc, "user2"); - assertEquals("Queue", "user2group", ctx1.getQueue()); + ApplicationPlacementContext ctx1 = r.getPlacementForApp(asc, "a1"); + assertEquals("Queue", "a1group", ctx1.getQueue()); - ApplicationPlacementContext ctx2 = r.getPlacementForApp(asc, "b4"); - assertEquals("Queue", "b4subgroup1", ctx2.getQueue()); + ApplicationPlacementContext ctx2 = r.getPlacementForApp(asc, "e"); + assertEquals("Queue", "esubgroup1", ctx2.getQueue()); } finally { if (mockRM != null) { mockRM.close();