Attaching the WIP patch for this part of the issue...

Hi Tan, Wangda
As per our discussion I have done the scriptNodeLabelsPRovider similar to the NodeHealthCheckerService. And moved the yarn configuration changes related to the script into this jira itself. Please review

Hi Naganarasimha G R,
After looked at the patch, and implementation of NodeHealthMonitorExecutor, I think it may not proper to put a "common" class for them to yarn.utils. Reasons are:

1. We already have a ShellCommandExecutor, that is enough to run a general script, and set timeout, fetch output, etc.
2. The purpose of NodeHealthScriptRunner and NodeLabelScriptRunner (The name in my mind ) are different: NodeHealthScriptRunner needs do a lot of checks for exception because it need to tell RM what actual happens, but NodeLabelScriptRunner doesn't need so, just a list of successfully fetched labels or no, if failed to run script, log errors to log should be enough.

So I suggest do to this like NodeHealthScriptRunner, and remove what we don't need, it should be able to,

1. If the script is failed or timeout or exception happened, and log them properly
2. Upon successfully execute script, parse output for labels. I think we can define, lines start with "NODE_LABELS:" are node labels, and splited by ",". Like:
   NODE_LABELS:java, windows\n
   ...(other messages)
   NODE_LABELS:gpu,x86
   The result should be {"java", "windows", "gpu", "x86"}

   .
   And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM.

Does this make sense to you?

And in addition, NM_SCRIPT_LABELS_PROVIDER_PREFIX -> NM_NODE_LABELS_SCRIPT_PROVIDER_PREFIX is more clear to me, similar to others

Thanks,
Wangda

Hi Tan, Wangda,

And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM.

Two concerns if we do this:
1. Two times labels will get validated, once in NM and once in RM(NodeLabelsManager.replaceNodeLabels ). And anyway as the exception message is planned to be propagated till NM and getting logged in NM, would this be required.?
2. Currently checkAndThrowLabelName is validating and throwing exception for a singe label, i think it would better to accept a collection and show that which all labels in the collection are not valid. So that user need not wait till multiple timer runs to identify all the set of valid labels. so was planning to rename the method to $$ validateLabels(Collection<String> labels) $$ and make it static to get it accessible for (conf & script)NodeLabelsProvider (if this basic validation is required in NM too)

Hi Naganarasimha G R,
For 1.
I think what I meant is just check label name locally in NM.
If NM register/heartbeat with RM failed with labels, I should have commented in YARN-2495: https://issues.apache.org/jira/browse/YARN-2495?focusedCommentId=14184146&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14184146.

use a flag say if the last sync about node labels is success or not

This should be also your proposal

For 2.
I think for now, let's keep it simple, I just don't want to change too much for what we have in NodeLabelsManager . As our discussion in YARN-2495, in the future we might need return rejected labels. So we can change this that that time. What do you think?

Thanks,
Wangda

Updated with review comments and test cases

submitting patch

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12678261/YARN-2729.20141031-1.patch
against trunk revision d1828d9.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5653//console

This message is automatically generated.

Hi Naganarasimha G R,
IIRC, the script based patch should be based on YARN-2495, and we should create a script-based labels provider extend NodeLabelsProviderService, correct? But I haven't seen much relationship between this and YARN-2495 besides configuration options.

Please let me know if I understood incorrectly.

Thanks,
Wangda

Its dependent on YARN-2495 and once that is commited will enable patch for this

Updating patch with Tan, Wangda's review comments

Re basing the patch and incorporating the changes based on yarn-2923 (changes in yarn configuration)

Hi Tan, Wangda
Rebasing the patch
Removing dependency on yarn-2923,
Changed configuration names to suit current conf suggestions
Have made it to fail fast on invalid configurations.

If the above modifications are fine then will start looking into changes required to make HadoopCommon's NodeHealthScriptRunner to make it reuseable. (In seperate jira)

Hi Tan, Wangda,
Have rebased the patch based on changes in yarn-2495,
one open point, whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12706372/YARN-2729.20150322-1.patch
against trunk revision 1a495fb.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7157//console

This message is automatically generated.

Hi Tan, Wangda,
Not sure the reason for failure in applying the patch ...I am able to do this in local setup (succeded after fuzz offset 65 lines), so i am rebasing it and uploading it.
Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ?
Also provide your feedback on earlier discussion :
whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12708567/YARN-2729.20150401-1.patch
against trunk revision 7610925.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

org.apache.hadoop.yarn.server.nodemanager.TestNodeManager

Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7182//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7182//console

