Rohith Sharma K S commented on YARN-2729:

Hi Naganarasinmha, thanks for delivering the patch.
Some doubts and somments
# Does ScriptBasedNodeLabelsProvider is periodically runs similar to 
# 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.
  protected void serviceStart() throws Exception {
      // run at-least once so that labels are set
# In {{serviceStop}} in ScriptBasedNodeLabelsProvider , move 
{{super.serviceStop();}} at end of serviceStop() so first it stops shexec and 
stopping parent.
  protected void serviceStop() throws Exception {
    if (shexec != null) {
      Process p = shexec.getProcess();
      if (p != null) {
# Is there anywher documented how the script output should be? 
# 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 : 
# About the test {{TestNodeStatusUpdaterForLabels}} failing randomly.May be 
some race condition is exist.
# 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.
 (The system cannot find the path specified)

> 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
>         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, YARN-2729.20150404-1.patch, 
> YARN-2729.20150517-1.patch, YARN-2729.20150830-1.patch, 
> YARN-2729.20150925-1.patch
> Support script based NodeLabelsProvider Interface in Distributed Node Label 
> Configuration Setup . 

This message was sent by Atlassian JIRA

Reply via email to