[ https://issues.apache.org/jira/browse/YARN-10102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17084989#comment-17084989 ]
Peter Bacsko edited comment on YARN-10102 at 4/16/20, 3:33 PM: --------------------------------------------------------------- So, did review the patch, some comments. 1. I can see that this is a new kind of mapping besides user ("u") and group ("g"). It would be more flexible if we had it like this: {{u:%user%:%specified}} {{g:group1:%specified}} So instead of introducing a new mapping type, I'd prefer to have this as a placeholder. 2. Recently a large enhancement has been submitted to trunk (YARN-9879), that is, you can use multiple leaf queues with the same name (eg. "root.users.alice" and "root.admins.alice" are both valid, which was not the case before). But there's also backward compatibility, so you can still reference a queue with its leaf name only (as long as it's unique). Also, queues have parents, which can be normal parent queues or managed parents. This brings to the following scenarios: # Submitted queue string "alice". Parent is not known it has to be looked up. # Submitted queue string is "root.admins.alice". We know immediately that the parent is "root.admins". After we retrieved the parent, there are still two possibilities: # Parent is managed parent (instance of {{ManagedParentQueue}}). In this case, just return {{getPlacementContext(mapping, queueName)}} because the queue will be created if it doesn't exist. # Parent is not managed. In this case, you have to check if the full path actually exists. If it does, return {{getPlacementContext(mapping, queueName)}} otherwise return "null" because the queue cannot be created. So I've been thinking somethin like this (if the mapping type is "user": {noformat} // Need to pass queue from ApplicationSubmissionContext, see getPlacementForApp() private ApplicationPlacementContext getPlacementForUser(String user, String targetQueue) [...] } else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) { return getContextForSecondaryGroup(user, mapping); } else if (mapping.getQueue().equals(SPECIFIED_MAPPING)) { <-- new mapping return getContextForSpecified(targetQueue, mapping); } else { return getPlacementContext(mapping); } [...] private ApplicationPlacementContext getContextForSpecified(String targetQueue, QueueMapping mapping) throws IOException { String parentQueueStr = null; CSQueue csParentQueue = null; CSQueue csTargetQueue = null; if (targetQueue.startsWith("root")) { // full path parentQueueStr = getParentFromString(targetQueue); // implement this csParentQueue = queueManager.getQueue(parentQueueStr); csTargetQueue = queueManager.getQueue(targetQueue); } else { parentQueueStr = getParentFromLeafName(targetQueue); // implement this csParentQueue = queueManager.getQueue(parentQueueStr); // this method should work for short name too csTargetQueue = queueManager.getQueue(targetQueue); } // ManagedParent, just return whatever defined in the submission context if (csParentQueue instanceof ManagedParent) { getPlacementContext(mapping, targetQueue); } else { // Otherwise we have to make sure that it exists if (csTargetQueue != null) { getPlacementContext(mapping, targetQueue); } else { // Queue doesn't exist and cannot be created return null; } } } {noformat} I haven't tested this at all, but *in theory* this is what we need. It's a bit more complicated but I believe this is the correct approach. cc [~prabhujoseph] [~maniraj...@gmail.com] was (Author: pbacsko): So, did review the patch, some comments. 1. I can see that this is a new kind of mapping besides user ("u") and group ("g"). It would be more flexible if we had it like this: {{u:%user%:%specified}} {{g:group1:%specified}} So instead of introducing a new mapping type, I'd prefer to have this as a placeholder. 2. Recently a large enhancement has been submitted to trunk (YARN-9879), that is, you can use multiple leaf queues with the same name (eg. "root.users.alice" and "root.admins.alice" are both valid, which was not the case before). But there's also backward compatibility, so you can still reference a queue with its leaf name only (as long as it's unique). Also, queues have parents, which can be normal parent queues or managed parents. This brings to the following scenarios: # Submitted queue string "alice". Parent is not known it has to be looked up. # Submitted queue string is "root.admins.alice". We know immediately that the parent is "root.admins". After we retrieved the parent, there are still two possibilities: # Parent is managed parent (instance of {{ManagedParentQueue}}). In this case, just return {{getPlacementContext(mapping, queueName)}} because the queue will be created if it doesn't exist. # Parent is not managed. In this case, you have to check if the full path actually exists. If it does, return {{getPlacementContext(mapping, queueName)}} otherwise return "null" because the queue cannot be created. So I've been thinking somethin like this (if the mapping type is "user": {noformat} // Need to pass queue from ApplicationSubmissionContext, see getPlacementForApp() private ApplicationPlacementContext getPlacementForUser(String user, String targetQueue) [...] } else if (mapping.getQueue().equals(SECONDARY_GROUP_MAPPING)) { return getContextForSecondaryGroup(user, mapping); } else if (mapping.getQueue().equals(SPECIFIED_MAPPING)) { <-- new mapping return getContextForSpecified(targetQueue, mapping); } else { return getPlacementContext(mapping); } [...] private ApplicationPlacementContext getContextForSpecified(String targetQueue, QueueMapping mapping) throws IOException { String parentQueueStr = null; CSQueue csParentQueue = null; CSQueue csTargetQueue = null; if (targetQueue.startsWith("root")) { // full path parentQueueStr = getParentFromString(targetQueue); // implement this csParentQueue = queueManager.getQueue(parentQueueStr ); csTargetQueue = queueManager.getQueue(targetQueue); } else { parentQueueStr = getParentFromLeafName(targetQueue); // implement this csParentQueue = queueManager.getQueue(parentQueueStr ); // this method should work for short name too csTargetQueue = queueManager.getQueue(targetQueue); } // ManagedParent, just return whatever defined in the submission context if (parentQueue instanceof ManagedParent) { getPlacementContext(mapping, targetQueue); } else { // Otherwise we have to make sure that it exists if (csTargetQueue != null) { getPlacementContext(mapping, targetQueue); } else { // Queue doesn't exist and cannot be created return null; } } } {noformat} I haven't tested this at all, but *in theory* this is what we need. It's a bit more complicated but I believe this is the correct approach. cc [~prabhujoseph] [~maniraj...@gmail.com] > Capacity scheduler: add support for %specified mapping > ------------------------------------------------------ > > Key: YARN-10102 > URL: https://issues.apache.org/jira/browse/YARN-10102 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Peter Bacsko > Assignee: Tanu Ajmera > Priority: Major > Attachments: YARN-10102-001.patch > > > The reduce the gap between Fair Scheduler and Capacity Scheduler, it's > reasonable to have a {{%specified}} mapping. This would be equivalent to the > {{<specified>}} placement rule in FS, that is, use the queue that comes in > with the application submission context. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org