This message is automatically generated.

Hi Naganarasimha G R,
Sorry for my late response,

Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ?

Interval-ms is configurable as we decided, I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched.

whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority).

And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch?

Thanks Tan, Wangda for the review,

I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched.

yes its configurable, but was just trying to avoid another thread of timer if not required and also was checking with you as i was not sure that it would be a better approach to take some decision based on the value configured for interval-ms (0 or -1 to not to launch the timer task) and i dint want to add a new configuration to enable or disable the timer

You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority).

yep felt so... hence dint further work on it.

And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch?

my mistake will update along with the conclusion for the first point ....

i dint want to add a new configuration to enable or disable the timer

Agree,

I think acceptable values of interval-ms should be -1 (do not launch timer) and > 0, other values should be illegal.

Attaching a patch to disable the timer and include the missing files

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch
against trunk revision c94d594.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
org.apache.hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA

The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
org.apache.hadoop.hdfs.TestLeaseRecovery2

Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7193//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7193//console

This message is automatically generated.

Testcase failures not related to my jira

Apparently Jenkins ran wrong tests, rekicked Jenkins.

Some comments:
1) Configuration:
Instead of distributed_node_labels_prefix, do you think is it better to name it : "yarn.node-labels.nm.provider"? The "distributed.node-labels-provider" doesn't clearly mentioned it runs in NM side.

I don't want to expose class to config unless it is necessary, now we have two options, one is script-based and another is config-based. We can set the two as "white-list", if a given value is not in whitelist, trying to get a class from the name. So the option will be: yarn.node-labels.nm.provider = "config/script/other-class-name".

Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here).
If you agree above, the name could be: yarn.node-labels.nm.provider-fetch-interval-ms (and provider-fetch-timeout-ms)

And script-related options could be:
yarn.node-labels.nm.provider.script.path/opts

2) Implementation of ScriptBasedNodeLabelsProvider
I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc.
Can you make an abstract class and inherited by ScriptBased?

DISABLE_TIMER_CONFIG should be a part of YarnConfiguration, all default of configurations should be a part of YarnConfiguration.

canRun -> something like verifyConfiguredScript, and directly throw exception when something wrong (so that admin can know what really happened, such as file not found, doesn't have execution permission, etc.), and it should be private non-static.

checkAndThrowLabelName should be called in NodeStatusUpdaterImpl

label need to be trim() when called checkAndThrowLabelName(...)

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch
against trunk revision 6a6a59d.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 2 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7205//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7205//console

This message is automatically generated.

Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here).

agree, i dont mind redoing, as long as its for better reason and i was expecting for changes here anyway.
For other comments on configuration will get it done,

I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc. Can you make an abstract class and inherited by ScriptBased?

I can do this (which i feel is correct), but if we do this then it might not be possible to generalize much NodeHealthSCriptRunner and ScriptBasedNodeLabelsProvider, which i feel should be ok

checkAndThrowLabelName should be called in NodeStatusUpdaterImpl

In a way it would be better in NodeStatusUpdaterImpl as we support external class to be a provider, but earlier thought it would not be good for additional checks as part of heart beat flow

label need to be trim() when called checkAndThrowLabelName(...)

Not required as checkAndThrowLabelName takes care of it, but missing test case will add it for NodeStatusUpdaterImpl
Other issues will rework in next patch

Hi Tan, Wangda,
Uploading the patch with fixing the review comments given earlier...

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12709432/YARN-2729.20150404-1.patch
against trunk revision ef591b1.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 3 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

-1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7219//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/7219//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7219//console

This message is automatically generated.

Tan, Wangda ,
Findbugs are not related to this jira ...

Hi Tan, Wangda, can you please take a look @ the reworked patch ... ?

Tan, Wangda, any updates on this patch required ?

Tx for working on this Naganarasimha G R!

Comments on the patch

YarnConfiguration

• The configuration properties should be yarn.nodemanager.node-labels.* instead of yarn.node-labels.nm.*
• SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes.
• NODE_LABELS_PROVIDER can be removed as you already have NODE_LABELS_PROVIDER_PREFIX, delete it.
• None of the prefix constants need to be public. I think it was a mistake all along, we don't need to continue that.
• DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration
• Some rename suggestions for the configs
  • DISTRIBUTED_NODE_LABELS_PREFIX -> NM_NODE_LABELS_PREFIX
  • NODE_LABELS_PROVIDER -> NM_NODE_LABELS_PROVIDER
  • Similarly change to NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS
    NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS
    DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS
    DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS
    NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PREFIX
    NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PATH
    NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_SCRIPT_OPTS

