Wangda Tan commented on YARN-3362:

Hi Naga,
Thanks for updating, I took a look at closer look at latest patch, and tried in 
my local cluster, some comments in code:
1) There's some duplicated code for CapacitySchedulerInfo, you only need keep 
one constructor, which takes label always
2) Also, {{protected CapacitySchedulerQueueInfoList getQueues}} doesn't need to 
do specifial logic when {{null == nodeLabel}}, queue can be ignored when: 
{{queue cannot access to label AND label is exclusive-label}}. 
accessibleToPartition in AbstractCSQueue already considered when label==""
3) CapacitySchedulerQueueInfo doesn't need two constructor as well
4) When you want to get "NO_LABEL", you should pass 
"RMNodeLabelsManager.NO_LABEL" instead of null.

Some comments in UI:
1) I've reconsidered the hierarchy, I think it's better to rename 
"Node_Label=<...>" to "Partition:<...>". For the Node Labels page, we should 
add a column which is "Node Label Type", and indicate they're partition. 
2) <NO_LABEL> should be <DEFAULT_PARTITION>, <NO_LABEL> itself may not clear 
enough, also in Node Labels page, we should change <NO_LABEL> to the same name 
here. Changes in Node Labels page could add in separated JIRA.
3) It's better to indicate queue in hierarchy also, now we have:
+ Partition: xxx
  + root
     + a
     + b
I think it's better to add a "Queue:" before queue name:
+ Partition: xxx
  + Queue: root
     + Queue: a
     + Queue: b
Otherwise people may get confused which one is partition and which one is queue.

4) In queue's metrics overview table, it mixing partition-specific and 
queue-generic metrics is not clear enough, I'm not sure if you can show it like 
this (If existing Hamlet framework supports this or not):
+ Partition: xxx
  + Queue: root
     + Queue: a
       |---------  Partition Specific Metrics -----------|
       |  Used Capacity      |    0.0%                   |
       |  Absolute Capacity  |   50.0%                   |

       |---------  Queue General Metrics ----------------|
       |          Other Metrics                          |

       Active user info:
       ........ Table ..........

     + Queue: b

> Add node label usage in RM CapacityScheduler web UI
> ---------------------------------------------------
>                 Key: YARN-3362
>                 URL: https://issues.apache.org/jira/browse/YARN-3362
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: capacityscheduler, resourcemanager, webapp
>            Reporter: Wangda Tan
>            Assignee: Naganarasimha G R
>         Attachments: CSWithLabelsView.png, Screen Shot 2015-04-29 at 11.42.17 
> AM.png, YARN-3362.20150428-3-modified.patch, YARN-3362.20150428-3.patch, 
> YARN-3362.20150506-1.patch, capacity-scheduler.xml
> We don't have node label usage in RM CapacityScheduler web UI now, without 
> this, user will be hard to understand what happened to nodes have labels 
> assign to it.

This message was sent by Atlassian JIRA

Reply via email to