[
https://issues.apache.org/jira/browse/YARN-1864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13991168#comment-13991168
]
Sandy Ryza commented on YARN-1864:
----------------------------------
Thanks Ashwin, it's looking good, a few more comments (mostly stylistic nits):
Do we need FSQueueType.PARENT_WITHOUT_CONFIGURED_LEAF as a separate queue type
from PARENT? It seems like the one place it's used is
{code}
+ // At this point all leaves and 'parents with at least one child' would
have been created.
+ // Now create parents with no configured leaf.
+ for (String name : queueConf.getConfiguredQueues().get(
+ FSQueueType.PARENT_WITHOUT_CONFIGURED_LEAF)) {
+ if (removeEmptyIncompatibleQueues(name, FSQueueType.PARENT)) {
+ getParentQueue(name, true);
+ }
+ }
{code}
getParentQueue is idempotent and updateAllocationConfiguration happens rarely
enough that avoiding iterating over all the parent queues isn't worth the extra
code complexity.
{code}
+ Map<FSQueueType, Set<String>> configuredQueues = new HashMap<FSQueueType,
Set<String>>();
{code}
Over 80 characters
{code}
+ newPlacementPolicy = QueuePlacementPolicy
+ .fromXml(placementPolicyElement,
+ configuredQueues, conf);
{code}
Can fit this on fewer lines
{code}
+ Map<FSQueueType,Set<String>> configuredQueues)
{code}
{code}
+ private final Map<FSQueueType,Set<String>> configuredQueues;
{code}
Need space after comma
{code}
+ // if queue exists and is a parent queue, return it if
+ // the caller asked for parent,if not return null
+ return queueType == FSQueueType.PARENT ? queue : null;
+ } else if (queue instanceof FSLeafQueue) {
+ // if queue exists and is a leaf queue, return it if
+ // the caller asked for leaf,if not return null
+ return queueType == FSQueueType.LEAF ? queue : null;
+ }
{code}
Can these be moved up into the getLeafQueue and getParentQueue methods?
{code}
+ * Make way for the given leaf or parent queue if possible, by removing
incompatible
{code}
"Make way for the given queue"
{code}
+ public boolean isTerminal() {
+ return false;
+ }
{code}
Should be terminal if the nested rule is terminal?
> Fair Scheduler Dynamic Hierarchical User Queues
> -----------------------------------------------
>
> Key: YARN-1864
> URL: https://issues.apache.org/jira/browse/YARN-1864
> Project: Hadoop YARN
> Issue Type: New Feature
> Components: scheduler
> Reporter: Ashwin Shankar
> Labels: scheduler
> Attachments: YARN-1864-v1.txt, YARN-1864-v2.txt, YARN-1864-v3.txt,
> YARN-1864-v4.txt, YARN-1864-v5.txt
>
>
> In Fair Scheduler, we want to be able to create user queues under any parent
> queue in the hierarchy. For eg. Say user1 submits a job to a parent queue
> called root.allUserQueues, we want be able to create a new queue called
> root.allUserQueues.user1 and run user1's job in it.Any further jobs submitted
> by this user to root.allUserQueues will be run in this newly created
> root.allUserQueues.user1.
> This is very similar to the 'user-as-default' feature in Fair Scheduler which
> creates user queues under root queue. But we want the ability to create user
> queues under ANY parent queue.
> Why do we want this ?
> 1. Preemption : these dynamically created user queues can preempt each other
> if its fair share is not met. So there is fairness among users.
> User queues can also preempt other non-user leaf queue as well if below fair
> share.
> 2. Allocation to user queues : we want all the user queries(adhoc) to consume
> only a fraction of resources in the shared cluster. By creating this
> feature,we could do that by giving a fair share to the parent user queue
> which is then redistributed to all the dynamically created user queues.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
