[
https://issues.apache.org/jira/browse/YARN-10506?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17263672#comment-17263672
]
Wangda Tan commented on YARN-10506:
-----------------------------------
Thanks for all updates, here's my comments:
1) CapacityScheduler:
Minor:
autoCreateLeafQueue should be moved to autoQueueHandler.
We can do this in a follow up patch, because now we have two places to handle
the auto queue creation, and ideally "autoQueueHandler" should be responsible
for that.
The same follow up patch should also clean up addQueue() method of
ResourceScheduler. It is only used by CapacitySchedulerPlanFollow, we don't
need to add it to the abstract class.
{code}
LeafQueue lq = autoQueueHandler.autoCreateQueuePath(placementContext);
{code}
Return value is not used.
2) CapacitySchedulerAutoQueueHandler:
Major:
2.1) I think the existing logic is to create auto queue based on
ApplicationPlacementContext, but when we do queue mapping, we need to indicate
if auto creation is allowed or not.
In the mapping rule, we have "create" flag, I think CSAutoQueueHandler should
consider the "create" flag.
Previously, in my patch, I added createLeafQueue and createParentQueue flag,
the latest patch removed the logic.
Can you share more thoughts on this? Any please let me know if I missed
anything.
Even though this logic can be done in a separate patch, but I still suggest to
handle it within this one for completeness.
(A side note, I noticed there's a method:
ParentQueue#isEligibleForAutoQueueCreation, it only handles "if a parent queue
can allow auto creation underneath or not", but cannot handle "if a creation
itself is allowed by placement policy or not").
2.2) autoCreateQueuePath doesn't look like atomic, it could create parent
first, but later found LeafQueue cannot be created:
{code}
if (parent instanceof ParentQueue) {
...
} else {
throw new SchedulerDynamicEditException(
"Could not auto-create leaf queue for " + queue.getQueue()
+ ". Queue mapping specifies an invalid parent queue "
+ "which does not exist"
+ queue.getParentQueue());
}
{code}
Can we make it atomic?
2.3) I think autoCreateParentHierarchy itself should be able to handle
LeafQueue creation, because itself can handle multiple leaves, we don't have to
maintain a separate LeafQueue creation logic:
{code}
ParentQueue parentQueue = (ParentQueue) parent;
LeafQueue leafQueue = parentQueue.addDynamicLeafQueue(
queue.getFullQueuePath());
queueManager.addQueue(leafQueue.getQueuePath(), leafQueue);
return leafQueue;
{code}
Minor:
- Rename autoCreateQueuePath to autoCreateQueue (we will never create a "queue
path")
- CSQueueUtils#extractQueuePath should be moved to CSAutoQueueHandler. (It
won't be used by other classes). And rename extractQueuePath to
extractApplicationPlacementContext (we don't just extract path)
3) CapacitySchedulerConfiguration change:
Major:
- Now we added queue-path.auto-queue-creation.enabled flag, which is in
parallel of auto-create-child-queue.enabled flag.
First of all, it is very confusing because there're two parameters looks
similar.
Second, We still need to check the weight mode is configured or not.
I'm actually thinking to get rid of the flag, and completely relying on
ParentQueue#addDynamicChildQueue to do the weights check: If a parent queue has
children use weight, we can proceed with queue creation.
We can improve the flag later, thoughts?
4) ParentQueue#addDynamicChildQueue:
Major:
- The logic ParentQueue#getCapacityConfigurationTypeForQueues returns PERCENT
when there's no children under the parent. For that case, I think we should
specially handle it inside getCapacityConfigurationTypeForQueues:
{code}
Which should return WEIGHT when Collection<CSQueue> queues isEmpty
{code}
And we should add an unit test to add queue under a static parent queue which
doesn't have children, because it will be a common case when user use the
feature.
> Update queue creation logic to use weight mode and allow the flexible
> static/dynamic creation
> ---------------------------------------------------------------------------------------------
>
> Key: YARN-10506
> URL: https://issues.apache.org/jira/browse/YARN-10506
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Benjamin Teke
> Assignee: Andras Gyori
> Priority: Major
> Attachments: YARN-10506-006-10504-010.patch,
> YARN-10506-007-10504-010.patch, YARN-10506-008.patch, YARN-10506.001.patch,
> YARN-10506.002.patch, YARN-10506.003.patch, YARN-10506.004.patch,
> YARN-10506.005.patch, YARN-10506.006-combined.patch, YARN-10506.006.patch,
> YARN-10506.007.patch, YARN-10506.009.patch
>
>
> The queue creation logic should be updated to use weight mode and support the
> flexible creation.
--
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
