Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-2492 (Clone of YARN-796) Allow for (admin) labels on nodes and resource-requests
  3. YARN-3579

CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings

    Details

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

      Description

      CommonNodeLabelsManager#getLabelsToNodes returns label name as string. It is not passing information such as Exclusivity etc back to REST interface apis.

      1. 0004-YARN-3579.patch
        14 kB
        Sunil G
      2. 0003-YARN-3579.patch
        14 kB
        Sunil G
      3. 0002-YARN-3579.patch
        14 kB
        Sunil G
      4. 0001-YARN-3579.patch
        7 kB
        Sunil G

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2143 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2143/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2143 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2143/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #185 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/185/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #185 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/185/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2125 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2125/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2125 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2125/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #195 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/195/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #195 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/195/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #196 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/196/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #196 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/196/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #927 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/927/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #927 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/927/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          Hide
          sunilg Sunil G added a comment -

          Thank You very much Wangda Tan for reviewing and committing the path.

          Show
          sunilg Sunil G added a comment - Thank You very much Wangda Tan for reviewing and committing the path.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Committed to trunk/branch-2. Thanks Sunil G!

          Show
          leftnoteasy Wangda Tan added a comment - Committed to trunk/branch-2. Thanks Sunil G !
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7820 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7820/)
          YARN-3579. CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7820 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7820/ ) YARN-3579 . CommonNodeLabelsManager should support NodeLabel instead of string label name when getting node-to-label/label-to-label mappings. (Sunil G via wangda) (wangda: rev d4f53fc9631d682cd79ba440aefa6750dcc898be) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/TestCommonNodeLabelsManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/RMNodeLabel.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/nodelabels/NodeLabelTestBase.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java
          Hide
          leftnoteasy Wangda Tan added a comment -

          Sunil G,
          Thanks for updating, latest patch LGTM, committing...

          Show
          leftnoteasy Wangda Tan added a comment - Sunil G , Thanks for updating, latest patch LGTM, committing...
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 15m 16s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 7m 43s There were no new javac warning messages.
          +1 javadoc 9m 52s There were no new javadoc warning messages.
          +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 0m 52s The applied patch generated 1 new checkstyle issues (total was 34, now 34).
          -1 whitespace 0m 1s The patch has 15 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 33s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 1m 24s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
          +1 yarn tests 1m 58s Tests passed in hadoop-yarn-common.
              39m 40s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12732573/0004-YARN-3579.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 065d8f2
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/whitespace.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7920/testReport/
          Java 1.7.0_55
          uname Linux asf904.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/7920/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 15m 16s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 7m 43s There were no new javac warning messages. +1 javadoc 9m 52s There were no new javadoc warning messages. +1 release audit 0m 25s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 52s The applied patch generated 1 new checkstyle issues (total was 34, now 34). -1 whitespace 0m 1s The patch has 15 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 24s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 1m 58s Tests passed in hadoop-yarn-common.     39m 40s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12732573/0004-YARN-3579.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 065d8f2 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/whitespace.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7920/artifact/patchprocess/testrun_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7920/testReport/ Java 1.7.0_55 uname Linux asf904.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7920/console This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Updating patch after a compilation problem.

          Show
          sunilg Sunil G added a comment - Updating patch after a compilation problem.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 14m 42s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          -1 javac 3m 19s The patch appears to cause the build to fail.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12732460/0003-YARN-3579.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 2463666
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/7910/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 42s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. -1 javac 3m 19s The patch appears to cause the build to fail. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12732460/0003-YARN-3579.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 2463666 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7910/console This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thank you Wangda Tan for sharing the comments. Updated a patch as per the same..

          protected Set<String> getLabelsByNode(NodeId nodeId)
          

          This method is used in test classes, I feel we can keep as protected itself. Pls share your thoughts.

          Show
          sunilg Sunil G added a comment - Thank you Wangda Tan for sharing the comments. Updated a patch as per the same.. protected Set< String > getLabelsByNode(NodeId nodeId) This method is used in test classes, I feel we can keep as protected itself. Pls share your thoughts.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Sunil G,
          Thanks for updating, few comments:

          1) Following methods can be private:
          generateNodeLabelsInfoPerNode/getLabelsToNodesMapping/getLabelsByNode/getLabelsByNode/getLabelsInfoByNode

          2) getLabelsInfoByNode can use getLabelsByNode result and get labelsInfo.

          3) findBugs warning?

          Show
          leftnoteasy Wangda Tan added a comment - Sunil G , Thanks for updating, few comments: 1) Following methods can be private: generateNodeLabelsInfoPerNode/getLabelsToNodesMapping/getLabelsByNode/getLabelsByNode/getLabelsInfoByNode 2) getLabelsInfoByNode can use getLabelsByNode result and get labelsInfo. 3) findBugs warning?
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 14m 42s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 7m 34s There were no new javac warning messages.
          +1 javadoc 9m 32s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 0m 55s The applied patch generated 1 new checkstyle issues (total was 33, now 33).
          -1 whitespace 0m 1s The patch has 15 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 33s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          -1 findbugs 1m 28s The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings.
          +1 yarn tests 1m 57s Tests passed in hadoop-yarn-common.
              38m 42s  



          Reason Tests
          FindBugs module:hadoop-yarn-common
            Comparison of String objects using == or != in org.apache.hadoop.yarn.nodelabels.CommonNodeLabelsManager.createNodeLabelFromLabelNames(Set) At CommonNodeLabelsManager.java:== or != in org.apache.hadoop.yarn.nodelabels.CommonNodeLabelsManager.createNodeLabelFromLabelNames(Set) At CommonNodeLabelsManager.java:[line 1014]



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12732271/0002-YARN-3579.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / f4e2b3c
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/whitespace.txt
          Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7892/testReport/
          Java 1.7.0_55
          uname Linux asf905.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/7892/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 42s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 7m 34s There were no new javac warning messages. +1 javadoc 9m 32s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 55s The applied patch generated 1 new checkstyle issues (total was 33, now 33). -1 whitespace 0m 1s The patch has 15 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. -1 findbugs 1m 28s The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 yarn tests 1m 57s Tests passed in hadoop-yarn-common.     38m 42s   Reason Tests FindBugs module:hadoop-yarn-common   Comparison of String objects using == or != in org.apache.hadoop.yarn.nodelabels.CommonNodeLabelsManager.createNodeLabelFromLabelNames(Set) At CommonNodeLabelsManager.java:== or != in org.apache.hadoop.yarn.nodelabels.CommonNodeLabelsManager.createNodeLabelFromLabelNames(Set) At CommonNodeLabelsManager.java: [line 1014] Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12732271/0002-YARN-3579.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / f4e2b3c checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/whitespace.txt Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7892/artifact/patchprocess/testrun_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7892/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7892/console This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thank You Wangda Tan for the comments.

          I updated the patch as per same. I used a generic method to remove the code duplication, kindly check the same.

          Show
          sunilg Sunil G added a comment - Thank You Wangda Tan for the comments. I updated the patch as per same. I used a generic method to remove the code duplication, kindly check the same.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Sunil,
          Thanks for working on this, some comments:

          • It seems to me getNodeIds is not necessary, even if it can avoid some duplications a little, but it copies whole NodeId set, I think we'd better not to do that.
          • getNodeLabelsInfo -> getNodeToLabelsInfo
          • getNodeLabelsInfo: 1. better to check for (String label : nodeLabelNames) { if label is NO_LABEL or not, only add label isn't NO_LABEL; 2. avoid adding nodeLabels when it's empty before nodeToLabelsInfo.put(nodeId, nodeLabels);; 3. You can keep a reference of NodeLabel in RMNodeLabel to avoid copy it everytime.
          • Is it possible to merge getNodeLabels/getNodeToLabelsInfo and getLabelsInfoToNodes/getLabelsToNodes using some generic type? Their logic are mostly same except one needs to get NodeLabel the other one doesn't need.
          Show
          leftnoteasy Wangda Tan added a comment - Hi Sunil, Thanks for working on this, some comments: It seems to me getNodeIds is not necessary, even if it can avoid some duplications a little, but it copies whole NodeId set, I think we'd better not to do that. getNodeLabelsInfo -> getNodeToLabelsInfo getNodeLabelsInfo: 1. better to check for (String label : nodeLabelNames) { if label is NO_LABEL or not, only add label isn't NO_LABEL; 2. avoid adding nodeLabels when it's empty before nodeToLabelsInfo.put(nodeId, nodeLabels); ; 3. You can keep a reference of NodeLabel in RMNodeLabel to avoid copy it everytime. Is it possible to merge getNodeLabels/getNodeToLabelsInfo and getLabelsInfoToNodes/getLabelsToNodes using some generic type? Their logic are mostly same except one needs to get NodeLabel the other one doesn't need.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 14m 55s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          -1 javac 7m 42s The applied patch generated 156 additional warning messages.
          +1 javadoc 9m 52s There were no new javadoc warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 0m 51s The applied patch generated 2 new checkstyle issues (total was 30, now 32).
          -1 whitespace 0m 0s The patch has 7 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 install 1m 36s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 1m 24s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
          +1 yarn tests 2m 6s Tests passed in hadoop-yarn-common.
              39m 26s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12730880/0001-YARN-3579.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 8e991f4
          javac https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/diffJavacWarnings.txt
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt
          whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/whitespace.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7765/testReport/
          Java 1.7.0_55
          uname Linux asf903.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/7765/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 55s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javac 7m 42s The applied patch generated 156 additional warning messages. +1 javadoc 9m 52s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 51s The applied patch generated 2 new checkstyle issues (total was 30, now 32). -1 whitespace 0m 0s The patch has 7 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 24s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 yarn tests 2m 6s Tests passed in hadoop-yarn-common.     39m 26s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12730880/0001-YARN-3579.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 8e991f4 javac https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/diffJavacWarnings.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/diffcheckstylehadoop-yarn-common.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/whitespace.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/7765/artifact/patchprocess/testrun_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/7765/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-YARN-Build/7765/console This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Hi Wangda Tan

          Attaching an initial version of patch.

          • getNodeLabelsInfo, which is a new version apis can be used in REST interface and admin side. This supports object instead of string
          • getNodeLabels will be still used in recover call flow where string is correct to use for.
          • getLabelsInfoToNodes and getLabelsToNodes are 2 different versions. getLabelsToNodes is marked as Deprecated and can be removed once REST and admin side started using new api.

          Pls share your thoughts on same.

          Show
          sunilg Sunil G added a comment - Hi Wangda Tan Attaching an initial version of patch. getNodeLabelsInfo, which is a new version apis can be used in REST interface and admin side. This supports object instead of string getNodeLabels will be still used in recover call flow where string is correct to use for. getLabelsInfoToNodes and getLabelsToNodes are 2 different versions. getLabelsToNodes is marked as Deprecated and can be removed once REST and admin side started using new api. Pls share your thoughts on same.
          Hide
          sunilg Sunil G added a comment -

          Sure Wangda Tan
          I will keep the REST changes in YARN-3521 itself.
          I will open another ticket for YarnClient. Thank you.

          Show
          sunilg Sunil G added a comment - Sure Wangda Tan I will keep the REST changes in YARN-3521 itself. I will open another ticket for YarnClient. Thank you.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Sunil G, thanks for open this JIRA, but I'm thinking about scope:

          • It's better to leave getLabelsToNodes REST changes in YARN-3521
          • This JIRA should at least contains getLabelsToNodes/getNodeLabels changes in NodeLabelsManager.
          • Another part is update YarnClient to use object instead of string, which can either comes together with this patch or in separated patch, feel free to create one .

          Thoughts?

          Show
          leftnoteasy Wangda Tan added a comment - Sunil G , thanks for open this JIRA, but I'm thinking about scope: It's better to leave getLabelsToNodes REST changes in YARN-3521 This JIRA should at least contains getLabelsToNodes/getNodeLabels changes in NodeLabelsManager. Another part is update YarnClient to use object instead of string, which can either comes together with this patch or in separated patch, feel free to create one . Thoughts?

            People

            • Assignee:
              sunilg Sunil G
              Reporter:
              sunilg Sunil G
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development