AbstractNodeLabelsProvider

• LOG is not used anywhere
• Add @Override annotations to the extended methods from the parent.
• serviceInit() should call super's serviceInit()
  ScriptBasedNodeLabelsProvider
• NodeLabelsScriptRunner can access scriptTimeout from the parent class but not scriptArgs. I think you should simply access scriptArgs directly and drop it from NodeLabelsScriptRunner's constructor.
• I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.
• Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider.
  Can you add the documentation for setting this up too too?

BTW, assuming YARN-3565 goes in first, you will have to make some changes here.

Thanks for the review comments Vinod Kumar Vavilapalli,

SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes

Actually these are not preffixes, as per Tan, Wangda's comment we had decided have whitelisting for provider : The option will be: yarn.node-labels.nm.provider = "config/script/other-class-name". . These are modifications for it.

DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration

Well As per one of the wangda's comment he had mentioned that possible values or default values of configurations had to be kept in YARNConfiguration, hence had placed it here, if required as per your comment can move it to AbstractNodeLabelsProvider

LOG is not used anywhere

Are the logs expected when the labels are set in setNodeLabels ? i can add here but anyway there were logs in NodeStatusUpdaterImpl on successfull and unsuccessfull attempts.

BTW, assuming YARN-3565 goes in first, you will have to make some changes here.

I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.

Well was thinking about this while working on YARN-3565, but dint modify the NodeLabelsProvider as currently Labels(currently partitions) which needs to be sent from NM have to be one of RM's CLUSTER NodeLabel set. So exclusiveness need not be sent from NM to RM as currently we support specifying the exclusiveness only during adding clusterNode labels. So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there?
Tan, Wangda and you can share your opinion on this, based on it will do the modifications.

Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider.

Well as per the class reponsibility i understand that NodeStatusUpdaterImpl is not supposed to have it but as it might be expected to be public we had to ensure that

• For every heartbeat labels are sent across only if modified
• doing basic validations before sending the modified labels

These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider.
Those changes were also from my previous patch, so no hard feelings in taking care of it if req .

Can you add the documentation for setting this up too too?

Well was planning to raise jira for updating documentation on top of NodeLabels but documentation for it is not yet completed. If required can just add some pdf here

Hi Tan, Wangda

1. rebased the patch on top of 3565
2. Moved common code which was earlier here to YARN-2923, as 2923 jira will be going first

I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.

So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there? Tan, Wangda and you can share your opinion on this, based on it will do the modifications.

I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future?

These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider.

Isn't AbstractNodeLabelsProvider a good place to do these steps?

Thanks Vinod Kumar Vavilapalli for replying,

I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future?

True, its better to think about it for both script(YARN-2729) and config (YARN-2923) based providers now itself if we are making it as public. Initial thought what i have is

NODE_LABELS=<Label Name>|<Label Type>[,Labels]
where Label Type = Partition, Constraint....
and default if not specified can be Partition.

Going further i think distributed labels will be more suitable for constraints/attribute YARN-3409 so we can think of having Constraint as default too. Also we need not specify Whether Partition is Exclusive or Non-Exclusive, it does not make significance from the NM as Exclusivity of partition labels are already specified while adding it to cluster labels set in RM.
One More suggestion is In Node Label object, can we think of having Enum instead of isExclusive, Enum can have ExclusivePartion,NonExclusivePartition, (Constraint in future) and so on.

Isn't AbstractNodeLabelsProvider a good place to do these steps?

Well AbstractNodeLabelsProvider will only be applicable to the whitelisted providers (Config and script) currently, And also the purpose was for removing duplicate code related to Timertask and related configs. So is your suggestion to expose the AbstractNodeLabelsProvider to be a public interface ? or can we think of having intermediate manager class and have configurations for timer requirement and leave the NodeLabelsProvider interface as is. ?

Hi Tan, Wangda & Vinod Kumar Vavilapalli,
Any further thoughts on the above comments ?

Attaching a patch to sync with the changes of YARN-2923.

-1 overall

This message was automatically generated.

Hi Tan, Wangda & Rohith Sharma K S,
Please find the attached patch with the offline discussed modifications

For Config :
Currently we are supporting configuration param : "yarn.nodemanager.node-labels.provider.configured-node-labels"
so instead can we have = > yarn.nodemanager.node-labels.provider.configured-node-partition which accepts only single partition and in future we can add yarn.nodemanager.node-labels.provider.configured-node-constraints which can support multiple constraints

