[
https://issues.apache.org/jira/browse/YARN-2495?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Naganarasimha G R updated YARN-2495:
------------------------------------
Attachment: YARN-2495.20150309-1.patch
Hi [~wangda],
Attaching the updated patch and find the status of the comments :
1,2,3,4 : all have been rectified, method name having "set" in the begin and
the end was not sounding appropriate, hence modified it to
areNodeLabelsSetInReq and setAreNodeLabelsSetInReq in both heartbeat and
Register
5) ??I think we may not need to check centralized/distributed configuration
here, centralized/distributed is a config in RM side.??
Earlier had moved this configuration type check in the NM.serviceInit (l)
before calling {{getNodeLabelsProviderService}} But now changed the method name
to {{createNodeLabelsProviderService}} and moved back the check inside this
method itself. As part of the YARN-2729 will be returning Script based node
label provider in the createNodeLabelsProviderService method.
??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.??
In current patch, null will be set only if configuration type is set as
centralized in NM and based on earlier(other jira) feedback from Vinod, i think
we need to fail fast and let the user know the error at the earliest, so script
node label provider will throw exception on erroneous conditions like script
not configured,no rights to execute etc.. and ensure NM will fail to start.
??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??
Actually dint get the intent of these 2 lines and felt like comment was not
complete... Is it you want to avoid check of configuration type in NM and move
it script node label provider or something ?
6) has been rectified, was added while analyzing test case failure.
7) ??isDistributedNodeLabels seems not so necessary here, and if you agree with
5), it's better to remove the field??
IIUC point5 was related to NM initializing the provider and point7 is related
to NodeStatusUpdaterImpl if so i dint get the relation. can you please clarify
these 2 points
8) ??Add null check or comment (provider returned node labels will always be
not-null, for areNodeLabelsUpdated in NodeStatusUpdaterImpl??
Before calling areNodeLabelsUpdated, i had already checked for null and set
empty labels @ line 626 (startStatusUpdater method)
9)??Since we already have TestNodeStatusUpdater, it's better to merge
TestNodeStatusUpdaterForLabels to it.??
Well there was already too many internal classes extending
NodeStatusUpdaterImpl and ResourceTrackerService. And personally felt very very
difficult to walk through the test case and try to reuse it and class had
already crossed 1666 lines of code and hence as it was loosing readability
added a new class. Please inform if required will merge it to the existing
class only
10) Have modified ResourceTrackerService based on ur comments and have pushed
some common code in register and Heartbeat to the common method.
All findbugs issues are not related to my modifications and following test case
failure is not related to my modification
TestRMRestart.testRMRestartGetApplicationList.
Also will be uploading a patch for 2729 to get the view of complete flow and
also should will be testable
> 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.20150309-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)
