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

Use CliParser to parse options in RMAdminCLI

    Details

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

      Description

      As discussed in YARN-4855, it is better to use CliParser rather than args to parse command line options in RMAdminCli.

      1. YARN-5697.001.patch
        15 kB
        Tao Jie
      2. YARN-5697.002.patch
        14 kB
        Tao Jie
      3. YARN-5697.003.patch
        14 kB
        Tao Jie
      4. YARN-5697.004.patch
        15 kB
        Tao Jie
      5. YARN-5697.005.patch
        15 kB
        Tao Jie
      6. YARN-5697.005-branch-2.8.patch
        18 kB
        Tao Jie

        Issue Links

          Activity

          Hide
          Tao Jie Tao Jie added a comment -

          When I was trying to parse command line in RMAdminCLI by CliParser, I found that existing acceptable command line logic is not so restrict.
          eg:
          yarn rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore label1 is acceptable, however when using CLIParser to parse this cmd line, label1 would be parsed as argument of option -directlyAccessNodeLabelStore rather than -addToClusterNodeLabels. Actually it makes more sense.
          Same situation in cmd -removeFromClusterNodeLabels <label1,label2,label3> (label splitted by ",") and -replaceLabelsOnNode [-failOnUnknownNodes] <"node1[:port]=label1,label2 node2[:port]=label1,label2">
          I am not sure if we should keep a loose logic to ensure compatibility(we should consider label1 is arg of -directlyAccessNodeLabelStore or -addToClusterNodeLabels, both are ok) or make the command line logic more accurate.
          Naganarasimha G R, Sunil G, Wangda Tan, thoughts?

          Show
          Tao Jie Tao Jie added a comment - When I was trying to parse command line in RMAdminCLI by CliParser, I found that existing acceptable command line logic is not so restrict. eg: yarn rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore label1 is acceptable, however when using CLIParser to parse this cmd line, label1 would be parsed as argument of option -directlyAccessNodeLabelStore rather than -addToClusterNodeLabels . Actually it makes more sense. Same situation in cmd -removeFromClusterNodeLabels <label1,label2,label3> (label splitted by ",") and -replaceLabelsOnNode [-failOnUnknownNodes] <"node1[:port]=label1,label2 node2[:port]=label1,label2"> I am not sure if we should keep a loose logic to ensure compatibility(we should consider label1 is arg of -directlyAccessNodeLabelStore or -addToClusterNodeLabels , both are ok) or make the command line logic more accurate. Naganarasimha G R , Sunil G , Wangda Tan , thoughts?
          Hide
          Tao Jie Tao Jie added a comment -

          Attached the latest patch which keeps compatible with existing command line rule, and does not care the sequence of arguments(without hyphen) and options(with a single hyphen)

          Show
          Tao Jie Tao Jie added a comment - Attached the latest patch which keeps compatible with existing command line rule, and does not care the sequence of arguments(without hyphen) and options(with a single hyphen)
          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 1 new or modified test files.
          +1 mvninstall 9m 1s trunk passed
          +1 compile 0m 27s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 mvneclipse 0m 19s trunk passed
          +1 findbugs 0m 39s trunk passed
          +1 javadoc 0m 19s trunk passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 23s the patch passed
          +1 javac 0m 23s the patch passed
          -1 checkstyle 0m 15s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 7 new + 97 unchanged - 1 fixed = 104 total (was 98)
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 44s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 16m 50s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          32m 40s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832389/YARN-5697.002.patch
          JIRA Issue YARN-5697
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 094a93d61cac 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 / bea004e
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13331/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13331/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/13331/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 1 new or modified test files. +1 mvninstall 9m 1s trunk passed +1 compile 0m 27s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 35s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 0m 39s trunk passed +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 23s the patch passed +1 javac 0m 23s the patch passed -1 checkstyle 0m 15s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 7 new + 97 unchanged - 1 fixed = 104 total (was 98) +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 44s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 16m 50s hadoop-yarn-client in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 32m 40s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832389/YARN-5697.002.patch JIRA Issue YARN-5697 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 094a93d61cac 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 / bea004e Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13331/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13331/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/13331/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 21s 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 30s trunk passed
          +1 compile 0m 24s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 29s trunk passed
          +1 javadoc 0m 15s 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 0 new + 97 unchanged - 1 fixed = 97 total (was 98)
          +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 33s the patch passed
          +1 javadoc 0m 12s the patch passed
          +1 unit 16m 19s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          29m 13s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832400/YARN-5697.003.patch
          JIRA Issue YARN-5697
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a2b0f71d32c9 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 / bea004e
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13332/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/13332/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 21s 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 30s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 29s trunk passed +1 javadoc 0m 15s 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 0 new + 97 unchanged - 1 fixed = 97 total (was 98) +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 33s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 16m 19s hadoop-yarn-client in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 29m 13s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832400/YARN-5697.003.patch JIRA Issue YARN-5697 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a2b0f71d32c9 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 / bea004e Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13332/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/13332/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 Tao Jie, i am fine with keeping it as its existing logic (i.e. argument to commands like addToClusterNodeLabels) as there were earlier intentions to deprecate "directlyAccessNodeLabelStore"

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , i am fine with keeping it as its existing logic (i.e. argument to commands like addToClusterNodeLabels) as there were earlier intentions to deprecate "directlyAccessNodeLabelStore"
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Naganarasimha G R, would you mind reviewing the latest patch, which goes well with existing logic(existing unittests passed).

          Show
          Tao Jie Tao Jie added a comment - Thank you Naganarasimha G R , would you mind reviewing the latest patch, which goes well with existing logic(existing unittests passed).
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the latest patch and most of it seems fine with me

          1. would like to see some test cases to see the client processing are correct by test class overridden RMAdminCLI.createAdminProtocol() and verify diff combination of inputs for handleReplaceLabelsOnNodes. as earlier i was able to see test case passing but the nodeid coming as invalid.
          2. wouldn't it be better to use cliParser.getOptionValues rather than remainArgs = cliParser.getArgs() and then check it ? then we need to keep the label mappings as command option values and additional options at the last which i feel is fine. If we take this approach accordingly we need to update usage command for replace. If agree then we need to handle in all other places too.
          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the latest patch and most of it seems fine with me would like to see some test cases to see the client processing are correct by test class overridden RMAdminCLI.createAdminProtocol() and verify diff combination of inputs for handleReplaceLabelsOnNodes. as earlier i was able to see test case passing but the nodeid coming as invalid. wouldn't it be better to use cliParser.getOptionValues rather than remainArgs = cliParser.getArgs() and then check it ? then we need to keep the label mappings as command option values and additional options at the last which i feel is fine. If we take this approach accordingly we need to update usage command for replace. If agree then we need to handle in all other places too.
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Naganarasimha G R for your comments.
          As I mentioned before, I tried cliParser.getOptionValues in the earlier patch but found incompatibility. Both rmadmin -replaceLabelsOnNode node1=label1 -directlyAccessNodeLabelStore and rmadmin -replaceLabelsOnNode -directlyAccessNodeLabelStore node1=label1 go well in existing logic. When I use cliParser.getOptionValues to parse the latter command, node1=label1 is parsed as optionValue of -directlyAccessNodeLabelStore rather than -replaceLabelsOnNode. Actually -directlyAccessNodeLabelStore is valid in any position. As a result I use cliParser.getArgs(), which ignores args sequence but keep compatible with existing logic.

          Show
          Tao Jie Tao Jie added a comment - Thank you Naganarasimha G R for your comments. As I mentioned before, I tried cliParser.getOptionValues in the earlier patch but found incompatibility. Both rmadmin -replaceLabelsOnNode node1=label1 -directlyAccessNodeLabelStore and rmadmin -replaceLabelsOnNode -directlyAccessNodeLabelStore node1=label1 go well in existing logic. When I use cliParser.getOptionValues to parse the latter command, node1=label1 is parsed as optionValue of -directlyAccessNodeLabelStore rather than -replaceLabelsOnNode . Actually -directlyAccessNodeLabelStore is valid in any position. As a result I use cliParser.getArgs() , which ignores args sequence but keep compatible with existing logic.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie,
          IMO we have never mentioned we support anywhere in this format "rmadmin -replaceLabelsOnNode -directlyAccessNodeLabelStore node1=label1" and ideally its wrong too, so i would suggest to use "node1=label1" as option values of replaceLabelsOnNode itself. (sorry to go back and forth on this). further there were talks on discarding option of directlyAccessNodeLabelStore which is used by none.

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , IMO we have never mentioned we support anywhere in this format "rmadmin -replaceLabelsOnNode -directlyAccessNodeLabelStore node1=label1" and ideally its wrong too, so i would suggest to use "node1=label1" as option values of replaceLabelsOnNode itself. (sorry to go back and forth on this). further there were talks on discarding option of directlyAccessNodeLabelStore which is used by none.
          Hide
          Tao Jie Tao Jie added a comment -

          Thank you Naganarasimha G R,
          I tried more ideal logic in earlier patch, but failed in testcase:TestRMAdminCLI#directlyAccessNodeLabelStore:

              // change the sequence of "-directlyAccessNodeLabelStore" and labels,
              // should not matter
              args =
                  new String[] { "-addToClusterNodeLabels",
                      "-directlyAccessNodeLabelStore", "x,y" };
              assertEquals(0, rmAdminCLI.run(args));
              assertTrue(dummyNodeLabelsManager.getClusterNodeLabelNames().containsAll(
                  ImmutableSet.of("x", "y")));
          

          It seems that we don't care about the position of -directlyAccessNodeLabelStore in command line currently.
          Although -directlyAccessNodeLabelStore is marked as deprecated, this option still leads to different code path currently:

          if (directlyAccessNodeLabelStore) {
                getNodeLabelManagerInstance(getConf()).replaceLabelsOnNode(map);
              } else {
                ResourceManagerAdministrationProtocol adminProtocol =
                    createAdminProtocol();
                ReplaceLabelsOnNodeRequest request =
                    ReplaceLabelsOnNodeRequest.newInstance(map);
                request.setFailOnUnknownNodes(failOnUnknownNodes);
                adminProtocol.replaceLabelsOnNode(request);
              }
          

          Should we just remove the logic about -directlyAccessNodeLabelStore in this patch?
          To make it clear,
          1, We should restrict command line format (rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore x,y will no longer be OK, also rmadmin -replaceLabelsOnNode -failOnUnknownNodes node1=label1 should be rmadmin -replaceLabelsOnNode node1=label1 -failOnUnknownNodes).
          2, We should remove code about -directlyAccessNodeLabelStore in this patch.
          3, We should modify document and remove -directlyAccessNodeLabelStore.
          Agree?

          Show
          Tao Jie Tao Jie added a comment - Thank you Naganarasimha G R , I tried more ideal logic in earlier patch, but failed in testcase:TestRMAdminCLI#directlyAccessNodeLabelStore: // change the sequence of "-directlyAccessNodeLabelStore" and labels, // should not matter args = new String [] { "-addToClusterNodeLabels" , "-directlyAccessNodeLabelStore" , "x,y" }; assertEquals(0, rmAdminCLI.run(args)); assertTrue(dummyNodeLabelsManager.getClusterNodeLabelNames().containsAll( ImmutableSet.of( "x" , "y" ))); It seems that we don't care about the position of -directlyAccessNodeLabelStore in command line currently. Although -directlyAccessNodeLabelStore is marked as deprecated, this option still leads to different code path currently: if (directlyAccessNodeLabelStore) { getNodeLabelManagerInstance(getConf()).replaceLabelsOnNode(map); } else { ResourceManagerAdministrationProtocol adminProtocol = createAdminProtocol(); ReplaceLabelsOnNodeRequest request = ReplaceLabelsOnNodeRequest.newInstance(map); request.setFailOnUnknownNodes(failOnUnknownNodes); adminProtocol.replaceLabelsOnNode(request); } Should we just remove the logic about -directlyAccessNodeLabelStore in this patch? To make it clear, 1, We should restrict command line format ( rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore x,y will no longer be OK, also rmadmin -replaceLabelsOnNode -failOnUnknownNodes node1=label1 should be rmadmin -replaceLabelsOnNode node1=label1 -failOnUnknownNodes ). 2, We should remove code about -directlyAccessNodeLabelStore in this patch. 3, We should modify document and remove -directlyAccessNodeLabelStore . Agree?
          Hide
          andrew.wang Andrew Wang added a comment -

          As a reminder, please don't set the Fix Version until a patch has been committed. Changing it to Target Version instead.

          Show
          andrew.wang Andrew Wang added a comment - As a reminder, please don't set the Fix Version until a patch has been committed. Changing it to Target Version instead.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie,
          Had a offline discussion with Sunil G even he was also of the same opinion that rmadmin -replaceLabelsOnNode node1=label1 [-failOnUnknownNodes] [~directlyAccessNodeLabelStore] would be the right way to go and we feel its not breaking anything .

          We should restrict command line format (rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore x,y will no longer be OK,

          fine with us.

          We should modify document and remove -directlyAccessNodeLabelStore.

          well modify document anyway you can handle as part of YARN-5270 (as per the conclusion here) but remove -directlyAccessNodeLabelStore no need to take here, IIRC there is already a jira for it.

          Thoughts ? cc/ Tan, Wangda

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , Had a offline discussion with Sunil G even he was also of the same opinion that rmadmin -replaceLabelsOnNode node1=label1 [-failOnUnknownNodes] [~directlyAccessNodeLabelStore] would be the right way to go and we feel its not breaking anything . We should restrict command line format (rmadmin -addToClusterNodeLabels -directlyAccessNodeLabelStore x,y will no longer be OK, fine with us. We should modify document and remove -directlyAccessNodeLabelStore. well modify document anyway you can handle as part of YARN-5270 (as per the conclusion here) but remove -directlyAccessNodeLabelStore no need to take here, IIRC there is already a jira for it. Thoughts ? cc/ Tan, Wangda
          Hide
          sunilg Sunil G added a comment -

          Yes. I also feel that if any sub options params has some value to be read, then it should immediately follow the command. This will help the readability.

          I think i prefer the approach suggested above.

          Show
          sunilg Sunil G added a comment - Yes. I also feel that if any sub options params has some value to be read, then it should immediately follow the command. This will help the readability. I think i prefer the approach suggested above.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Tao Jie working on this patch.

          Sunil G, Naganarasimha Garla
          Is there any input want to get from me? I'm not quite sure about the question. I will be fine if the patch doesn't break compatibility.

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Tao Jie working on this patch. Sunil G , Naganarasimha Garla Is there any input want to get from me? I'm not quite sure about the question. I will be fine if the patch doesn't break compatibility.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Tan, Wangda,
          -directlyAccessNodeLabelStore could be placed in between rmadmin -replaceLabelsOnNode and node1=label1 as per the code currently but we are not planning to support -directlyAccessNodeLabelStore and also in the documentation and usage we never mention in this way hence i feel its fine to go ahead with making node1=label1 as Option values of -replaceLabelsOnNode in which case -directlyAccessNodeLabelStore will be invalid if placed in between.
          so wanted to know your thoughts on making node1=label1 as option values ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Tan, Wangda , -directlyAccessNodeLabelStore could be placed in between rmadmin -replaceLabelsOnNode and node1=label1 as per the code currently but we are not planning to support -directlyAccessNodeLabelStore and also in the documentation and usage we never mention in this way hence i feel its fine to go ahead with making node1=label1 as Option values of -replaceLabelsOnNode in which case -directlyAccessNodeLabelStore will be invalid if placed in between. so wanted to know your thoughts on making node1=label1 as option values ?
          Hide
          leftnoteasy Wangda Tan added a comment -

          Naganarasimha G R,

          YARN-3581 marks this option is DEPRECATED, if this patch plans to be committed to trunk/branch-2 only, I think it's a good time to remove all directlyAccessNodeLabelStore related logics. (But in a separated JIRA).

          If this patch goes to branch-2.8 (better not in my opinion), we have to keep this option.

          Thoughts?

          Show
          leftnoteasy Wangda Tan added a comment - Naganarasimha G R , YARN-3581 marks this option is DEPRECATED, if this patch plans to be committed to trunk/branch-2 only, I think it's a good time to remove all directlyAccessNodeLabelStore related logics. (But in a separated JIRA). If this patch goes to branch-2.8 (better not in my opinion), we have to keep this option. Thoughts?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda,
          Thanks for sharing your views, me and Sunil G had a discussion about this during the bug bash, we both were of the opinion it would be better to handle node label mapping as cliParser.getOptionValues of -replaceLabelsOnNode than handling just the additional arguments because,

          1. like any other commands it would be better to have it as OptionValues than just additional arguments of the command.
          2. This would help in parsing the command cleanly in the code
          3. -directlyAccessNodeLabelStore hardly used by anyone and is already deprecated. no where in the documentation we have specified that it can appear in between -replaceLabelsOnNode and node label mapping . And in terms of breaking compatibility, actually it doesn't break anything but just that the way command arguments are positioned differs.
          4. If this patch goes to branch-2.8 (better not in my opinion), we have to keep this option Well problem is once we do not have this in 2.8, then its like we allow -failOnUnknownNodes to come between the -replaceLabelsOnNode and node label mapping which i feel i inappropriate. and later we will not be able to modify it as again we will break the compatibility (if we assume to this placing as incompatible change).
          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , Thanks for sharing your views, me and Sunil G had a discussion about this during the bug bash, we both were of the opinion it would be better to handle node label mapping as cliParser.getOptionValues of -replaceLabelsOnNode than handling just the additional arguments because, like any other commands it would be better to have it as OptionValues than just additional arguments of the command. This would help in parsing the command cleanly in the code -directlyAccessNodeLabelStore hardly used by anyone and is already deprecated. no where in the documentation we have specified that it can appear in between -replaceLabelsOnNode and node label mapping . And in terms of breaking compatibility, actually it doesn't break anything but just that the way command arguments are positioned differs. If this patch goes to branch-2.8 (better not in my opinion), we have to keep this option Well problem is once we do not have this in 2.8, then its like we allow -failOnUnknownNodes to come between the -replaceLabelsOnNode and node label mapping which i feel i inappropriate. and later we will not be able to modify it as again we will break the compatibility (if we assume to this placing as incompatible change).
          Hide
          leftnoteasy Wangda Tan added a comment -

          Thanks Naganarasimha G R for the summary, agree with all your points. Just go ahead review/commit the patch if it does not break compatibility

          Show
          leftnoteasy Wangda Tan added a comment - Thanks Naganarasimha G R for the summary, agree with all your points. Just go ahead review/commit the patch if it does not break compatibility
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tan, Wangda, IMO it doesnt break hence taking it further besides its not in critical flow path
          Tao Jie, care to rebase the patch as per the conclusion we had and update the documentation(YARN-5720) too accordingly !

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tan, Wangda , IMO it doesnt break hence taking it further besides its not in critical flow path Tao Jie , care to rebase the patch as per the conclusion we had and update the documentation( YARN-5720 ) too accordingly !
          Hide
          Tao Jie Tao Jie added a comment -

          Updated this patch as discussed above.
          And removing -directlyAccessNodeLabelStore is not included in this patch.
          Naganarasimha G R, mind giving a review on this patch?

          Show
          Tao Jie Tao Jie added a comment - Updated this patch as discussed above. And removing -directlyAccessNodeLabelStore is not included in this patch. Naganarasimha G R , mind giving a review on this patch?
          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 6m 46s trunk passed
          +1 compile 0m 21s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 25s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 1 fixed = 98 total (was 98)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 16s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 41s the patch passed
          +1 javadoc 0m 16s the patch passed
          -1 unit 16m 18s hadoop-yarn-client in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          29m 25s



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



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue YARN-5697
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836292/YARN-5697.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2c2fc4ff6eab 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 / 7ba74be
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13716/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13716/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13716/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/13716/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT 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 6m 46s trunk passed +1 compile 0m 21s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -0 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 1 fixed = 98 total (was 98) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 41s the patch passed +1 javadoc 0m 16s the patch passed -1 unit 16m 18s hadoop-yarn-client in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 29m 25s Reason Tests Failed junit tests hadoop.yarn.client.api.impl.TestAMRMClient Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5697 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836292/YARN-5697.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2c2fc4ff6eab 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 / 7ba74be Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13716/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13716/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13716/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/13716/console Powered by Apache Yetus 0.4.0-SNAPSHOT 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 23s 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 16s trunk passed
          +1 mvnsite 0m 26s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 15s trunk passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 18s the patch passed
          +1 javac 0m 18s the patch passed
          -0 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 2 new + 97 unchanged - 1 fixed = 99 total (was 98)
          +1 mvnsite 0m 23s 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 12s the patch passed
          +1 unit 15m 59s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          29m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue YARN-5697
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836294/YARN-5697.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 12beaf56a95c 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 / 7ba74be
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13717/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13717/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/13717/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT 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 23s 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 16s trunk passed +1 mvnsite 0m 26s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 15s trunk passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 18s the patch passed +1 javac 0m 18s the patch passed -0 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 2 new + 97 unchanged - 1 fixed = 99 total (was 98) +1 mvnsite 0m 23s 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 12s the patch passed +1 unit 15m 59s hadoop-yarn-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 29m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5697 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836294/YARN-5697.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 12beaf56a95c 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 / 7ba74be Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13717/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13717/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/13717/console Powered by Apache Yetus 0.4.0-SNAPSHOT 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 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 6m 41s 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 18s the patch passed
          +1 javac 0m 18s the patch passed
          -0 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 1 fixed = 98 total (was 98)
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 34s the patch passed
          +1 javadoc 0m 12s the patch passed
          +1 unit 15m 56s hadoop-yarn-client in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          28m 37s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue YARN-5697
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836303/YARN-5697.005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ceecf04638fd 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 / 7ba74be
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13719/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13719/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/13719/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT 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 6m 41s 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 18s the patch passed +1 javac 0m 18s the patch passed -0 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 97 unchanged - 1 fixed = 98 total (was 98) +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 34s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 15m 56s hadoop-yarn-client in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 28m 37s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue YARN-5697 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836303/YARN-5697.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ceecf04638fd 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 / 7ba74be Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13719/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13719/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/13719/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tao Jie,
          +1, Latest patch looks good to me and hence going ahead and committing it.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , +1, Latest patch looks good to me and hence going ahead and committing it.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie, patch fails to apply on 2.8 can you please rebase for 2.8 ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , patch fails to apply on 2.8 can you please rebase for 2.8 ?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10748 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10748/)
          YARN-5697. Use CliParser to parse options in RMAdminCLI. Contributed by (naganarasimha_gr: rev aacf214a274fb5149437f287c542339966c03ea5)

          • (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-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10748 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10748/ ) YARN-5697 . Use CliParser to parse options in RMAdminCLI. Contributed by (naganarasimha_gr: rev aacf214a274fb5149437f287c542339966c03ea5) (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-client/src/test/java/org/apache/hadoop/yarn/client/cli/TestRMAdminCLI.java
          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 25s branch-2.8 passed
          +1 compile 0m 16s branch-2.8 passed with JDK v1.8.0_111
          +1 compile 0m 20s branch-2.8 passed with JDK v1.7.0_111
          +1 checkstyle 0m 15s branch-2.8 passed
          +1 mvnsite 0m 24s branch-2.8 passed
          +1 mvneclipse 0m 14s branch-2.8 passed
          +1 findbugs 0m 36s branch-2.8 passed
          +1 javadoc 0m 14s branch-2.8 passed with JDK v1.8.0_111
          +1 javadoc 0m 16s branch-2.8 passed with JDK v1.7.0_111
          +1 mvninstall 0m 18s the patch passed
          +1 compile 0m 14s the patch passed with JDK v1.8.0_111
          +1 javac 0m 14s the patch passed
          +1 compile 0m 18s the patch passed with JDK v1.7.0_111
          +1 javac 0m 18s the patch passed
          -0 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 41 unchanged - 3 fixed = 42 total (was 44)
          +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 45s the patch passed
          +1 javadoc 0m 11s the patch passed with JDK v1.8.0_111
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111
          -1 unit 66m 8s hadoop-yarn-client in the patch failed with JDK v1.7.0_111.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          146m 4s



          Reason Tests
          JDK v1.8.0_111 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy
            hadoop.yarn.client.TestGetGroups
            hadoop.yarn.client.TestApplicationClientProtocolOnHA
          JDK v1.8.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
          JDK v1.7.0_111 Failed junit tests 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 Issue YARN-5697
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836490/YARN-5697.005-branch-2.8.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7ed7e992eec3 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 / 9b64e30
          Default Java 1.7.0_111
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_111 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13745/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt
          unit https://builds.apache.org/job/PreCommit-YARN-Build/13745/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/13745/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/13745/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT 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 25s branch-2.8 passed +1 compile 0m 16s branch-2.8 passed with JDK v1.8.0_111 +1 compile 0m 20s branch-2.8 passed with JDK v1.7.0_111 +1 checkstyle 0m 15s branch-2.8 passed +1 mvnsite 0m 24s branch-2.8 passed +1 mvneclipse 0m 14s branch-2.8 passed +1 findbugs 0m 36s branch-2.8 passed +1 javadoc 0m 14s branch-2.8 passed with JDK v1.8.0_111 +1 javadoc 0m 16s branch-2.8 passed with JDK v1.7.0_111 +1 mvninstall 0m 18s the patch passed +1 compile 0m 14s the patch passed with JDK v1.8.0_111 +1 javac 0m 14s the patch passed +1 compile 0m 18s the patch passed with JDK v1.7.0_111 +1 javac 0m 18s the patch passed -0 checkstyle 0m 12s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client: The patch generated 1 new + 41 unchanged - 3 fixed = 42 total (was 44) +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 45s the patch passed +1 javadoc 0m 11s the patch passed with JDK v1.8.0_111 +1 javadoc 0m 13s the patch passed with JDK v1.7.0_111 -1 unit 66m 8s hadoop-yarn-client in the patch failed with JDK v1.7.0_111. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 146m 4s Reason Tests JDK v1.8.0_111 Failed junit tests hadoop.yarn.client.api.impl.TestAMRMProxy   hadoop.yarn.client.TestGetGroups   hadoop.yarn.client.TestApplicationClientProtocolOnHA JDK v1.8.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 JDK v1.7.0_111 Failed junit tests 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 Issue YARN-5697 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12836490/YARN-5697.005-branch-2.8.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7ed7e992eec3 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 / 9b64e30 Default Java 1.7.0_111 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_111 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13745/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13745/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/13745/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/13745/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tao Jie,
          can you please take a look at the test case failures?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tao Jie , can you please take a look at the test case failures?
          Hide
          Tao Jie Tao Jie added a comment -

          Hi Naganarasimha G R,
          I checked test log and it seems that test case failures are due to test environment:

          testNonExistentUser(org.apache.hadoop.yarn.client.TestGetGroups) Time elapsed: 0.004 sec <<< ERROR!
          java.net.UnknownHostException: Invalid host name: local host is: (unknown); destination host is: "7ed7e992eec3":8033; java.net.UnknownHostException; For more details see: http://wiki.apache.org/hadoop/UnknownHost

          Also I run failed test cases on my local environment, all of them are fine.

          Show
          Tao Jie Tao Jie added a comment - Hi Naganarasimha G R , I checked test log and it seems that test case failures are due to test environment: testNonExistentUser(org.apache.hadoop.yarn.client.TestGetGroups) Time elapsed: 0.004 sec <<< ERROR! java.net.UnknownHostException: Invalid host name: local host is: (unknown); destination host is: "7ed7e992eec3":8033; java.net.UnknownHostException; For more details see: http://wiki.apache.org/hadoop/UnknownHost Also I run failed test cases on my local environment, all of them are fine.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tao Jie, all the test cases passes locally for me too !,
          Committing it to branch-2.8, Thanks for the contribution Tao Jie and additional review from Sunil G & Tan, Wangda

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tao Jie , all the test cases passes locally for me too !, Committing it to branch-2.8, Thanks for the contribution Tao Jie and additional review from Sunil G & Tan, Wangda

            People

            • Assignee:
              jhung Jonathan Hung
              Reporter:
              Tao Jie Tao Jie
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development