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 
 - 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:
1144          Queue  queue = attempt.getQueue();
1145          CSQueue csQueue = queue instanceof CSQueue
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 

> 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

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to