Wangda Tan commented on YARN-2729:

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 = 

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:

*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 

checkAndThrowLabelName should be called in NodeStatusUpdaterImpl

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

> Support script based NodeLabelsProvider Interface in Distributed Node Label 
> Configuration Setup
> -----------------------------------------------------------------------------------------------
>                 Key: YARN-2729
>                 URL: https://issues.apache.org/jira/browse/YARN-2729
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager
>            Reporter: Naganarasimha G R
>            Assignee: Naganarasimha G R
>             Fix For: 2.8.0
>         Attachments: YARN-2729.20141023-1.patch, YARN-2729.20141024-1.patch, 
> YARN-2729.20141031-1.patch, YARN-2729.20141120-1.patch, 
> YARN-2729.20141210-1.patch, YARN-2729.20150309-1.patch, 
> YARN-2729.20150322-1.patch, YARN-2729.20150401-1.patch, 
> YARN-2729.20150402-1.patch
> Support script based NodeLabelsProvider Interface in Distributed Node Label 
> Configuration Setup . 

This message was sent by Atlassian JIRA

Reply via email to