For Script :
Currently in the patch i am searching for lines starting with "NODE_LABELS:" in script output and labels separated by "," though we currently support only a single Partition per node.
So my suggestion we can currently support for parsing lines starting with "NODE_PARTITION:" and send the string following this pattern as Node Label. Later in future we can support for "NODE_CONSTRAINTS:" and accept multiple constraints for it.

-1 overall

This message was automatically generated.

TestNodeStatusUpdaterForLabels is passing locally and check style is for yarnConfiguration file size so not much can be done for it. Rohith Sharma K S can you please take a look at this jira ?

Hi Naganarasinmha, thanks for delivering the patch.
Some doubts and somments

1. Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider?
2. Does serviceStart in ScriptBasedNodeLabelsProvider really required since you are calling super.serviceStart();? I think it can be removed. The below code runs twice or attempt to run twice.

    @Override
     protected void serviceStart() throws Exception {
       super.serviceStart();
       if (intervalTime == DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER) {
         // run at-least once so that labels are set
         timerTask.run();
       }
     }
   

3. In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent.

     protected void serviceStop() throws Exception {
       super.serviceStop();
       if (shexec != null) {
         Process p = shexec.getProcess();
         if (p != null) {
           p.destroy();
         }
       }
     }
   

4. Is there anywher documented how the script output should be?
5. The patch removes method convertToNodeLabelSet. I think some code optimization can be done keeping it. Currently, these removed lines are written in both providers.

Tests :

1. About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist.
2. When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error. Are you getting below error? But individual class run test is passing. I think some cleanup has issue.

   java.io.FileNotFoundException: D:\Hadoop\hadoop-yarn-project\hadoop-yarn\hadoop-yarn-server\hadoop-yarn-server-nodemanager\target\org.apache.hadoop.yarn.server.nodemanager.nodelabels.TestConfigurationNodeLabelsProvider-localDir\yarn-site.xml (The system cannot find the path specified)
   

Thanks for the review Rohith Sharma K S,

Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider?

Yes as it extends AbstractNodeLabelsProvider it supports periodic execution.

Is there anywher documented how the script output should be?

yes its documented in yarn-default.xml and javadocs for the class ScriptBasedNodeLabelsProvider.java. Do you have any other place in mind where it can be updated?

About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist.

Yes this is issue has been already raised as YARN-4169, but nutting to do in specific to this jira and was not actually reproducible in my local setup.

When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error

Not facing this issue, will sync offline with you on this.

Other issues have been handled in the patch along with one more minor issue which was got during testing, In ResourceTrackerService we are checking for distributed or delegated-centralized without checking for node labels enabled which caused unnecessary logs on RM side.

-1 overall

This message was automatically generated.

Hi Rohith Sharma K S

When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error

Thanks for pointing out this issue, Its happening only when the order of execution is ConfigNLP and ScriptNLP and the temp dir containing Yarn-Site.xml is getting deleted in @AfterClass

-1 overall

This message was automatically generated.

Hi Rohith Sharma K S,
Seems like test case failure is not related to the patch and the check style is related to number of lines in YARN Configuration, so i think jenkins output is fine, can you please take a look.

Overall patch look good to me.
I have one doubt/comment. If I am missing anything correct me,

1. NodeLabelsProvider is abstract class. Why it was not made as Interface?
2. And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable.
   Any thoughts? cc :/Wangda Tan

Adding to above

1. From the below description, I assume that NodeLabelsProvider instance can be created only if configuratin-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also.

   When node labels "yarn.node-labels.configuration-type" is of type "distributed" Administrators can configure the source of the node labels provider by configuring this parameter
   

Thanks for the comments Rohith Sharma K S,

1.NodeLabelsProvider is abstract class. Why it was not made as Interface?

Good point i think making it as interface should be sufficient and in NodeManager we can do addIfService(nodeLabelsProvider), this will will give the implementations of the NodeLabelsProvider to be much more flexible. will update current patch to incorporate these modifications shortly

2. And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable.

I presume you might of got it wrong over configuration of NodeLabelsProvider in NM. Basically there is whitelist of script and config and if this doesnt match then we are assuming its a qualified classname and try load it as you mentioned. Hence still its pluggable. correct me if i am wrong in understanding your comment here

From the below description, I assume that NodeLabelsProvider instance can be created only if configuration-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also.

This was a conscious decision which was taken to minimize the number of configurations on the NM side. It was sufficient to configure provider only as in NM, if configuration-type is configured and provider is not configured then we need to throw exception. configuration-type is mainly for the RM to deal with NM's HB/registrations and blocking CLI and REST interactions.

