[
https://issues.apache.org/jira/browse/YARN-2495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14349708#comment-14349708
]
Wangda Tan commented on YARN-2495:
----------------------------------
1) Changes of CommonNodeLalelsManager is uncessary
2) NodeHeartbeat's protobuf field name should consistent with java class method
name:
nodeLabels-> areNodeLabelsSet
3) NodeHeartbeatRequestPBImpl: getter/setter should consistent, like
get/setAreNodeLabelsSet
4) Same comment on RegisterNodeManagerRequestPBImpl
5) Integration with NM (NodeManager.java):
I think we may not need to check centralized/distributed configuration here,
centralized/distributed is a config in RM side. In NM side, it should be how to
get node labels, if user doesn't configure any script file for it, it should be
null and no instance of NodeLabelProviderService will be added to NM.
So back to code, you can just leave getNodeLabelsProviderService(..), which
will be implemented in YARN-2729.
If you agree, we need change the name {{isDistributedNodeLabelsConf}} to
6) Why change this to notifyAll? I think it may be unnecessary:
{code}
synchronized (this.heartbeatMonitor) {
this.heartbeatMonitor.notify();
}
{code}
7) isDistributedNodeLabels seems not so necessary here, and if you agree with
5), it's better to remove the field.
8) Add null check or comment (provider returned node labels will always be
not-null, for areNodeLabelsUpdated in NodeStatusUpdaterImpl
9) Since we already have TestNodeStatusUpdater, it's better to merge
TestNodeStatusUpdaterForLabels to it.
10) ResourceTrackerService:
When register and found illegal labels, this should happen before instanting
RMNodeImpl
Maybe you can still merge some common code for register/update?
Will include more tests reviewing in next turn. And could you take a look at
failed tests/findbugs, etc.?
> Allow admin specify labels from each NM (Distributed configuration)
> -------------------------------------------------------------------
>
> Key: YARN-2495
> URL: https://issues.apache.org/jira/browse/YARN-2495
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Wangda Tan
> Assignee: Naganarasimha G R
> Attachments: YARN-2495.20141023-1.patch, YARN-2495.20141024-1.patch,
> YARN-2495.20141030-1.patch, YARN-2495.20141031-1.patch,
> YARN-2495.20141119-1.patch, YARN-2495.20141126-1.patch,
> YARN-2495.20141204-1.patch, YARN-2495.20141208-1.patch,
> YARN-2495.20150305-1.patch, YARN-2495_20141022.1.patch
>
>
> Target of this JIRA is to allow admin specify labels in each NM, this covers
> - User can set labels in each NM (by setting yarn-site.xml (YARN-2923) or
> using script suggested by [~aw] (YARN-2729) )
> - NM will send labels to RM via ResourceTracker API
> - RM will set labels in NodeLabelManager when NM register/update labels
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
