Naganarasimha G R commented on YARN-2729:

bq. 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 
:) 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, 

bq. 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

bq. 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 

bq. 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

> 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