Attaching a patch to make NodeLabelsProvider as a interface instead of abstract class

-1 overall

This message was automatically generated.

+1 lgtm.. I will commit it after couple of days if there is no more comments on the patch.

Hi Naganarasimha G R and Rohith Sharma K S
Sorry for coming in late. I have some doubts here.

In ScriptBasedNodeLabelsProvider#serviceStop, shexec process is destroyed and the super.stop() is invoked. Hence I feel it may still be possible that shexec may again get executed (if one timer run happens in this timeframe) before timer is cancelled from AbstractNodeLabelsProvider#serviceStop. We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this). Thoughts?

couple of other comments:
1. ScriptBasedNodeLabelsProvider#serviceInit: verifyConfiguredScript is invoked with nodeLabelsScript which is a class variable. It may not be needed.
2. I have one doubt in fetchLabelsFromScriptOutput. scriptOutput is split with newline and each line is checked whether its starts with NODE_LABEL_PARTITION_PATTERN. But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel. I didnt understand this part. Could you please have a look on this.

Thanks Sunil G for the comments and better late than never !

We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this).

IMHO i feel the former approach is better than the latter as i am not inclined towards approach of having checks in the parts of code which gets frequently executed just to avoid corner case . Hence i am reverting the fix for the comment which was given by Rohith Sharma K S. IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ?

But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel. I didnt understand this part.

As discussed offline i have done these modifications as a node can belong to only one Partition Label and when we support Constraints we can support more patterns later.
Other issues i have fixed. If better approach is possible for first will rework.

-1 overall

This message was automatically generated.

Testcase failure are not related to the modifications in the patch and findbugs error when run manually reported this error which is also not related to the patch. Rohith Sharma K S can you please take a look ?

<file classname="org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl"><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.builder; locked 95% of time" lineNumber="390"/><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.proto; locked 94% of time" lineNumber="390"/><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.viaProto; locked 94% of time" lineNumber="390"/></file>

Hence i am reverting the fix for the comment which was given by Rohith Sharma K S. IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ?

The reason why I suggested in my earlier comment i.e In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent. is Consider NM has services A,B,C,D which are started in the order of ABCD. When stopping, the services are stopped in reverse order i.e D,C,B,A. Say, D has child class D1. If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end.

Better way to avoid all conflicts is create abstract method say cleanUp() in AbstractNodeLabelProvider. This method can be override in ScriptBasedNodeLabelsProvider. A sample change is like below

  @Override
  protected void serviceStop() throws Exception {
    if (nodeLabelsScheduler != null) {
      nodeLabelsScheduler.cancel();
    }
    cleanUp();
    super.serviceStop();
  }

Yes Rohith Sharma K S.
With this, child class will have the capability to clean up stuffs which can be done after time is cancelled.
So shExec.destroy() can be called from cleanUp() which is implemented/overridden in ScriptBasedNodeLabelsProvider

Thanks Rohith Sharma K S & Sunil G, for the comments . I have updated the patch with the approach specified. but i have a query.

If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end.

I could not get the above statement clearly, IIUC the order would be either D1,D,C,B,A when super.serviceStop() is called first and D,D1,C,B,A when super.serviceStop() is called at the last. Latter is what we are trying to achieve earlier, correct me if i am wrong.

-1 overall

This message was automatically generated.

+1 lgtm for the latest patch.. I will trigger jenkins again to verify QA report..

-1 overall

This message was automatically generated.

Hi Rohith Sharma K S, seems like jenkins report is fine, checkstyle issue is related to file length...

+1. Patch looks good. Thank you NGarla_Unused.

committed to branch-2/trunk.. Thanks Naganarasimha G R for contributions and patience!! Thanks Sunil G Wangda Tan Vinod Kumar Vavilapalli for review..

FAILURE: Integrated in Hadoop-trunk-Commit #8707 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8707/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java

FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #597 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/597/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java

FAILURE: Integrated in Hadoop-Mapreduce-trunk #2528 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2528/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java

Thanks Review and Commit , Rohith Sharma K S, Tan, Wangda & Sunil G.

SUCCESS: Integrated in Hadoop-Yarn-trunk #1321 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1321/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java

FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #585 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/585/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java

FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #538 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/538/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/CHANGES.txt

FAILURE: Integrated in Hadoop-Hdfs-trunk #2475 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2475/)
YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
• hadoop-yarn-project/CHANGES.txt
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
• hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java