Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-4855

Should check if node exists when replace nodelabels

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Today when we add nodelabels to nodes, it would succeed even if nodes are not existing NodeManger in cluster without any message.
      It could be like this:
      When we use yarn rmadmin -replaceLabelsOnNode --fail-on-unkown-nodes "node1=label1" , it would be denied if node is unknown.

      1. YARN-4855.001.patch
        13 kB
        Tao Jie
      2. YARN-4855.002.patch
        14 kB
        Tao Jie
      3. YARN-4855.003.patch
        14 kB
        Tao Jie
      4. YARN-4855.004.patch
        14 kB
        Tao Jie
      5. YARN-4855.005.patch
        14 kB
        Tao Jie
      6. YARN-4855.006.patch
        15 kB
        Tao Jie
      7. YARN-4855.007.patch
        15 kB
        Tao Jie
      8. YARN-4855.008.patch
        15 kB
        Naganarasimha G R
      9. YARN-4855.009.patch
        15 kB
        Tao Jie
      10. YARN-4855.010.patch
        17 kB
        Tao Jie
      11. YARN-4855.011.patch
        18 kB
        Tao Jie
      12. YARN-4855.012.patch
        18 kB
        Tao Jie
      13. YARN-4855.013.patch
        18 kB
        Tao Jie
      14. YARN-4855-branch-2.8.patch
        18 kB
        Tao Jie

        Issue Links

          Activity

          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Tao Jie, This would be a incompatible change and agree it would have been better in the way you had mentioned but anyway it has been done to ease the load of the admin to cross verify if the node is up or down, so i do not think this can be taken up now without breaking compatability. Thoughts?
          cc / Tan, Wangda

          Show
          Naganarasimha Naganarasimha G R added a comment - Tao Jie , This would be a incompatible change and agree it would have been better in the way you had mentioned but anyway it has been done to ease the load of the admin to cross verify if the node is up or down, so i do not think this can be taken up now without breaking compatability. Thoughts? cc / Tan, Wangda
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          admin to cross verify if the node is up or down => admin need not cross verify if the node is up or down (meant)

          Show
          Naganarasimha Naganarasimha G R added a comment - admin to cross verify if the node is up or down => admin need not cross verify if the node is up or down (meant)
          Hide
          Tao Jie Tao Jie added a comment -

          Naganarasimha G R, agree. To keep compatible, how about improvement like this:
          yarn rmadmin -replaceLabelsOnNode "node1=label1" performs as it is working today.
          yarn rmadmin -replaceLabelsOnNode -checkNode "node1=label1" would check nodes existence and be denied when node not exist

          Show
          Tao Jie Tao Jie added a comment - Naganarasimha G R , agree. To keep compatible, how about improvement like this: yarn rmadmin -replaceLabelsOnNode "node1=label1" performs as it is working today. yarn rmadmin -replaceLabelsOnNode -checkNode "node1=label1" would check nodes existence and be denied when node not exist
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tao Jie, for sharing the alternate option but would like to know the background as to why this is required ? If any strong usecase then can think of supporting it !

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , for sharing the alternate option but would like to know the background as to why this is required ? If any strong usecase then can think of supporting it !
          Hide
          Tao Jie Tao Jie added a comment -

          Naganarasimha G R, thanks for reply.
          We are trying to split resource pool for certain Applications from our cluster by nodeLabel. For example, we need a resource pool of 10 nodes, then we run a script with rmadmin -replaceLabelsOnNode cmd. However, this command always run successfully even though nodes are not available in cluster (node is down or misspelling of hostname). As a result, we should do something more to ensure the capacity of the resource pool. So I hope this option may help.

          Show
          Tao Jie Tao Jie added a comment - Naganarasimha G R , thanks for reply. We are trying to split resource pool for certain Applications from our cluster by nodeLabel. For example, we need a resource pool of 10 nodes, then we run a script with rmadmin -replaceLabelsOnNode cmd. However, this command always run successfully even though nodes are not available in cluster (node is down or misspelling of hostname). As a result, we should do something more to ensure the capacity of the resource pool. So I hope this option may help.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for working on the patch, but would like to know the view of other community members as you are modifying the protocol in the patch.
          Any thoughts ? cc/ Tan, Wangda, Sunil G

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for working on the patch, but would like to know the view of other community members as you are modifying the protocol in the patch. Any thoughts ? cc/ Tan, Wangda , Sunil G
          Hide
          sunilg Sunil G added a comment -

          This would be a incompatible change

          Yes. I agree with NGarla_Unused here. This can be an incompatible behavior compared to older version. Suddenly we may get some exception which were not thrown earlier.

          yarn rmadmin -replaceLabelsOnNode -checkNode "node1=label1" seems to me like a workaround rather than a clean fix. And it may be difficult to understand what is meant by -checkNode. So I would like to get some more thoughts on original point with few suggestions.

          • Could we support this validation in replaceLabelsOnNode and document it cleanly about the new validation. OR
          • Can we skip such nodes which are not existing, but log it clearly (may be audit log too).

          I think in this earlier stages, it may be better. But its very much debatable. Thoughts?

          Show
          sunilg Sunil G added a comment - This would be a incompatible change Yes. I agree with NGarla_Unused here. This can be an incompatible behavior compared to older version. Suddenly we may get some exception which were not thrown earlier. yarn rmadmin -replaceLabelsOnNode -checkNode "node1=label1" seems to me like a workaround rather than a clean fix. And it may be difficult to understand what is meant by -checkNode . So I would like to get some more thoughts on original point with few suggestions. Could we support this validation in replaceLabelsOnNode and document it cleanly about the new validation. OR Can we skip such nodes which are not existing, but log it clearly (may be audit log too). I think in this earlier stages, it may be better. But its very much debatable. Thoughts?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Sunil G for sharing your views, as i mentioned earlier IIUC the intent of not validating the existance of the node is to just make admin life easier so that in certain cases he need not worry whether the node is temporarily down and he can just apply the label mappings. so approaches you mentioned overrides this behavior.

          But I can also understand Tao Jie's concern that if the node (host ip or port) is wrongly configured better to warn before hand than erroneously configure the cluster.
          So we have pro's and cons with the existing approach.

          So i am ok with Tao Jie's approach but with better option name (-verifyNode ?), also one more aspect is does it require a protocol change can we do it in the client side ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Sunil G for sharing your views, as i mentioned earlier IIUC the intent of not validating the existance of the node is to just make admin life easier so that in certain cases he need not worry whether the node is temporarily down and he can just apply the label mappings. so approaches you mentioned overrides this behavior. But I can also understand Tao Jie 's concern that if the node (host ip or port) is wrongly configured better to warn before hand than erroneously configure the cluster. So we have pro's and cons with the existing approach. So i am ok with Tao Jie 's approach but with better option name (-verifyNode ?), also one more aspect is does it require a protocol change can we do it in the client side ?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Tao Jie and discussions from Naganarasimha G R, Sunil G.

          Comment looks doable and backward compatible. However, -checkNode seems very confusing. How about renaming it to "--fail-on-unkown-nodes", which is longer but clearer.

          Thoughts?

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Tao Jie and discussions from Naganarasimha G R , Sunil G . Comment looks doable and backward compatible. However, -checkNode seems very confusing. How about renaming it to "--fail-on-unkown-nodes", which is longer but clearer. Thoughts?
          Hide
          sunilg Sunil G added a comment -

          he need not worry whether the node is temporarily down and he can just apply the label mappings

          Thanks Naganarasimha G R for the detailed clarification. This makes sense to me.

          How about renaming it to "--fail-on-unkown-nodes"

          +1 for this as we all agree that current naming is confusing. Thanks Wangda Tan and Naganarasimha G R.

          Show
          sunilg Sunil G added a comment - he need not worry whether the node is temporarily down and he can just apply the label mappings Thanks Naganarasimha G R for the detailed clarification. This makes sense to me. How about renaming it to "--fail-on-unkown-nodes" +1 for this as we all agree that current naming is confusing. Thanks Wangda Tan and Naganarasimha G R .
          Hide
          Tao Jie Tao Jie added a comment -

          Sunil G Wangda Tan Naganarasimha G R Thanks for your comments.
          I agree "--fail-on-unkown-nodes" makes the option more clear.

          also one more aspect is does it require a protocol change can we do it in the client side ?

          When we do verification on client side, client should request for all active node list (if verify on server side, node list is available in memory). My earlier concern is that it will bring more load if we do this verification as default behavior. However when we keep the default behavior as it used to be, and add one option for the node verification, I think on client side is more concise.

          More consideration´╝î
          When we use yarn rmadmin -replaceLabelsOnNode and add node label to wrong nodes, this information seems to leak on RM. It stores in RM memory/filesystem, but could not be found in website or by shell cmd. As a result, it is difficult to remove or correct. Maybe it could be fixed in another Jira.
          Please correct me if I am wrong, thanks!

          Show
          Tao Jie Tao Jie added a comment - Sunil G Wangda Tan Naganarasimha G R Thanks for your comments. I agree "--fail-on-unkown-nodes" makes the option more clear. also one more aspect is does it require a protocol change can we do it in the client side ? When we do verification on client side, client should request for all active node list (if verify on server side, node list is available in memory). My earlier concern is that it will bring more load if we do this verification as default behavior. However when we keep the default behavior as it used to be, and add one option for the node verification, I think on client side is more concise. More consideration´╝î When we use yarn rmadmin -replaceLabelsOnNode and add node label to wrong nodes, this information seems to leak on RM. It stores in RM memory/filesystem, but could not be found in website or by shell cmd. As a result, it is difficult to remove or correct. Maybe it could be fixed in another Jira. Please correct me if I am wrong, thanks!
          Hide
          leftnoteasy Wangda Tan added a comment -

          Actually I think pulling running NM data from RM is a doable idea, it avoids modifying RMAdmin protocol. Yes the approach adds more overhead to network. But in practice we don't need update node partition very frequently before we have YARN-3409. And it will be only required when --fail-on-unkown-nodes is specified.

          Thoughts?

          Show
          leftnoteasy Wangda Tan added a comment - Actually I think pulling running NM data from RM is a doable idea, it avoids modifying RMAdmin protocol. Yes the approach adds more overhead to network. But in practice we don't need update node partition very frequently before we have YARN-3409 . And it will be only required when --fail-on-unkown-nodes is specified. Thoughts?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 45s trunk passed
          +1 compile 0m 16s trunk passed with JDK v1.8.0_77
          +1 compile 0m 19s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 14s trunk passed
          +1 mvnsite 0m 22s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 0m 34s trunk passed
          +1 javadoc 0m 14s trunk passed with JDK v1.8.0_77
          +1 javadoc 0m 17s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 13s the patch passed with JDK v1.8.0_77
          +1 javac 0m 13s the patch passed
          +1 compile 0m 16s the patch passed with JDK v1.7.0_95
          +1 javac 0m 16s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: patch generated 6 new + 41 unchanged - 3 fixed = 47 total (was 44)
          +1 mvnsite 0m 20s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 0m 43s the patch passed
          +1 javadoc 0m 12s the patch passed with JDK v1.8.0_77
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_95
          -1 unit 66m 8s hadoop-yarn-client in the patch failed with JDK v1.8.0_77.
          -1 unit 66m 23s hadoop-yarn-client in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 16s Patch does not generate ASF License warnings.
          145m 56s



          Reason Tests
          JDK v1.8.0_77 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy
            hadoop.yarn.client.TestGetGroups
          JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient
          JDK v1.7.0_95 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy
            hadoop.yarn.client.TestGetGroups
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797957/YARN-4855.002.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c2763fa22212 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1b78b2b
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11018/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11018/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 45s trunk passed +1 compile 0m 16s trunk passed with JDK v1.8.0_77 +1 compile 0m 19s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 22s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 0m 34s trunk passed +1 javadoc 0m 14s trunk passed with JDK v1.8.0_77 +1 javadoc 0m 17s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 18s the patch passed +1 compile 0m 13s the patch passed with JDK v1.8.0_77 +1 javac 0m 13s the patch passed +1 compile 0m 16s the patch passed with JDK v1.7.0_95 +1 javac 0m 16s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: patch generated 6 new + 41 unchanged - 3 fixed = 47 total (was 44) +1 mvnsite 0m 20s the patch passed +1 mvneclipse 0m 11s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 0m 43s the patch passed +1 javadoc 0m 12s the patch passed with JDK v1.8.0_77 +1 javadoc 0m 14s the patch passed with JDK v1.7.0_95 -1 unit 66m 8s hadoop-yarn-client in the patch failed with JDK v1.8.0_77. -1 unit 66m 23s hadoop-yarn-client in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 16s Patch does not generate ASF License warnings. 145m 56s Reason Tests JDK v1.8.0_77 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.TestGetGroups JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient JDK v1.7.0_95 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.TestGetGroups JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797957/YARN-4855.002.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c2763fa22212 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1b78b2b Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-YARN-Build/11018/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11018/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/11018/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Tao Jie Tao Jie added a comment -

          Attached patch fixes whitespace and some of style check

          Show
          Tao Jie Tao Jie added a comment - Attached patch fixes whitespace and some of style check
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 9m 31s trunk passed
          +1 compile 0m 26s trunk passed with JDK v1.8.0_77
          +1 compile 0m 21s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 43s trunk passed
          +1 javadoc 0m 26s trunk passed with JDK v1.8.0_77
          +1 javadoc 0m 20s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 23s the patch passed
          +1 compile 0m 25s the patch passed with JDK v1.8.0_77
          +1 javac 0m 25s the patch passed
          +1 compile 0m 21s the patch passed with JDK v1.7.0_95
          +1 javac 0m 21s the patch passed
          -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: patch generated 5 new + 41 unchanged - 3 fixed = 46 total (was 44)
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 1s Patch has no whitespace issues.
          +1 findbugs 0m 52s the patch passed
          +1 javadoc 0m 22s the patch passed with JDK v1.8.0_77
          +1 javadoc 0m 20s the patch passed with JDK v1.7.0_95
          -1 unit 67m 4s hadoop-yarn-client in the patch failed with JDK v1.8.0_77.
          -1 unit 66m 44s hadoop-yarn-client in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          151m 58s



          Reason Tests
          JDK v1.8.0_77 Failed junit tests hadoop.yarn.client.TestGetGroups
            hadoop.yarn.client.api.impl.TestAMRMProxy
          JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient
          JDK v1.7.0_95 Failed junit tests hadoop.yarn.client.TestGetGroups
            hadoop.yarn.client.api.impl.TestAMRMProxy
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797985/YARN-4855.003.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0280dca981ec 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 1ff27f9
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11021/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11021/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 9m 31s trunk passed +1 compile 0m 26s trunk passed with JDK v1.8.0_77 +1 compile 0m 21s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 43s trunk passed +1 javadoc 0m 26s trunk passed with JDK v1.8.0_77 +1 javadoc 0m 20s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 23s the patch passed +1 compile 0m 25s the patch passed with JDK v1.8.0_77 +1 javac 0m 25s the patch passed +1 compile 0m 21s the patch passed with JDK v1.7.0_95 +1 javac 0m 21s the patch passed -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: patch generated 5 new + 41 unchanged - 3 fixed = 46 total (was 44) +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 1s Patch has no whitespace issues. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 22s the patch passed with JDK v1.8.0_77 +1 javadoc 0m 20s the patch passed with JDK v1.7.0_95 -1 unit 67m 4s hadoop-yarn-client in the patch failed with JDK v1.8.0_77. -1 unit 66m 44s hadoop-yarn-client in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 151m 58s Reason Tests JDK v1.8.0_77 Failed junit tests hadoop.yarn.client.TestGetGroups   hadoop.yarn.client.api.impl.TestAMRMProxy JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient JDK v1.7.0_95 Failed junit tests hadoop.yarn.client.TestGetGroups   hadoop.yarn.client.api.impl.TestAMRMProxy JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797985/YARN-4855.003.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0280dca981ec 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1ff27f9 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-YARN-Build/11021/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11021/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/11021/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Tao Jie Tao Jie added a comment -

          Attached patch updated, Naganarasimha G R, Sunil G, Wangda Tan would you mind giving it a quick review?

          Show
          Tao Jie Tao Jie added a comment - Attached patch updated, Naganarasimha G R , Sunil G , Wangda Tan would you mind giving it a quick review?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 33s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 1s trunk passed
          +1 compile 0m 18s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 23s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 31s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 17s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 21 new + 123 unchanged - 5 fixed = 144 total (was 128)
          +1 mvnsite 0m 20s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 11s the patch passed
          -1 unit 8m 16s hadoop-yarn-client in the patch failed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          20m 44s



          Reason Tests
          Failed junit tests hadoop.yarn.client.api.impl.TestYarnClient
            hadoop.yarn.client.cli.TestLogsCLI



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:85209cc
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12815691/YARN-4855.004.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9bb2dde7dc74 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c25021f
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12166/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12166/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 33s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 1s trunk passed +1 compile 0m 18s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 23s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 17s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 21 new + 123 unchanged - 5 fixed = 144 total (was 128) +1 mvnsite 0m 20s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 11s the patch passed -1 unit 8m 16s hadoop-yarn-client in the patch failed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 20m 44s Reason Tests Failed junit tests hadoop.yarn.client.api.impl.TestYarnClient   hadoop.yarn.client.cli.TestLogsCLI Subsystem Report/Notes Docker Image:yetus/hadoop:85209cc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12815691/YARN-4855.004.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9bb2dde7dc74 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c25021f Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12166/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12166/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12166/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Tao Jie Tao Jie added a comment -

          Rebased the patch, Naganarasimha G R, Sunil G, Wangda Tan could you take a review of it? Thanks in advance!

          Show
          Tao Jie Tao Jie added a comment - Rebased the patch, Naganarasimha G R , Sunil G , Wangda Tan could you take a review of it? Thanks in advance!
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 39s trunk passed
          +1 compile 0m 22s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 0m 31s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 20s the patch passed
          +1 compile 0m 18s the patch passed
          +1 javac 0m 18s the patch passed
          -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 20 new + 98 unchanged - 2 fixed = 118 total (was 100)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 13s the patch passed
          +1 unit 15m 57s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          30m 17s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826299/YARN-4855.005.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ab510c90a71e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 20ae1fa
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12960/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12960/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12960/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 39s trunk passed +1 compile 0m 22s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 20s the patch passed +1 compile 0m 18s the patch passed +1 javac 0m 18s the patch passed -1 checkstyle 0m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 20 new + 98 unchanged - 2 fixed = 118 total (was 100) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 13s the patch passed +1 unit 15m 57s hadoop-yarn-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 30m 17s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826299/YARN-4855.005.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ab510c90a71e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 20ae1fa Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12960/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12960/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12960/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          sorry for the delay Tao Jie, Will review tomorrow India time, but can you check the checkstyle issues are related to the patch and any of it can be addressed ?

          Show
          Naganarasimha Naganarasimha G R added a comment - sorry for the delay Tao Jie , Will review tomorrow India time, but can you check the checkstyle issues are related to the patch and any of it can be addressed ?
          Hide
          Tao Jie Tao Jie added a comment -

          refresh the patch and refine the checkstyle issues

          Show
          Tao Jie Tao Jie added a comment - refresh the patch and refine the checkstyle issues
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 59s trunk passed
          +1 compile 0m 20s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 29s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 5 new + 97 unchanged - 3 fixed = 102 total (was 100)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 37s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 12s the patch passed
          +1 unit 15m 59s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          28m 20s



          Reason Tests
          FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
            Write to static field org.apache.hadoop.yarn.client.cli.RMAdminCLI.yarnClient from instance method org.apache.hadoop.yarn.client.cli.RMAdminCLI.setYarnClient(YarnClient) At RMAdminCLI.java:from instance method org.apache.hadoop.yarn.client.cli.RMAdminCLI.setYarnClient(YarnClient) At RMAdminCLI.java:[line 179]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826540/YARN-4855.006.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9e39ff4426f6 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6f4b0d3
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12980/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12980/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.html
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12980/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/12980/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 59s trunk passed +1 compile 0m 20s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 29s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 18s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 5 new + 97 unchanged - 3 fixed = 102 total (was 100) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 37s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 12s the patch passed +1 unit 15m 59s hadoop-yarn-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 28m 20s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client   Write to static field org.apache.hadoop.yarn.client.cli.RMAdminCLI.yarnClient from instance method org.apache.hadoop.yarn.client.cli.RMAdminCLI.setYarnClient(YarnClient) At RMAdminCLI.java:from instance method org.apache.hadoop.yarn.client.cli.RMAdminCLI.setYarnClient(YarnClient) At RMAdminCLI.java: [line 179] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826540/YARN-4855.006.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9e39ff4426f6 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6f4b0d3 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/12980/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/12980/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12980/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/12980/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie,
          Sorry for the really long delay in reviewing, Some nits what i found in the patch are :

          1. findbugs issue is there need to be corrected
          2. checkstyle issue still exists, see the white space
          3. acceptNode => isRMRegisteredNode or isValidNode, former seems to be better
          4. "./yarn rmadmin -replaceLabelsOnNode " then error message is not displayed and silently finishes
          5. seems like RMAdminCLI doesnt require both setYarnClient and createYarnClient.
          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , Sorry for the really long delay in reviewing, Some nits what i found in the patch are : findbugs issue is there need to be corrected checkstyle issue still exists, see the white space acceptNode => isRMRegisteredNode or isValidNode, former seems to be better "./yarn rmadmin -replaceLabelsOnNode " then error message is not displayed and silently finishes seems like RMAdminCLI doesnt require both setYarnClient and createYarnClient .
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you for your comments, Naganarasimha G R.

          acceptNode => isRMRegisteredNode or isValidNode, former seems to be better

          The logic of verifying nodes is to compare set of requested nodes to set of registered nodes, and check if former set is subset of the latter. The method acceptNode here is to compare one node to another(check if they are the same one). isRMRegiteredNode seems like compare one node to a set of nodes. I will trying to improve like this(maybe it is easier to understand).

          seems like RMAdminCLI doesnt require both setYarnClient and createYarnClient.

          setYarnClient is used only in testcase, where we need to have a mock yarnClient(like the property localNodeLabelsManager). I tried to set yarnClient to protected, and assigned it in test, it triggered another findbugs warning. I will trying to refine here, if you know some best practices, please tell me.

          Show
          Tao Jie Tao Jie added a comment - Thank you for your comments, Naganarasimha G R . acceptNode => isRMRegisteredNode or isValidNode, former seems to be better The logic of verifying nodes is to compare set of requested nodes to set of registered nodes, and check if former set is subset of the latter. The method acceptNode here is to compare one node to another (check if they are the same one). isRMRegiteredNode seems like compare one node to a set of nodes. I will trying to improve like this(maybe it is easier to understand). seems like RMAdminCLI doesnt require both setYarnClient and createYarnClient. setYarnClient is used only in testcase, where we need to have a mock yarnClient(like the property localNodeLabelsManager ). I tried to set yarnClient to protected, and assigned it in test, it triggered another findbugs warning. I will trying to refine here, if you know some best practices, please tell me.
          Hide
          Tao Jie Tao Jie added a comment -

          "./yarn rmadmin -replaceLabelsOnNode " then error message is not displayed and silently finishes

          I have tried this command and it prints error message as expected, since there already have arg check in run()

          Show
          Tao Jie Tao Jie added a comment - "./yarn rmadmin -replaceLabelsOnNode " then error message is not displayed and silently finishes I have tried this command and it prints error message as expected, since there already have arg check in run()
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 55s trunk passed
          +1 compile 0m 20s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 24s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 29s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 3 fixed = 98 total (was 100)
          +1 mvnsite 0m 21s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 33s the patch passed
          +1 javadoc 0m 12s the patch passed
          -1 unit 15m 53s hadoop-yarn-client in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          28m 4s



          Reason Tests
          Failed junit tests hadoop.yarn.client.api.impl.TestAMRMClient



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827033/YARN-4855.007.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7e6c26ca4c06 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 07650bc
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13014/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13014/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 55s trunk passed +1 compile 0m 20s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 24s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 29s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 3 fixed = 98 total (was 100) +1 mvnsite 0m 21s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 33s the patch passed +1 javadoc 0m 12s the patch passed -1 unit 15m 53s hadoop-yarn-client in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 28m 4s Reason Tests Failed junit tests hadoop.yarn.client.api.impl.TestAMRMClient Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827033/YARN-4855.007.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7e6c26ca4c06 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 07650bc Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13014/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13014/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/13014/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for working on the review comments Tao Jie,
          I have uploaded patch for following items :

          1. acceptNode was not fitting hence made it isNodeSame
          2. "./yarn rmadmin -replaceLabelsOnNode" : sorry my bad, it was going wrong when --fail-on-unkown-nodes followed with no args
          3. I will trying to refine here, if you know some best practices, please tell me : Have corrected in the attached, Please take a look.

          please verify whether my patch is fine.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for working on the review comments Tao Jie , I have uploaded patch for following items : acceptNode was not fitting hence made it isNodeSame "./yarn rmadmin -replaceLabelsOnNode" : sorry my bad, it was going wrong when --fail-on-unkown-nodes followed with no args I will trying to refine here, if you know some best practices, please tell me : Have corrected in the attached, Please take a look. please verify whether my patch is fine.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 27s trunk passed
          +1 compile 0m 20s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 18s trunk passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 3 fixed = 98 total (was 100)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 0m 35s the patch passed
          +1 javadoc 0m 13s the patch passed
          +1 unit 15m 59s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          28m 55s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827891/YARN-4855.008.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 8d7839c39c49 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / bee9f57
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13070/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13070/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13070/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13070/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 27s trunk passed +1 compile 0m 20s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -1 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 3 fixed = 98 total (was 100) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 12s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 0m 35s the patch passed +1 javadoc 0m 13s the patch passed +1 unit 15m 59s hadoop-yarn-client in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 28m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827891/YARN-4855.008.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8d7839c39c49 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bee9f57 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13070/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13070/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13070/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client Console output https://builds.apache.org/job/PreCommit-YARN-Build/13070/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Naganarasimha G R. Looks good to me!

          Show
          Tao Jie Tao Jie added a comment - Thank you Naganarasimha G R . Looks good to me!
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tao Jie,
          White space issue can be fixed while committing and nutting much can be done for checkstyle, so will wait for some time before committing it.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , White space issue can be fixed while committing and nutting much can be done for checkstyle, so will wait for some time before committing it.
          Hide
          sunilg Sunil G added a comment -

          HI Tao Jie and NGarla_Unused

          Thanks for the work on this item. I have doubts on the patch.

          • Thinking out node, replaceLabelsOnNodes seems a little complex for me. Could we add a server side API to check whether set of nodes are registered or not? ConcurrentMap<NodeId, RMNode> getRMNodes() is available at server side. So it will be faster and better approach.
          • If we are sticking with client side impl, then we could try make use of NodeId#compareTo instead of isNodeSame
          Show
          sunilg Sunil G added a comment - HI Tao Jie and NGarla_Unused Thanks for the work on this item. I have doubts on the patch. Thinking out node, replaceLabelsOnNodes seems a little complex for me. Could we add a server side API to check whether set of nodes are registered or not? ConcurrentMap<NodeId, RMNode> getRMNodes() is available at server side. So it will be faster and better approach. If we are sticking with client side impl, then we could try make use of NodeId#compareTo instead of isNodeSame
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you for comments, Sunil G.
          Doing nodes check on client side is to avoid modifying RmAdminProtocol, and we don't expect we do this operation frequently.
          I looked NodeId#compareTo more closely, it seems that it will compare both host and port in NodeId#compareTo while in isNodeSame it will check host and port if port is set, otherwise only host will be checked.

          Show
          Tao Jie Tao Jie added a comment - Thank you for comments, Sunil G . Doing nodes check on client side is to avoid modifying RmAdminProtocol, and we don't expect we do this operation frequently. I looked NodeId#compareTo more closely, it seems that it will compare both host and port in NodeId#compareTo while in isNodeSame it will check host and port if port is set, otherwise only host will be checked.
          Hide
          sunilg Sunil G added a comment -

          HI Tao Jie, I think we need not have to modify some apis in RmAdminProtocol, We could add a new api itself. If there is a clear usecase, i think there is no issue in adding an api.
          But its better we discuss more in that. Looping Naganarasimha Garla and Wangda Tan.

          Show
          sunilg Sunil G added a comment - HI Tao Jie , I think we need not have to modify some apis in RmAdminProtocol , We could add a new api itself. If there is a clear usecase, i think there is no issue in adding an api. But its better we discuss more in that. Looping Naganarasimha Garla and Wangda Tan .
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Some how missed to comment on this ...
          Though i was in sync with Tao Jie's view in his earlier comment, on second thoughts not sure whether performance would be good in a large cluster where in we need to fetch and validate in client side for about 4000 nodes to validate few nodes being modified. So i think we can try to modify the protocol "yarn_server_resourcemanager_service_protos.proto#ReplaceLabelsOnNodeRequestProto" to add a option boolean flag to validate the existence of the node and we can move the logic to server side...
          Thoughts?

          Show
          Naganarasimha Naganarasimha G R added a comment - Some how missed to comment on this ... Though i was in sync with Tao Jie 's view in his earlier comment, on second thoughts not sure whether performance would be good in a large cluster where in we need to fetch and validate in client side for about 4000 nodes to validate few nodes being modified. So i think we can try to modify the protocol "yarn_server_resourcemanager_service_protos.proto#ReplaceLabelsOnNodeRequestProto" to add a option boolean flag to validate the existence of the node and we can move the logic to server side... Thoughts?
          Hide
          sunilg Sunil G added a comment -

          I think I suggested for a new api to avoid issue with compatibility. After checking again and also as confirmed by Naganarasimha Garla in above comment, it looks safe to modify existing ReplaceLabelsOnNodeRequestProto. +1 for this approach.

          Show
          sunilg Sunil G added a comment - I think I suggested for a new api to avoid issue with compatibility. After checking again and also as confirmed by Naganarasimha Garla in above comment, it looks safe to modify existing ReplaceLabelsOnNodeRequestProto . +1 for this approach.
          Hide
          Tao Jie Tao Jie added a comment -

          Updated the patch and moved validating logic form client side to server side.
          It looks to me that verify nodes on server side(actually I did with this approach in the very earlier patch).
          Naganarasimha G R, Sunil G thanks for sharing your points, would you mind giving it a review?

          Show
          Tao Jie Tao Jie added a comment - Updated the patch and moved validating logic form client side to server side. It looks to me that verify nodes on server side(actually I did with this approach in the very earlier patch). Naganarasimha G R , Sunil G thanks for sharing your points, would you mind giving it a review?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 53s Maven dependency ordering for branch
          +1 mvninstall 6m 51s trunk passed
          +1 compile 2m 21s trunk passed
          +1 checkstyle 0m 41s trunk passed
          +1 mvnsite 2m 0s trunk passed
          +1 mvneclipse 0m 56s trunk passed
          +1 findbugs 3m 25s trunk passed
          +1 javadoc 1m 19s trunk passed
          0 mvndep 0m 9s Maven dependency ordering for patch
          +1 mvninstall 1m 36s the patch passed
          +1 compile 2m 28s the patch passed
          +1 cc 2m 28s the patch passed
          +1 javac 2m 28s the patch passed
          -1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 90 unchanged - 2 fixed = 91 total (was 92)
          +1 mvnsite 1m 57s the patch passed
          +1 mvneclipse 0m 50s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 3m 58s the patch passed
          +1 javadoc 1m 15s the patch passed
          +1 unit 0m 24s hadoop-yarn-api in the patch passed.
          +1 unit 2m 18s hadoop-yarn-common in the patch passed.
          -1 unit 33m 44s hadoop-yarn-server-resourcemanager in the patch failed.
          -1 unit 15m 56s hadoop-yarn-client in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          85m 14s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation
            hadoop.yarn.client.cli.TestRMAdminCLI



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829114/YARN-4855.009.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux f24f47a90f22 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / ea839bd
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13139/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13139/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 53s Maven dependency ordering for branch +1 mvninstall 6m 51s trunk passed +1 compile 2m 21s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 2m 0s trunk passed +1 mvneclipse 0m 56s trunk passed +1 findbugs 3m 25s trunk passed +1 javadoc 1m 19s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 36s the patch passed +1 compile 2m 28s the patch passed +1 cc 2m 28s the patch passed +1 javac 2m 28s the patch passed -1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 90 unchanged - 2 fixed = 91 total (was 92) +1 mvnsite 1m 57s the patch passed +1 mvneclipse 0m 50s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 3m 58s the patch passed +1 javadoc 1m 15s the patch passed +1 unit 0m 24s hadoop-yarn-api in the patch passed. +1 unit 2m 18s hadoop-yarn-common in the patch passed. -1 unit 33m 44s hadoop-yarn-server-resourcemanager in the patch failed. -1 unit 15m 56s hadoop-yarn-client in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 85m 14s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation   hadoop.yarn.client.cli.TestRMAdminCLI Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829114/YARN-4855.009.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux f24f47a90f22 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ea839bd Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt https://builds.apache.org/job/PreCommit-YARN-Build/13139/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13139/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13139/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 9s Maven dependency ordering for branch
          +1 mvninstall 6m 49s trunk passed
          +1 compile 2m 25s trunk passed
          +1 checkstyle 0m 41s trunk passed
          +1 mvnsite 2m 2s trunk passed
          +1 mvneclipse 0m 59s trunk passed
          +1 findbugs 3m 27s trunk passed
          -1 javadoc 0m 22s hadoop-yarn-server-resourcemanager in trunk failed.
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 38s the patch passed
          +1 compile 2m 18s the patch passed
          +1 cc 2m 18s the patch passed
          +1 javac 2m 18s the patch passed
          +1 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167)
          +1 mvnsite 1m 53s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 46s the patch passed
          -1 javadoc 0m 19s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 23s hadoop-yarn-api in the patch passed.
          +1 unit 2m 16s hadoop-yarn-common in the patch passed.
          +1 unit 33m 52s hadoop-yarn-server-resourcemanager in the patch passed.
          +1 unit 16m 5s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          84m 30s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829185/YARN-4855.010.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux 6f515a0bf55e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b8a30f2
          Default Java 1.8.0_101
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13147/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13147/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13147/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13147/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 49s trunk passed +1 compile 2m 25s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 2m 2s trunk passed +1 mvneclipse 0m 59s trunk passed +1 findbugs 3m 27s trunk passed -1 javadoc 0m 22s hadoop-yarn-server-resourcemanager in trunk failed. 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 38s the patch passed +1 compile 2m 18s the patch passed +1 cc 2m 18s the patch passed +1 javac 2m 18s the patch passed +1 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167) +1 mvnsite 1m 53s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 46s the patch passed -1 javadoc 0m 19s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 23s hadoop-yarn-api in the patch passed. +1 unit 2m 16s hadoop-yarn-common in the patch passed. +1 unit 33m 52s hadoop-yarn-server-resourcemanager in the patch passed. +1 unit 16m 5s hadoop-yarn-client in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 84m 30s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829185/YARN-4855.010.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux 6f515a0bf55e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b8a30f2 Default Java 1.8.0_101 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13147/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13147/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13147/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13147/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Tao Jie a lot for updating the patch!
          Generally approach looks good, some comments:

          1) Rename suggestions:

          • ReplaceLabelsOnNodeRequest#setVerifyNodes/getVerifyNodes to get/setFailOnUnknownNodes
          • Same for yarn_server_resourcemanager_service_protos.proto changes

          2) AdminService:

          • For node with port specified (and port != 0), could you use the map to look up key exists (use containsKey) instead of look and compare?
          • Inaddition to getRMNodes, I think we also need to check getInactiveRMNodes, node which is decommissioning should be treated as known node, to me it is a valid use case to modify label f decommissioning nodes.
          Show
          leftnoteasy Wangda Tan added a comment - Thanks Tao Jie a lot for updating the patch! Generally approach looks good, some comments: 1) Rename suggestions: ReplaceLabelsOnNodeRequest#setVerifyNodes/getVerifyNodes to get/setFailOnUnknownNodes Same for yarn_server_resourcemanager_service_protos.proto changes 2) AdminService: For node with port specified (and port != 0), could you use the map to look up key exists (use containsKey) instead of look and compare? Inaddition to getRMNodes, I think we also need to check getInactiveRMNodes, node which is decommissioning should be treated as known node, to me it is a valid use case to modify label f decommissioning nodes.
          Hide
          Tao Jie Tao Jie added a comment -

          Wangda Tan , thank you for your comment!
          Patch updated respect to your suggestion.

          Show
          Tao Jie Tao Jie added a comment - Wangda Tan , thank you for your comment! Patch updated respect to your suggestion.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 1s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 54s Maven dependency ordering for branch
          +1 mvninstall 6m 53s trunk passed
          +1 compile 2m 16s trunk passed
          +1 checkstyle 0m 38s trunk passed
          +1 mvnsite 1m 55s trunk passed
          +1 mvneclipse 0m 56s trunk passed
          +1 findbugs 3m 23s trunk passed
          -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed.
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 42s the patch passed
          +1 compile 2m 43s the patch passed
          +1 cc 2m 43s the patch passed
          +1 javac 2m 43s the patch passed
          -1 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 165 unchanged - 2 fixed = 166 total (was 167)
          +1 mvnsite 2m 27s the patch passed
          +1 mvneclipse 0m 52s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 4m 41s the patch passed
          -1 javadoc 0m 26s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 31s hadoop-yarn-api in the patch passed.
          +1 unit 2m 31s hadoop-yarn-common in the patch passed.
          -1 unit 33m 50s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 16m 6s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          87m 51s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.TestContainerResourceUsage



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829310/YARN-4855.011.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux 08c2bdbfbb73 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 98bdb51
          Default Java 1.8.0_101
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13157/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13157/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 1s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 54s Maven dependency ordering for branch +1 mvninstall 6m 53s trunk passed +1 compile 2m 16s trunk passed +1 checkstyle 0m 38s trunk passed +1 mvnsite 1m 55s trunk passed +1 mvneclipse 0m 56s trunk passed +1 findbugs 3m 23s trunk passed -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed. 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 42s the patch passed +1 compile 2m 43s the patch passed +1 cc 2m 43s the patch passed +1 javac 2m 43s the patch passed -1 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 165 unchanged - 2 fixed = 166 total (was 167) +1 mvnsite 2m 27s the patch passed +1 mvneclipse 0m 52s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 41s the patch passed -1 javadoc 0m 26s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 31s hadoop-yarn-api in the patch passed. +1 unit 2m 31s hadoop-yarn-common in the patch passed. -1 unit 33m 50s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 16m 6s hadoop-yarn-client in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 87m 51s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestContainerResourceUsage Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829310/YARN-4855.011.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux 08c2bdbfbb73 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 98bdb51 Default Java 1.8.0_101 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13157/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13157/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13157/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Tao Jie, thanks for patiently redoing on most of the rework.

          It looks to me that verify nodes on server side(actually I did with this approach in the very earlier patch).

          Had forgotten your initial approach thanks for pointing it out i think you started to differ the approach after wangda's comment and Tan, Wangda also has already reviewed your latest patch so i think even he too is fine with your latest approach.

          I think we also need to check getInactiveRMNodes, node which is decommissioning should be treated as known node

          Yes wangda, i think it would be good to support this too and i think its been covered in the latest patch

          Latest patch seems to be good with approach just few small nits :

          1. The way we are handling "-fail-on-unknown-nodes" is not correct in the CLI, i had just typed wrongly while testing as "fail-on-unkown-nodes" so it silently passes taking "-fail-on-unkown-nodes" as the host name as per the existing code. For handling of additional options in a command we need to follow the approach similiar to one present in nodeCLI where we take from command parser cliParser.hasOption(NODE_SHOW_DETAILS) instead from the position, so that on wrong command it pops up an error
          2. if you agree on above we need to change "--fail-on-unknown-nodes" to "-fail-on-unknown-nodes". Also would like to see whether camel casing is better than this
          3. AdminService. ln no 850 "Replace labels on unknown nodes:" => "Failed to replace labels as there are unknown nodes:" would be better as we fail updating for all the mappings
          4. test failure message "Should not fail on unknown node when not verify nodes" on unknown node" instead "Should not fail on unknown node when fail-on-unkown-nodes is set false"
          Show
          Naganarasimha Naganarasimha G R added a comment - Tao Jie , thanks for patiently redoing on most of the rework. It looks to me that verify nodes on server side(actually I did with this approach in the very earlier patch). Had forgotten your initial approach thanks for pointing it out i think you started to differ the approach after wangda's comment and Tan, Wangda also has already reviewed your latest patch so i think even he too is fine with your latest approach. I think we also need to check getInactiveRMNodes, node which is decommissioning should be treated as known node Yes wangda, i think it would be good to support this too and i think its been covered in the latest patch Latest patch seems to be good with approach just few small nits : The way we are handling "- fail-on-unknown-nodes" is not correct in the CLI, i had just typed wrongly while testing as " fail-on-unkown-nodes" so it silently passes taking " -fail-on-unkown-nodes" as the host name as per the existing code. For handling of additional options in a command we need to follow the approach similiar to one present in nodeCLI where we take from command parser cliParser.hasOption(NODE_SHOW_DETAILS) instead from the position, so that on wrong command it pops up an error if you agree on above we need to change "--fail-on-unknown-nodes" to "-fail-on-unknown-nodes". Also would like to see whether camel casing is better than this AdminService. ln no 850 "Replace labels on unknown nodes:" => "Failed to replace labels as there are unknown nodes:" would be better as we fail updating for all the mappings test failure message "Should not fail on unknown node when not verify nodes" on unknown node" instead "Should not fail on unknown node when fail-on-unkown-nodes is set false"
          Hide
          Tao Jie Tao Jie added a comment -

          Naganarasimha G R, thank you for comments!
          Yes, it is better to use org.apache.commons.cli.CommandLine&Options to parse CLI commands, and it would make a lot of work to change existing code in RMAdminCLI. I think it is better to do those refactor job in another JIRA, do you agree?
          3&4 are fixed in the latest patch.

          Show
          Tao Jie Tao Jie added a comment - Naganarasimha G R , thank you for comments! Yes, it is better to use org.apache.commons.cli.CommandLine&Options to parse CLI commands, and it would make a lot of work to change existing code in RMAdminCLI. I think it is better to do those refactor job in another JIRA, do you agree? 3&4 are fixed in the latest patch.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tao Jie,
          agree if you want to update all it would be considerable effort but what about only for this command ?
          Why i am insisting once we provide a "--fail-on-unknown-nodes" then modifying to "-fail-on-unknown-nodes" will be compatability break and if new jira is only for this command then also it would not make sense.
          thoughts from others ? cc/ Sunil G & Tan, Wangda

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , agree if you want to update all it would be considerable effort but what about only for this command ? Why i am insisting once we provide a "--fail-on-unknown-nodes" then modifying to "-fail-on-unknown-nodes" will be compatability break and if new jira is only for this command then also it would not make sense. thoughts from others ? cc/ Sunil G & Tan, Wangda
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 11s Maven dependency ordering for branch
          +1 mvninstall 7m 7s trunk passed
          +1 compile 2m 23s trunk passed
          +1 checkstyle 0m 40s trunk passed
          +1 mvnsite 2m 0s trunk passed
          +1 mvneclipse 0m 57s trunk passed
          +1 findbugs 3m 26s trunk passed
          -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed.
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 43s the patch passed
          +1 compile 2m 18s the patch passed
          +1 cc 2m 18s the patch passed
          +1 javac 2m 18s the patch passed
          +1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167)
          +1 mvnsite 1m 54s the patch passed
          +1 mvneclipse 0m 49s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 56s the patch passed
          -1 javadoc 0m 19s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 0m 23s hadoop-yarn-api in the patch passed.
          +1 unit 2m 17s hadoop-yarn-common in the patch passed.
          -1 unit 34m 35s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 16m 45s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 27s The patch does not generate ASF License warnings.
          86m 29s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829400/YARN-4855.012.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux fe65b3fed91b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c6d1d74
          Default Java 1.8.0_101
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13162/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13162/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 7m 7s trunk passed +1 compile 2m 23s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 2m 0s trunk passed +1 mvneclipse 0m 57s trunk passed +1 findbugs 3m 26s trunk passed -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed. 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 43s the patch passed +1 compile 2m 18s the patch passed +1 cc 2m 18s the patch passed +1 javac 2m 18s the patch passed +1 checkstyle 0m 38s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167) +1 mvnsite 1m 54s the patch passed +1 mvneclipse 0m 49s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 56s the patch passed -1 javadoc 0m 19s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 0m 23s hadoop-yarn-api in the patch passed. +1 unit 2m 17s hadoop-yarn-common in the patch passed. -1 unit 34m 35s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 16m 45s hadoop-yarn-client in the patch passed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 86m 29s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829400/YARN-4855.012.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux fe65b3fed91b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c6d1d74 Default Java 1.8.0_101 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13162/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13162/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13162/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thanks Tao Jie and Naganarasimha G R for taking the discussions forward.

          We already support sub options as below.

          [-refreshNodes [-g|graceful [timeout in seconds] -client|server]]
          

          So i think "-fail-on-unknown-nodes" makes more sense to me. However, generally RMAdminCLI options are using camel casing. And we are not supporting much of sub-options till now here unlike in ApplicationCLI. And ApplicationCLI already has appStates etc. So may be we could make this sub option like failOnUnknownNodes.

          Show
          sunilg Sunil G added a comment - Thanks Tao Jie and Naganarasimha G R for taking the discussions forward. We already support sub options as below. [-refreshNodes [-g|graceful [timeout in seconds] -client|server]] So i think "-fail-on-unknown-nodes" makes more sense to me. However, generally RMAdminCLI options are using camel casing. And we are not supporting much of sub-options till now here unlike in ApplicationCLI. And ApplicationCLI already has appStates etc. So may be we could make this sub option like failOnUnknownNodes .
          Hide
          sunilg Sunil G added a comment -

          To clarify more, i am fine with options like

          • -fail-on-unknown-nodes with a single hyphen
          • or use came casing like failOnUnknownNodes

          Eventhough we already use such came cases, i dont think every one likes this. So i am fine with option1 also.

          Show
          sunilg Sunil G added a comment - To clarify more, i am fine with options like -fail-on-unknown-nodes with a single hyphen or use came casing like failOnUnknownNodes Eventhough we already use such came cases, i dont think every one likes this. So i am fine with option1 also.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for your copinion Sunil G, but further to this more importantly we would like yours and Tan, Wangda's view on using cliParser.hasOption(NODE_SHOW_DETAILS) or the existing approach checking the args [i], as i informed in my earlier comment when i wrongly input "fail-on-unkown-nodes" and it just took this argument as the nodeId based on the existing logic and parser did not flag it as an invalid option.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for your copinion Sunil G , but further to this more importantly we would like yours and Tan, Wangda 's view on using cliParser.hasOption(NODE_SHOW_DETAILS) or the existing approach checking the args [i] , as i informed in my earlier comment when i wrongly input "fail-on-unkown-nodes" and it just took this argument as the nodeId based on the existing logic and parser did not flag it as an invalid option.
          Hide
          leftnoteasy Wangda Tan added a comment -

          I prefer to make this option consistent to what we have, now we have almost all options in camel case, so it will be better to not add the hyphen-connected option.

          And for parsing the command using args vs. using CliParser, I will strongly suggest to use the cliParser, but that can come in a separated patch.

          Show
          leftnoteasy Wangda Tan added a comment - I prefer to make this option consistent to what we have, now we have almost all options in camel case, so it will be better to not add the hyphen-connected option. And for parsing the command using args vs. using CliParser, I will strongly suggest to use the cliParser, but that can come in a separated patch.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          so it will be better to not add the hyphen-connected option.

          agree...

          will strongly suggest to use the cliParser, but that can come in a separated patch.

          Ok In that case lets try to target this at the earliest and will not commit this patch to 2.8 until the followup jira comes in.

          Tao Jie, hope you can quickly create a patch to modify the option to "failOnUnknownNodes" and raise a new jira to use CliParser's options where ever required.

          Show
          Naganarasimha Naganarasimha G R added a comment - so it will be better to not add the hyphen-connected option. agree... will strongly suggest to use the cliParser, but that can come in a separated patch. Ok In that case lets try to target this at the earliest and will not commit this patch to 2.8 until the followup jira comes in. Tao Jie , hope you can quickly create a patch to modify the option to "failOnUnknownNodes" and raise a new jira to use CliParser's options where ever required.
          Hide
          sunilg Sunil G added a comment -

          I also generally agree with Wangda's approach. Improving to use cliParser option could be tracked separately.

          Show
          sunilg Sunil G added a comment - I also generally agree with Wangda's approach. Improving to use cliParser option could be tracked separately.
          Hide
          Tao Jie Tao Jie added a comment -

          Rename --fail-on-unkown-nodes to -failOnUnknownNodes...

          Show
          Tao Jie Tao Jie added a comment - Rename --fail-on-unkown-nodes to -failOnUnknownNodes ...
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 12s Maven dependency ordering for branch
          +1 mvninstall 9m 1s trunk passed
          +1 compile 3m 22s trunk passed
          +1 checkstyle 0m 50s trunk passed
          +1 mvnsite 2m 47s trunk passed
          +1 mvneclipse 1m 10s trunk passed
          +1 findbugs 4m 19s trunk passed
          +1 javadoc 1m 44s trunk passed
          0 mvndep 0m 13s Maven dependency ordering for patch
          +1 mvninstall 2m 6s the patch passed
          +1 compile 3m 9s the patch passed
          +1 cc 3m 9s the patch passed
          +1 javac 3m 9s the patch passed
          +1 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167)
          +1 mvnsite 2m 23s the patch passed
          +1 mvneclipse 1m 2s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 4m 41s the patch passed
          +1 javadoc 1m 30s the patch passed
          +1 unit 0m 35s hadoop-yarn-api in the patch passed.
          +1 unit 2m 49s hadoop-yarn-common in the patch passed.
          -1 unit 41m 33s hadoop-yarn-server-resourcemanager in the patch failed.
          +1 unit 16m 20s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          102m 32s



          Reason Tests
          Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831222/YARN-4855.013.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux 162f4e99edbd 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fe9ebe2
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13262/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13262/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13262/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13262/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 9m 1s trunk passed +1 compile 3m 22s trunk passed +1 checkstyle 0m 50s trunk passed +1 mvnsite 2m 47s trunk passed +1 mvneclipse 1m 10s trunk passed +1 findbugs 4m 19s trunk passed +1 javadoc 1m 44s trunk passed 0 mvndep 0m 13s Maven dependency ordering for patch +1 mvninstall 2m 6s the patch passed +1 compile 3m 9s the patch passed +1 cc 3m 9s the patch passed +1 javac 3m 9s the patch passed +1 checkstyle 0m 45s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 165 unchanged - 2 fixed = 165 total (was 167) +1 mvnsite 2m 23s the patch passed +1 mvneclipse 1m 2s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 4m 41s the patch passed +1 javadoc 1m 30s the patch passed +1 unit 0m 35s hadoop-yarn-api in the patch passed. +1 unit 2m 49s hadoop-yarn-common in the patch passed. -1 unit 41m 33s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 16m 20s hadoop-yarn-client in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 102m 32s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831222/YARN-4855.013.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux 162f4e99edbd 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fe9ebe2 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/13262/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13262/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13262/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13262/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the patch Tao Jie, +1 LGTM and as discussed hope you could raise a new jira on work on it.
          test case failure is tracked in YARN-5057, will go ahead and commit it by tomorrow if no further comments on it.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the patch Tao Jie , +1 LGTM and as discussed hope you could raise a new jira on work on it. test case failure is tracked in YARN-5057 , will go ahead and commit it by tomorrow if no further comments on it.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the patch Tao Jie, committed to trunk and branch-2, As the new option is a compatible modification we can plan to add it in 2.8 but patch seems to fail on branch-2.8, so can you create a patch for 2.8 ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the patch Tao Jie , committed to trunk and branch-2, As the new option is a compatible modification we can plan to add it in 2.8 but patch seems to fail on branch-2.8, so can you create a patch for 2.8 ?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10528 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10528/)
          YARN-4855. Should check if node exists when replace nodelabels. (naganarasimha_gr: rev 6e130c308cf1b97e8386b6a43c26d72d2850119c)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server/yarn_server_resourcemanager_service_protos.proto
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/AdminService.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/ReplaceLabelsOnNodeRequest.java
          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/impl/pb/ReplaceLabelsOnNodeRequestPBImpl.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10528 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10528/ ) YARN-4855 . Should check if node exists when replace nodelabels. (naganarasimha_gr: rev 6e130c308cf1b97e8386b6a43c26d72d2850119c) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/server/yarn_server_resourcemanager_service_protos.proto (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/AdminService.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RMAdminCLI.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/ReplaceLabelsOnNodeRequest.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/api/protocolrecords/impl/pb/ReplaceLabelsOnNodeRequestPBImpl.java
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the contribution Tao Jie, and reviews from Sunil G & Tan, Wangda.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the contribution Tao Jie , and reviews from Sunil G & Tan, Wangda .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 26s Maven dependency ordering for branch
          +1 mvninstall 7m 46s branch-2.8 passed
          +1 compile 2m 8s branch-2.8 passed with JDK v1.8.0_101
          +1 compile 2m 28s branch-2.8 passed with JDK v1.7.0_111
          +1 checkstyle 0m 41s branch-2.8 passed
          +1 mvnsite 2m 21s branch-2.8 passed
          +1 mvneclipse 1m 4s branch-2.8 passed
          +1 findbugs 4m 30s branch-2.8 passed
          +1 javadoc 1m 35s branch-2.8 passed with JDK v1.8.0_101
          +1 javadoc 1m 54s branch-2.8 passed with JDK v1.7.0_111
          0 mvndep 0m 13s Maven dependency ordering for patch
          +1 mvninstall 1m 58s the patch passed
          +1 compile 2m 18s the patch passed with JDK v1.8.0_101
          +1 cc 2m 18s the patch passed
          +1 javac 2m 18s the patch passed
          +1 compile 2m 34s the patch passed with JDK v1.7.0_111
          +1 cc 2m 34s the patch passed
          +1 javac 2m 34s the patch passed
          -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 81 unchanged - 4 fixed = 83 total (was 85)
          +1 mvnsite 2m 10s the patch passed
          +1 mvneclipse 0m 57s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 5m 19s the patch passed
          +1 javadoc 1m 8s the patch passed with JDK v1.8.0_101
          +1 javadoc 1m 21s the patch passed with JDK v1.7.0_111
          +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_101.
          +1 unit 2m 5s hadoop-yarn-common in the patch passed with JDK v1.8.0_101.
          -1 unit 74m 5s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_101.
          -1 unit 65m 58s hadoop-yarn-client in the patch failed with JDK v1.8.0_101.
          +1 unit 0m 26s hadoop-yarn-api in the patch passed with JDK v1.7.0_111.
          +1 unit 2m 37s hadoop-yarn-common in the patch passed with JDK v1.7.0_111.
          -1 unit 77m 42s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_111.
          -1 unit 66m 17s hadoop-yarn-client in the patch failed with JDK v1.7.0_111.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          335m 38s



          Reason Tests
          JDK v1.8.0_101 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
            hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.client.api.impl.TestAMRMProxy
            hadoop.yarn.client.TestGetGroups
          JDK v1.8.0_101 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient
          JDK v1.7.0_111 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens
            hadoop.yarn.server.resourcemanager.TestAMAuthorization
            hadoop.yarn.server.resourcemanager.webapp.TestRMWebappAuthentication
            hadoop.yarn.client.api.impl.TestAMRMProxy
            hadoop.yarn.client.TestGetGroups
          JDK v1.7.0_111 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI
            org.apache.hadoop.yarn.client.api.impl.TestYarnClient
            org.apache.hadoop.yarn.client.api.impl.TestAMRMClient
            org.apache.hadoop.yarn.client.api.impl.TestNMClient



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:5af2af1
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831334/YARN-4855-branch-2.8.patch
          JIRA Issue YARN-4855
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc
          uname Linux 564a43271d54 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2.8 / 3a89a88
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_101.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_101.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_111.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_111.txt
          unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_101.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_101.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_111.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_111.txt
          JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13265/testReport/
          modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/13265/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 26s Maven dependency ordering for branch +1 mvninstall 7m 46s branch-2.8 passed +1 compile 2m 8s branch-2.8 passed with JDK v1.8.0_101 +1 compile 2m 28s branch-2.8 passed with JDK v1.7.0_111 +1 checkstyle 0m 41s branch-2.8 passed +1 mvnsite 2m 21s branch-2.8 passed +1 mvneclipse 1m 4s branch-2.8 passed +1 findbugs 4m 30s branch-2.8 passed +1 javadoc 1m 35s branch-2.8 passed with JDK v1.8.0_101 +1 javadoc 1m 54s branch-2.8 passed with JDK v1.7.0_111 0 mvndep 0m 13s Maven dependency ordering for patch +1 mvninstall 1m 58s the patch passed +1 compile 2m 18s the patch passed with JDK v1.8.0_101 +1 cc 2m 18s the patch passed +1 javac 2m 18s the patch passed +1 compile 2m 34s the patch passed with JDK v1.7.0_111 +1 cc 2m 34s the patch passed +1 javac 2m 34s the patch passed -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 81 unchanged - 4 fixed = 83 total (was 85) +1 mvnsite 2m 10s the patch passed +1 mvneclipse 0m 57s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 5m 19s the patch passed +1 javadoc 1m 8s the patch passed with JDK v1.8.0_101 +1 javadoc 1m 21s the patch passed with JDK v1.7.0_111 +1 unit 0m 20s hadoop-yarn-api in the patch passed with JDK v1.8.0_101. +1 unit 2m 5s hadoop-yarn-common in the patch passed with JDK v1.8.0_101. -1 unit 74m 5s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.8.0_101. -1 unit 65m 58s hadoop-yarn-client in the patch failed with JDK v1.8.0_101. +1 unit 0m 26s hadoop-yarn-api in the patch passed with JDK v1.7.0_111. +1 unit 2m 37s hadoop-yarn-common in the patch passed with JDK v1.7.0_111. -1 unit 77m 42s hadoop-yarn-server-resourcemanager in the patch failed with JDK v1.7.0_111. -1 unit 66m 17s hadoop-yarn-client in the patch failed with JDK v1.7.0_111. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 335m 38s Reason Tests JDK v1.8.0_101 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.TestGetGroups JDK v1.8.0_101 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient JDK v1.7.0_111 Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.webapp.TestRMWebappAuthentication   hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.TestGetGroups JDK v1.7.0_111 Timed out junit tests org.apache.hadoop.yarn.client.cli.TestYarnCLI   org.apache.hadoop.yarn.client.api.impl.TestYarnClient   org.apache.hadoop.yarn.client.api.impl.TestAMRMClient   org.apache.hadoop.yarn.client.api.impl.TestNMClient Subsystem Report/Notes Docker Image:yetus/hadoop:5af2af1 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831334/YARN-4855-branch-2.8.patch JIRA Issue YARN-4855 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle cc uname Linux 564a43271d54 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2.8 / 3a89a88 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_101 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_101.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_101.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_111.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_111.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.8.0_101.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.8.0_101.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdk1.7.0_111.txt https://builds.apache.org/job/PreCommit-YARN-Build/13265/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client-jdk1.7.0_111.txt JDK v1.7.0_111 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13265/testReport/ modules C: 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-resourcemanager hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/13265/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie, forgot to mention that we need to mention these updates to the commands in the documentation @ 2 locations one is NodeLabel.html and other is command reference. I am fine with new jira or addendum patch.

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , forgot to mention that we need to mention these updates to the commands in the documentation @ 2 locations one is NodeLabel.html and other is command reference . I am fine with new jira or addendum patch.

            People

            • Assignee:
              Tao Jie Tao Jie
              Reporter:
              Tao Jie Tao Jie
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development