[
https://issues.apache.org/jira/browse/YARN-9879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17052616#comment-17052616
]
Wangda Tan commented on YARN-9879:
----------------------------------
Thanks [~shuzirra] for the monster patch!
Took a quick look at the patch, overall looks good. (I skipped the hardest
queue mapping module to leave to other folks to review).
1) Make sure commented code is not part of the final patch.
2) CSQueueStore:
- Only fullNameQueues is ConcurrentHashMap, is it intentional?
- getByShortName can be converted to private method, and the
{{CapacitySchedulerQueueManager#getQueueByShortName}} is not used, can be
removed.
- Instead of Synchronized lock, I suggest to use ReadWriteLock, the method
like {{get}} is not safe since it access multiple fields. There's very
infrequent write to queue map comparing to read.
3) CapacityScheduler.java:
{code:java}
1144 Queue queue = attempt.getQueue();
1145 CSQueue csQueue = queue instanceof CSQueue
{code}
This check is uncessceary. When CS is enabled, all queues in the RM is CSQueue.
4) CapacitySchedulerConfigValidator.java:
validateQueueHierarchy: Have mixed usage of queueName and queuePath, suggest
to move to queuePath for less ambiguous.
5) There're 18 TODOs in the patch, I suggest to mark "must-to-fix" TODOs to
FIXME, in most cases TODO means we will never do it. :). In Hadoop there're 731
TODOs.
> Allow multiple leaf queues with the same name in CS
> ---------------------------------------------------
>
> Key: YARN-9879
> URL: https://issues.apache.org/jira/browse/YARN-9879
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Gergely Pollak
> Assignee: Gergely Pollak
> Priority: Major
> Labels: fs2cs
> Attachments: CSQueue.getQueueUsage.txt, DesignDoc_v1.pdf,
> YARN-9879.POC001.patch, YARN-9879.POC002.patch, YARN-9879.POC003.patch,
> YARN-9879.POC004.patch, YARN-9879.POC005.patch, YARN-9879.POC006.patch,
> YARN-9879.POC007.patch, YARN-9879.POC008.patch, YARN-9879.POC009.patch,
> YARN-9879.POC010.patch, YARN-9879.POC011.patch
>
>
> Currently the leaf queue's name must be unique regardless of its position in
> the queue hierarchy.
> Design doc and first proposal is being made, I'll attach it as soon as it's
> done.
--
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
