Details

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

      Description

      Support script based NodeLabelsProvider Interface in Distributed Node Label Configuration Setup .

      Miscellaneous Issues :

      1. In configurationNodeLabelsProvider instead of taking multiple labels from single configuration, we need to support exclusive configuration for partition (single label).
      2. Proper logging when registration of Node Fails
      3. Classloader was not getting reset from custom class loader in TestConfigurationNodeLabelsProvider.java which could make test cases fail in certain conditions
      4. In ResourceTrackerService we need to consider distributed configuration only when node labels are enabled. leads to lots of logs in certain conditions
      5. NodeLabelsProvider needs to be a interface rather than abstract class
      1. YARN-2729.20151310-2.patch
        36 kB
        Naganarasimha G R
      2. YARN-2729.20151310-1.patch
        35 kB
        Naganarasimha G R
      3. YARN-2729.20151023-1.patch
        40 kB
        Naganarasimha G R
      4. YARN-2729.20151019.patch
        39 kB
        Naganarasimha G R
      5. YARN-2729.20151015-1.patch
        40 kB
        Naganarasimha G R
      6. YARN-2729.20150925-1.patch
        32 kB
        Naganarasimha G R
      7. YARN-2729.20150830-1.patch
        22 kB
        Naganarasimha G R
      8. YARN-2729.20150517-1.patch
        19 kB
        Naganarasimha G R
      9. YARN-2729.20150404-1.patch
        41 kB
        Naganarasimha G R
      10. YARN-2729.20150402-1.patch
        26 kB
        Naganarasimha G R
      11. YARN-2729.20150401-1.patch
        7 kB
        Naganarasimha G R
      12. YARN-2729.20150322-1.patch
        24 kB
        Naganarasimha G R
      13. YARN-2729.20150309-1.patch
        24 kB
        Naganarasimha G R
      14. YARN-2729.20141210-1.patch
        18 kB
        Naganarasimha G R
      15. YARN-2729.20141120-1.patch
        18 kB
        Naganarasimha G R
      16. YARN-2729.20141031-1.patch
        18 kB
        Naganarasimha G R
      17. YARN-2729.20141024-1.patch
        15 kB
        Naganarasimha G R
      18. YARN-2729.20141023-1.patch
        13 kB
        Naganarasimha G R

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2475 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2475/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • 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-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2475 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2475/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml 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-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #538 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/538/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #538 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/538/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java hadoop-yarn-project/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #585 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/585/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.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-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #585 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/585/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.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-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #1321 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1321/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.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-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #1321 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1321/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.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-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Review and Commit , Rohith Sharma K S, Tan, Wangda & Sunil G.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Review and Commit , Rohith Sharma K S , Tan, Wangda & Sunil G .
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2528 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2528/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • 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-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2528 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2528/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) 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-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #597 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/597/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #597 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/597/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8707 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8707/)
          YARN-2729. Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99)

          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8707 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8707/ ) YARN-2729 . Support script based NodeLabelsProvider Interface in (rohithsharmaks: rev 5acdde4744c131e05db7b4b5f7d684fed7608b99) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/NodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/AbstractNodeLabelsProvider.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-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestNodeStatusUpdaterForLabels.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestConfigurationNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/ScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/nodelabels/TestScriptBasedNodeLabelsProvider.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceTrackerService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeManager.java
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          committed to branch-2/trunk.. Thanks Naganarasimha G R for contributions and patience!! Thanks Sunil G Wangda Tan Vinod Kumar Vavilapalli for review..

          Show
          rohithsharma Rohith Sharma K S added a comment - committed to branch-2/trunk.. Thanks Naganarasimha G R for contributions and patience!! Thanks Sunil G Wangda Tan Vinod Kumar Vavilapalli for review..
          Hide
          sunilg Sunil G added a comment -

          +1. Patch looks good. Thank you NGarla_Unused.

          Show
          sunilg Sunil G added a comment - +1. Patch looks good. Thank you NGarla_Unused .
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Rohith Sharma K S, seems like jenkins report is fine, checkstyle issue is related to file length...

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Rohith Sharma K S , seems like jenkins report is fine, checkstyle issue is related to file length...
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 21m 26s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 3 new or modified test files.
          +1 javac 8m 2s There were no new javac warning messages.
          +1 javadoc 10m 37s 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 2m 18s The applied patch generated 1 new checkstyle issues (total was 212, now 212).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 32s mvn install still works.
          +1 eclipse:eclipse 0m 36s The patch built with eclipse:eclipse.
          +1 findbugs 6m 4s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 24s Tests passed in hadoop-yarn-api.
          +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common.
          +1 yarn tests 9m 10s Tests passed in hadoop-yarn-server-nodemanager.
          +1 yarn tests 57m 59s Tests passed in hadoop-yarn-server-resourcemanager.
              121m 21s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12768168/YARN-2729.20151023-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 1aa735c
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-common.html
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9572/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/9572/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 21m 26s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 3 new or modified test files. +1 javac 8m 2s There were no new javac warning messages. +1 javadoc 10m 37s 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 2m 18s The applied patch generated 1 new checkstyle issues (total was 212, now 212). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 36s The patch built with eclipse:eclipse. +1 findbugs 6m 4s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 24s Tests passed in hadoop-yarn-api. +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common. +1 yarn tests 9m 10s Tests passed in hadoop-yarn-server-nodemanager. +1 yarn tests 57m 59s Tests passed in hadoop-yarn-server-resourcemanager.     121m 21s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12768168/YARN-2729.20151023-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 1aa735c Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-common.html Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9572/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9572/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/9572/console This message was automatically generated.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          +1 lgtm for the latest patch.. I will trigger jenkins again to verify QA report..

          Show
          rohithsharma Rohith Sharma K S added a comment - +1 lgtm for the latest patch.. I will trigger jenkins again to verify QA report..
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 21m 25s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 3 new or modified test files.
          +1 javac 7m 57s There were no new javac warning messages.
          +1 javadoc 10m 43s 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 2m 16s The applied patch generated 1 new checkstyle issues (total was 212, now 212).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 36s mvn install still works.
          +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse.
          +1 findbugs 6m 3s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api.
          +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common.
          +1 yarn tests 9m 13s Tests passed in hadoop-yarn-server-nodemanager.
          -1 yarn tests 53m 43s Tests failed in hadoop-yarn-server-resourcemanager.
              117m 11s  



          Reason Tests
          Failed unit tests hadoop.yarn.server.resourcemanager.security.TestRMDelegationTokens
            hadoop.yarn.server.resourcemanager.resourcetracker.TestRMNMRPCResponseId
            hadoop.yarn.server.resourcemanager.rmcontainer.TestRMContainerImpl
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerEventLog
            hadoop.yarn.server.resourcemanager.TestApplicationCleanup
            hadoop.yarn.server.resourcemanager.TestRMEmbeddedElector
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerQueueACLs
            hadoop.yarn.server.resourcemanager.TestResourceManager
            hadoop.yarn.server.resourcemanager.TestApplicationMasterService
            hadoop.yarn.server.resourcemanager.TestRMHAForNodeLabels
            hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel
            hadoop.yarn.server.resourcemanager.monitor.capacity.TestProportionalCapacityPreemptionPolicy
            hadoop.yarn.server.resourcemanager.resourcetracker.TestNMReconnect
            hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppAttempt
            hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry
            hadoop.yarn.server.resourcemanager.monitor.TestSchedulingMonitor
            hadoop.yarn.server.resourcemanager.TestRMProxyUsersConf
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairScheduler
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerDynamicBehavior
            hadoop.yarn.server.resourcemanager.TestApplicationACLs
            hadoop.yarn.server.resourcemanager.TestContainerResourceUsage
            hadoop.yarn.server.resourcemanager.scheduler.TestQueueMetrics
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerNodeLabelUpdate
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerFairShare
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerPreemption
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestAppRunnability
            hadoop.yarn.server.resourcemanager.recovery.TestZKRMStateStore
            hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSLeafQueue
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestSchedulingUpdate
            hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12768168/YARN-2729.20151023-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 124a412
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-common.html
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9534/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/9534/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 21m 25s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 3 new or modified test files. +1 javac 7m 57s There were no new javac warning messages. +1 javadoc 10m 43s 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 2m 16s The applied patch generated 1 new checkstyle issues (total was 212, now 212). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse. +1 findbugs 6m 3s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api. +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common. +1 yarn tests 9m 13s Tests passed in hadoop-yarn-server-nodemanager. -1 yarn tests 53m 43s Tests failed in hadoop-yarn-server-resourcemanager.     117m 11s   Reason Tests Failed unit tests hadoop.yarn.server.resourcemanager.security.TestRMDelegationTokens   hadoop.yarn.server.resourcemanager.resourcetracker.TestRMNMRPCResponseId   hadoop.yarn.server.resourcemanager.rmcontainer.TestRMContainerImpl   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerEventLog   hadoop.yarn.server.resourcemanager.TestApplicationCleanup   hadoop.yarn.server.resourcemanager.TestRMEmbeddedElector   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerQueueACLs   hadoop.yarn.server.resourcemanager.TestResourceManager   hadoop.yarn.server.resourcemanager.TestApplicationMasterService   hadoop.yarn.server.resourcemanager.TestRMHAForNodeLabels   hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel   hadoop.yarn.server.resourcemanager.monitor.capacity.TestProportionalCapacityPreemptionPolicy   hadoop.yarn.server.resourcemanager.resourcetracker.TestNMReconnect   hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppAttempt   hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry   hadoop.yarn.server.resourcemanager.monitor.TestSchedulingMonitor   hadoop.yarn.server.resourcemanager.TestRMProxyUsersConf   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairScheduler   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerDynamicBehavior   hadoop.yarn.server.resourcemanager.TestApplicationACLs   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage   hadoop.yarn.server.resourcemanager.scheduler.TestQueueMetrics   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerNodeLabelUpdate   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerFairShare   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerPreemption   hadoop.yarn.server.resourcemanager.scheduler.fair.TestAppRunnability   hadoop.yarn.server.resourcemanager.recovery.TestZKRMStateStore   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSLeafQueue   hadoop.yarn.server.resourcemanager.scheduler.fair.TestSchedulingUpdate   hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter   hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12768168/YARN-2729.20151023-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 124a412 Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-common.html Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9534/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9534/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/9534/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Rohith Sharma K S & Sunil G, for the comments . I have updated the patch with the approach specified. but i have a query.

          If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end.

          I could not get the above statement clearly, IIUC the order would be either D1,D,C,B,A when super.serviceStop() is called first and D,D1,C,B,A when super.serviceStop() is called at the last. Latter is what we are trying to achieve earlier, correct me if i am wrong.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Rohith Sharma K S & Sunil G , for the comments . I have updated the patch with the approach specified. but i have a query. If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end. I could not get the above statement clearly, IIUC the order would be either D1,D,C,B,A when super.serviceStop() is called first and D,D1,C,B,A when super.serviceStop() is called at the last. Latter is what we are trying to achieve earlier, correct me if i am wrong.
          Hide
          sunilg Sunil G added a comment -

          Yes Rohith Sharma K S.
          With this, child class will have the capability to clean up stuffs which can be done after time is cancelled.
          So shExec.destroy() can be called from cleanUp() which is implemented/overridden in ScriptBasedNodeLabelsProvider

          Show
          sunilg Sunil G added a comment - Yes Rohith Sharma K S . With this, child class will have the capability to clean up stuffs which can be done after time is cancelled. So shExec.destroy() can be called from cleanUp() which is implemented/overridden in ScriptBasedNodeLabelsProvider
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          Hence i am reverting the fix for the comment which was given by Rohith Sharma K S. IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ?

          The reason why I suggested in my earlier comment i.e In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent. is Consider NM has services A,B,C,D which are started in the order of ABCD. When stopping, the services are stopped in reverse order i.e D,C,B,A. Say, D has child class D1. If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end.

          Better way to avoid all conflicts is create abstract method say cleanUp() in AbstractNodeLabelProvider. This method can be override in ScriptBasedNodeLabelsProvider. A sample change is like below

            @Override
            protected void serviceStop() throws Exception {
              if (nodeLabelsScheduler != null) {
                nodeLabelsScheduler.cancel();
              }
              cleanUp();
              super.serviceStop();
            }
          
          Show
          rohithsharma Rohith Sharma K S added a comment - Hence i am reverting the fix for the comment which was given by Rohith Sharma K S. IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ? The reason why I suggested in my earlier comment i.e In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent. is Consider NM has services A,B,C,D which are started in the order of ABCD. When stopping, the services are stopped in reverse order i.e D,C,B,A. Say, D has child class D1. If you call super.serviceStop() first, D1 clean up will not happen first rather, D1(D,C,B,A)-cleanup D1 which is at the end. Better way to avoid all conflicts is create abstract method say cleanUp() in AbstractNodeLabelProvider. This method can be override in ScriptBasedNodeLabelsProvider. A sample change is like below @Override protected void serviceStop() throws Exception { if (nodeLabelsScheduler != null ) { nodeLabelsScheduler.cancel(); } cleanUp(); super .serviceStop(); }
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Testcase failure are not related to the modifications in the patch and findbugs error when run manually reported this error which is also not related to the patch. Rohith Sharma K S can you please take a look ?

          <file classname="org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl"><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.builder; locked 95% of time" lineNumber="390"/><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.proto; locked 94% of time" lineNumber="390"/><BugInstance type="IS2_INCONSISTENT_SYNC" priority="Normal" category="MT_CORRECTNESS" message="Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.viaProto; locked 94% of time" lineNumber="390"/></file>
          
          Show
          Naganarasimha Naganarasimha G R added a comment - Testcase failure are not related to the modifications in the patch and findbugs error when run manually reported this error which is also not related to the patch. Rohith Sharma K S can you please take a look ? <file classname= "org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl" ><BugInstance type= "IS2_INCONSISTENT_SYNC" priority= "Normal" category= "MT_CORRECTNESS" message= "Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.builder; locked 95% of time" lineNumber= "390" /><BugInstance type= "IS2_INCONSISTENT_SYNC" priority= "Normal" category= "MT_CORRECTNESS" message= "Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.proto; locked 94% of time" lineNumber= "390" /><BugInstance type= "IS2_INCONSISTENT_SYNC" priority= "Normal" category= "MT_CORRECTNESS" message= "Inconsistent synchronization of org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateResponsePBImpl.viaProto; locked 94% of time" lineNumber= "390" /></file>
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 20m 10s Findbugs (version 3.0.0) appears to be broken on trunk.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 3 new or modified test files.
          +1 javac 7m 55s There were no new javac warning messages.
          +1 javadoc 10m 23s 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 2m 2s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 34s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          -1 findbugs 5m 57s The patch appears to introduce 3 new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 25s Tests passed in hadoop-yarn-api.
          -1 yarn tests 1m 59s Tests failed in hadoop-yarn-common.
          +1 yarn tests 8m 56s Tests passed in hadoop-yarn-server-nodemanager.
          -1 yarn tests 62m 8s Tests failed in hadoop-yarn-server-resourcemanager.
              123m 17s  



          Reason Tests
          FindBugs module:hadoop-yarn-common
          Failed unit tests hadoop.yarn.logaggregation.TestAggregatedLogsBlock
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12767357/YARN-2729.20151019.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 7f0e1eb
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9477/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/9477/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 20m 10s Findbugs (version 3.0.0) appears to be broken on trunk. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 3 new or modified test files. +1 javac 7m 55s There were no new javac warning messages. +1 javadoc 10m 23s 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 2m 2s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 34s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. -1 findbugs 5m 57s The patch appears to introduce 3 new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 25s Tests passed in hadoop-yarn-api. -1 yarn tests 1m 59s Tests failed in hadoop-yarn-common. +1 yarn tests 8m 56s Tests passed in hadoop-yarn-server-nodemanager. -1 yarn tests 62m 8s Tests failed in hadoop-yarn-server-resourcemanager.     123m 17s   Reason Tests FindBugs module:hadoop-yarn-common Failed unit tests hadoop.yarn.logaggregation.TestAggregatedLogsBlock   hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12767357/YARN-2729.20151019.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 7f0e1eb Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9477/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9477/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/9477/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Sunil G for the comments and better late than never !

          We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this).

          IMHO i feel the former approach is better than the latter as i am not inclined towards approach of having checks in the parts of code which gets frequently executed just to avoid corner case . Hence i am reverting the fix for the comment which was given by Rohith Sharma K S. IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ?

          But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel. I didnt understand this part.

          As discussed offline i have done these modifications as a node can belong to only one Partition Label and when we support Constraints we can support more patterns later.
          Other issues i have fixed. If better approach is possible for first will rework.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Sunil G for the comments and better late than never ! We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this). IMHO i feel the former approach is better than the latter as i am not inclined towards approach of having checks in the parts of code which gets frequently executed just to avoid corner case . Hence i am reverting the fix for the comment which was given by Rohith Sharma K S . IIUC there are no impacts if we do super.serviceStop though theoritically its not the right way to do . Thoughts ? But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel. I didnt understand this part. As discussed offline i have done these modifications as a node can belong to only one Partition Label and when we support Constraints we can support more patterns later. Other issues i have fixed. If better approach is possible for first will rework.
          Hide
          sunilg Sunil G added a comment -

          Hi Naganarasimha G R and Rohith Sharma K S
          Sorry for coming in late. I have some doubts here.

          In ScriptBasedNodeLabelsProvider#serviceStop, shexec process is destroyed and the super.stop() is invoked. Hence I feel it may still be possible that shexec may again get executed (if one timer run happens in this timeframe) before timer is cancelled from AbstractNodeLabelsProvider#serviceStop. We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this). Thoughts?

          couple of other comments:
          1. ScriptBasedNodeLabelsProvider#serviceInit: verifyConfiguredScript is invoked with nodeLabelsScript which is a class variable. It may not be needed.
          2. I have one doubt in fetchLabelsFromScriptOutput. scriptOutput is split with newline and each line is checked whether its starts with NODE_LABEL_PARTITION_PATTERN. But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel. I didnt understand this part. Could you please have a look on this.

          Show
          sunilg Sunil G added a comment - Hi Naganarasimha G R and Rohith Sharma K S Sorry for coming in late. I have some doubts here. In ScriptBasedNodeLabelsProvider#serviceStop , shexec process is destroyed and the super.stop() is invoked. Hence I feel it may still be possible that shexec may again get executed (if one timer run happens in this timeframe) before timer is cancelled from AbstractNodeLabelsProvider#serviceStop . We can do super.stop() first from ScriptBasedNodeLabelsProvider. But I feel it may not be better, so could we reset shexec to null after attached process is destroyed (may also need a null check in timetask if we do this). Thoughts? couple of other comments: 1. ScriptBasedNodeLabelsProvider#serviceInit: verifyConfiguredScript is invoked with nodeLabelsScript which is a class variable. It may not be needed. 2. I have one doubt in fetchLabelsFromScriptOutput . scriptOutput is split with newline and each line is checked whether its starts with NODE_LABEL_PARTITION_PATTERN. But if we have multiple lines from input string which starts with NODE_LABEL_PARTITION_PATTERN, then only last line will be set to nodePartitionLabel . I didnt understand this part. Could you please have a look on this.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          +1 lgtm.. I will commit it after couple of days if there is no more comments on the patch.

          Show
          rohithsharma Rohith Sharma K S added a comment - +1 lgtm.. I will commit it after couple of days if there is no more comments on the patch.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 21m 11s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 3 new or modified test files.
          +1 javac 7m 58s There were no new javac warning messages.
          +1 javadoc 10m 36s 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 2m 15s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 30s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 5m 54s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 24s Tests passed in hadoop-yarn-api.
          +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common.
          +1 yarn tests 8m 58s Tests passed in hadoop-yarn-server-nodemanager.
          +1 yarn tests 57m 25s Tests passed in hadoop-yarn-server-resourcemanager.
              119m 59s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12766839/YARN-2729.20151015-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 7a98d94
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9459/testReport/
          Java 1.7.0_55
          uname Linux asf902.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/9459/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 21m 11s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 3 new or modified test files. +1 javac 7m 58s There were no new javac warning messages. +1 javadoc 10m 36s 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 2m 15s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 30s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 5m 54s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 24s Tests passed in hadoop-yarn-api. +1 yarn tests 2m 3s Tests passed in hadoop-yarn-common. +1 yarn tests 8m 58s Tests passed in hadoop-yarn-server-nodemanager. +1 yarn tests 57m 25s Tests passed in hadoop-yarn-server-resourcemanager.     119m 59s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766839/YARN-2729.20151015-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 7a98d94 Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9459/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9459/testReport/ Java 1.7.0_55 uname Linux asf902.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/9459/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Attaching a patch to make NodeLabelsProvider as a interface instead of abstract class

          Show
          Naganarasimha Naganarasimha G R added a comment - Attaching a patch to make NodeLabelsProvider as a interface instead of abstract class
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the comments Rohith Sharma K S,

          1.NodeLabelsProvider is abstract class. Why it was not made as Interface?

          Good point i think making it as interface should be sufficient and in NodeManager we can do addIfService(nodeLabelsProvider), this will will give the implementations of the NodeLabelsProvider to be much more flexible. will update current patch to incorporate these modifications shortly

          2. And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable.

          I presume you might of got it wrong over configuration of NodeLabelsProvider in NM. Basically there is whitelist of script and config and if this doesnt match then we are assuming its a qualified classname and try load it as you mentioned. Hence still its pluggable. correct me if i am wrong in understanding your comment here

          From the below description, I assume that NodeLabelsProvider instance can be created only if configuration-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also.

          This was a conscious decision which was taken to minimize the number of configurations on the NM side. It was sufficient to configure provider only as in NM, if configuration-type is configured and provider is not configured then we need to throw exception. configuration-type is mainly for the RM to deal with NM's HB/registrations and blocking CLI and REST interactions.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the comments Rohith Sharma K S , 1.NodeLabelsProvider is abstract class. Why it was not made as Interface? Good point i think making it as interface should be sufficient and in NodeManager we can do addIfService(nodeLabelsProvider) , this will will give the implementations of the NodeLabelsProvider to be much more flexible. will update current patch to incorporate these modifications shortly 2. And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable. I presume you might of got it wrong over configuration of NodeLabelsProvider in NM. Basically there is whitelist of script and config and if this doesnt match then we are assuming its a qualified classname and try load it as you mentioned. Hence still its pluggable. correct me if i am wrong in understanding your comment here From the below description, I assume that NodeLabelsProvider instance can be created only if configuration-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also. This was a conscious decision which was taken to minimize the number of configurations on the NM side. It was sufficient to configure provider only as in NM, if configuration-type is configured and provider is not configured then we need to throw exception. configuration-type is mainly for the RM to deal with NM's HB/registrations and blocking CLI and REST interactions.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          Adding to above

          1. From the below description, I assume that NodeLabelsProvider instance can be created only if configuratin-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also.
            When node labels "yarn.node-labels.configuration-type" is of type "distributed" Administrators can configure the source of the node labels provider by configuring this parameter
            
          Show
          rohithsharma Rohith Sharma K S added a comment - Adding to above From the below description, I assume that NodeLabelsProvider instance can be created only if configuratin-type is distributed. And I think before creating an instance for provider, there should be check for configuration-type also. When node labels "yarn.node-labels.configuration-type" is of type "distributed" Administrators can configure the source of the node labels provider by configuring this parameter
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          Overall patch look good to me.
          I have one doubt/comment. If I am missing anything correct me,

          1. NodeLabelsProvider is abstract class. Why it was not made as Interface?
          2. And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable.
            Any thoughts? cc :/Wangda Tan
          Show
          rohithsharma Rohith Sharma K S added a comment - Overall patch look good to me. I have one doubt/comment. If I am missing anything correct me, NodeLabelsProvider is abstract class. Why it was not made as Interface? And for each NodeLabelsProvide implementing any other way in future which require modification in NodeManager to create an instance of NodeLabelsProvider. Instead of that loading implemented class would be easier and also it is pluggable. Any thoughts? cc :/ Wangda Tan
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Rohith Sharma K S,
          Seems like test case failure is not related to the patch and the check style is related to number of lines in YARN Configuration, so i think jenkins output is fine, can you please take a look.

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Rohith Sharma K S , Seems like test case failure is not related to the patch and the check style is related to number of lines in YARN Configuration, so i think jenkins output is fine, can you please take a look.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 25m 56s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +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 9m 31s There were no new javac warning messages.
          +1 javadoc 13m 37s There were no new javadoc warning messages.
          +1 release audit 0m 31s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 3m 12s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 53s mvn install still works.
          +1 eclipse:eclipse 0m 42s The patch built with eclipse:eclipse.
          +1 findbugs 8m 12s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 30s Tests passed in hadoop-yarn-api.
          +1 yarn tests 2m 28s Tests passed in hadoop-yarn-common.
          +1 yarn tests 9m 31s Tests passed in hadoop-yarn-server-nodemanager.
          -1 yarn tests 61m 4s Tests failed in hadoop-yarn-server-resourcemanager.
              138m 4s  



          Reason Tests
          Timed out tests org.apache.hadoop.yarn.server.resourcemanager.TestRM



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12766273/YARN-2729.20151310-2.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 69b025d
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9421/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/9421/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 25m 56s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +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 9m 31s There were no new javac warning messages. +1 javadoc 13m 37s There were no new javadoc warning messages. +1 release audit 0m 31s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 3m 12s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 53s mvn install still works. +1 eclipse:eclipse 0m 42s The patch built with eclipse:eclipse. +1 findbugs 8m 12s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 30s Tests passed in hadoop-yarn-api. +1 yarn tests 2m 28s Tests passed in hadoop-yarn-common. +1 yarn tests 9m 31s Tests passed in hadoop-yarn-server-nodemanager. -1 yarn tests 61m 4s Tests failed in hadoop-yarn-server-resourcemanager.     138m 4s   Reason Tests Timed out tests org.apache.hadoop.yarn.server.resourcemanager.TestRM Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766273/YARN-2729.20151310-2.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 69b025d Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9421/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9421/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/9421/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Rohith Sharma K S

          When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error

          Thanks for pointing out this issue, Its happening only when the order of execution is ConfigNLP and ScriptNLP and the temp dir containing Yarn-Site.xml is getting deleted in @AfterClass

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Rohith Sharma K S When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error Thanks for pointing out this issue, Its happening only when the order of execution is ConfigNLP and ScriptNLP and the temp dir containing Yarn-Site.xml is getting deleted in @AfterClass
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 24m 0s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
          +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 9m 5s There were no new javac warning messages.
          +1 javadoc 11m 44s There were no new javadoc warning messages.
          +1 release audit 0m 27s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 2m 33s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 45s mvn install still works.
          +1 eclipse:eclipse 0m 39s The patch built with eclipse:eclipse.
          +1 findbugs 7m 8s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 26s Tests passed in hadoop-yarn-api.
          -1 yarn tests 2m 7s Tests failed in hadoop-yarn-common.
          +1 yarn tests 9m 8s Tests passed in hadoop-yarn-server-nodemanager.
          -1 yarn tests 62m 13s Tests failed in hadoop-yarn-server-resourcemanager.
              132m 7s  



          Reason Tests
          Failed unit tests hadoop.yarn.logaggregation.TestAggregatedLogsBlock
            hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12766229/YARN-2729.20151310-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / c60a16f
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9416/testReport/
          Java 1.7.0_55
          uname Linux asf901.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/9416/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 24m 0s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +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 9m 5s There were no new javac warning messages. +1 javadoc 11m 44s There were no new javadoc warning messages. +1 release audit 0m 27s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 33s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 45s mvn install still works. +1 eclipse:eclipse 0m 39s The patch built with eclipse:eclipse. +1 findbugs 7m 8s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 26s Tests passed in hadoop-yarn-api. -1 yarn tests 2m 7s Tests failed in hadoop-yarn-common. +1 yarn tests 9m 8s Tests passed in hadoop-yarn-server-nodemanager. -1 yarn tests 62m 13s Tests failed in hadoop-yarn-server-resourcemanager.     132m 7s   Reason Tests Failed unit tests hadoop.yarn.logaggregation.TestAggregatedLogsBlock   hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12766229/YARN-2729.20151310-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / c60a16f Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/trunkFindbugsWarningshadoop-yarn-server-nodemanager.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt hadoop-yarn-server-resourcemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9416/artifact/patchprocess/testrun_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9416/testReport/ Java 1.7.0_55 uname Linux asf901.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/9416/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the review Rohith Sharma K S,

          Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider?

          Yes as it extends AbstractNodeLabelsProvider it supports periodic execution.

          Is there anywher documented how the script output should be?

          yes its documented in yarn-default.xml and javadocs for the class ScriptBasedNodeLabelsProvider.java. Do you have any other place in mind where it can be updated?

          About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist.

          Yes this is issue has been already raised as YARN-4169, but nutting to do in specific to this jira and was not actually reproducible in my local setup.

          When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error

          Not facing this issue, will sync offline with you on this.

          Other issues have been handled in the patch along with one more minor issue which was got during testing, In ResourceTrackerService we are checking for distributed or delegated-centralized without checking for node labels enabled which caused unnecessary logs on RM side.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the review Rohith Sharma K S , Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider? Yes as it extends AbstractNodeLabelsProvider it supports periodic execution. Is there anywher documented how the script output should be? yes its documented in yarn-default.xml and javadocs for the class ScriptBasedNodeLabelsProvider.java . Do you have any other place in mind where it can be updated? About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist. Yes this is issue has been already raised as YARN-4169 , but nutting to do in specific to this jira and was not actually reproducible in my local setup. When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error Not facing this issue, will sync offline with you on this. Other issues have been handled in the patch along with one more minor issue which was got during testing, In ResourceTrackerService we are checking for distributed or delegated-centralized without checking for node labels enabled which caused unnecessary logs on RM side.
          Hide
          rohithsharma Rohith Sharma K S added a comment -

          Hi Naganarasinmha, thanks for delivering the patch.
          Some doubts and somments

          1. Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider?
          2. Does serviceStart in ScriptBasedNodeLabelsProvider really required since you are calling super.serviceStart();? I think it can be removed. The below code runs twice or attempt to run twice.
             @Override
              protected void serviceStart() throws Exception {
                super.serviceStart();
                if (intervalTime == DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER) {
                  // run at-least once so that labels are set
                  timerTask.run();
                }
              }
            
          3. In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent.
              protected void serviceStop() throws Exception {
                super.serviceStop();
                if (shexec != null) {
                  Process p = shexec.getProcess();
                  if (p != null) {
                    p.destroy();
                  }
                }
              }
            
          4. Is there anywher documented how the script output should be?
          5. The patch removes method convertToNodeLabelSet. I think some code optimization can be done keeping it. Currently, these removed lines are written in both providers.

          Tests :

          1. About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist.
          2. When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error. Are you getting below error? But individual class run test is passing. I think some cleanup has issue.
            java.io.FileNotFoundException: D:\Hadoop\hadoop-yarn-project\hadoop-yarn\hadoop-yarn-server\hadoop-yarn-server-nodemanager\target\org.apache.hadoop.yarn.server.nodemanager.nodelabels.TestConfigurationNodeLabelsProvider-localDir\yarn-site.xml (The system cannot find the path specified)
            
          Show
          rohithsharma Rohith Sharma K S added a comment - Hi Naganarasinmha, thanks for delivering the patch. Some doubts and somments Does ScriptBasedNodeLabelsProvider is periodically runs similar to ConfigurationNodeLabelsProvider? Does serviceStart in ScriptBasedNodeLabelsProvider really required since you are calling super.serviceStart(); ? I think it can be removed. The below code runs twice or attempt to run twice. @Override protected void serviceStart() throws Exception { super.serviceStart(); if (intervalTime == DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER) { // run at-least once so that labels are set timerTask.run(); } } In serviceStop in ScriptBasedNodeLabelsProvider , move super.serviceStop(); at end of serviceStop() so first it stops shexec and stopping parent. protected void serviceStop() throws Exception { super .serviceStop(); if (shexec != null ) { Process p = shexec.getProcess(); if (p != null ) { p.destroy(); } } } Is there anywher documented how the script output should be? The patch removes method convertToNodeLabelSet . I think some code optimization can be done keeping it. Currently, these removed lines are written in both providers. Tests : About the test TestNodeStatusUpdaterForLabels failing randomly.May be some race condition is exist. When I run tests locally at package level i.e org.apache.hadoop.yarn.server.nodemanager.nodelabels, all the tests cases in TestScriptBasedNodeLabelsProvider are failing with below error. Are you getting below error? But individual class run test is passing. I think some cleanup has issue. java.io.FileNotFoundException: D:\Hadoop\hadoop-yarn-project\hadoop-yarn\hadoop-yarn-server\hadoop-yarn-server-nodemanager\target\org.apache.hadoop.yarn.server.nodemanager.nodelabels.TestConfigurationNodeLabelsProvider-localDir\yarn-site.xml (The system cannot find the path specified)
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          TestNodeStatusUpdaterForLabels is passing locally and check style is for yarnConfiguration file size so not much can be done for it. Rohith Sharma K S can you please take a look at this jira ?

          Show
          Naganarasimha Naganarasimha G R added a comment - TestNodeStatusUpdaterForLabels is passing locally and check style is for yarnConfiguration file size so not much can be done for it. Rohith Sharma K S can you please take a look at this jira ?
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 19m 43s 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 8m 1s There were no new javac warning messages.
          +1 javadoc 10m 9s There were no new javadoc warning messages.
          +1 release audit 0m 21s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 53s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 29s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 4m 23s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api.
          +1 yarn tests 2m 11s Tests passed in hadoop-yarn-common.
          -1 yarn tests 9m 1s Tests failed in hadoop-yarn-server-nodemanager.
              58m 48s  



          Reason Tests
          Failed unit tests hadoop.yarn.server.nodemanager.TestNodeStatusUpdaterForLabels



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12762403/YARN-2729.20150925-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 83e65c5
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9267/testReport/
          Java 1.7.0_55
          uname Linux asf901.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/9267/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 19m 43s 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 8m 1s There were no new javac warning messages. +1 javadoc 10m 9s There were no new javadoc warning messages. +1 release audit 0m 21s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 53s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 29s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 4m 23s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api. +1 yarn tests 2m 11s Tests passed in hadoop-yarn-common. -1 yarn tests 9m 1s Tests failed in hadoop-yarn-server-nodemanager.     58m 48s   Reason Tests Failed unit tests hadoop.yarn.server.nodemanager.TestNodeStatusUpdaterForLabels Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12762403/YARN-2729.20150925-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 83e65c5 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/9267/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/9267/testReport/ Java 1.7.0_55 uname Linux asf901.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/9267/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda & Rohith Sharma K S,
          Please find the attached patch with the offline discussed modifications

          For Config :
          Currently we are supporting configuration param : "yarn.nodemanager.node-labels.provider.configured-node-labels"
          so instead can we have = > yarn.nodemanager.node-labels.provider.configured-node-partition which accepts only single partition and in future we can add yarn.nodemanager.node-labels.provider.configured-node-constraints which can support multiple constraints

          For Script :
          Currently in the patch i am searching for lines starting with "NODE_LABELS:" in script output and labels separated by "," though we currently support only a single Partition per node.
          So my suggestion we can currently support for parsing lines starting with "NODE_PARTITION:" and send the string following this pattern as Node Label. Later in future we can support for "NODE_CONSTRAINTS:" and accept multiple constraints for it.

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda & Rohith Sharma K S , Please find the attached patch with the offline discussed modifications For Config : Currently we are supporting configuration param : "yarn.nodemanager.node-labels.provider.configured-node-labels" so instead can we have = > yarn.nodemanager.node-labels.provider.configured-node-partition which accepts only single partition and in future we can add yarn.nodemanager.node-labels.provider.configured-node-constraints which can support multiple constraints For Script : Currently in the patch i am searching for lines starting with "NODE_LABELS:" in script output and labels separated by "," though we currently support only a single Partition per node. So my suggestion we can currently support for parsing lines starting with "NODE_PARTITION:" and send the string following this pattern as Node Label. Later in future we can support for "NODE_CONSTRAINTS:" and accept multiple constraints for it.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 18m 52s 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 1 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 23s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 51s The applied patch generated 1 new checkstyle issues (total was 211, now 211).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 27s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 4m 21s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api.
          +1 yarn tests 1m 58s Tests passed in hadoop-yarn-common.
          +1 yarn tests 7m 47s Tests passed in hadoop-yarn-server-nodemanager.
              55m 52s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12753202/YARN-2729.20150830-1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 837fb75
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt
          hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-api.txt
          hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-common.txt
          hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8946/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/8946/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 18m 52s 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 1 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 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 51s The applied patch generated 1 new checkstyle issues (total was 211, now 211). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 27s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 4m 21s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 yarn tests 0m 23s Tests passed in hadoop-yarn-api. +1 yarn tests 1m 58s Tests passed in hadoop-yarn-common. +1 yarn tests 7m 47s Tests passed in hadoop-yarn-server-nodemanager.     55m 52s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12753202/YARN-2729.20150830-1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 837fb75 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/diffcheckstylehadoop-yarn-api.txt hadoop-yarn-api test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-api.txt hadoop-yarn-common test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-common.txt hadoop-yarn-server-nodemanager test log https://builds.apache.org/job/PreCommit-YARN-Build/8946/artifact/patchprocess/testrun_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/8946/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/8946/console This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Attaching a patch to sync with the changes of YARN-2923.

          Show
          Naganarasimha Naganarasimha G R added a comment - Attaching a patch to sync with the changes of YARN-2923 .
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda & Vinod Kumar Vavilapalli,
          Any further thoughts on the above comments ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda & Vinod Kumar Vavilapalli , Any further thoughts on the above comments ?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Vinod Kumar Vavilapalli for replying,

          I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future?

          True, its better to think about it for both script(YARN-2729) and config (YARN-2923) based providers now itself if we are making it as public. Initial thought what i have is

          NODE_LABELS=<Label Name>|<Label Type>[,Labels]
          where Label Type = Partition, Constraint....
          and default if not specified can be Partition.
          

          Going further i think distributed labels will be more suitable for constraints/attribute YARN-3409 so we can think of having Constraint as default too. Also we need not specify Whether Partition is Exclusive or Non-Exclusive, it does not make significance from the NM as Exclusivity of partition labels are already specified while adding it to cluster labels set in RM.
          One More suggestion is In Node Label object, can we think of having Enum instead of isExclusive, Enum can have ExclusivePartion,NonExclusivePartition, (Constraint in future) and so on.

          Isn't AbstractNodeLabelsProvider a good place to do these steps?

          Well AbstractNodeLabelsProvider will only be applicable to the whitelisted providers (Config and script) currently, And also the purpose was for removing duplicate code related to Timertask and related configs. So is your suggestion to expose the AbstractNodeLabelsProvider to be a public interface ? or can we think of having intermediate manager class and have configurations for timer requirement and leave the NodeLabelsProvider interface as is. ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Vinod Kumar Vavilapalli for replying, I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future? True, its better to think about it for both script( YARN-2729 ) and config ( YARN-2923 ) based providers now itself if we are making it as public. Initial thought what i have is NODE_LABELS=<Label Name>|<Label Type>[,Labels] where Label Type = Partition, Constraint.... and default if not specified can be Partition. Going further i think distributed labels will be more suitable for constraints/attribute YARN-3409 so we can think of having Constraint as default too. Also we need not specify Whether Partition is Exclusive or Non-Exclusive, it does not make significance from the NM as Exclusivity of partition labels are already specified while adding it to cluster labels set in RM. One More suggestion is In Node Label object, can we think of having Enum instead of isExclusive, Enum can have ExclusivePartion,NonExclusivePartition, (Constraint in future) and so on. Isn't AbstractNodeLabelsProvider a good place to do these steps? Well AbstractNodeLabelsProvider will only be applicable to the whitelisted providers (Config and script) currently, And also the purpose was for removing duplicate code related to Timertask and related configs. So is your suggestion to expose the AbstractNodeLabelsProvider to be a public interface ? or can we think of having intermediate manager class and have configurations for timer requirement and leave the NodeLabelsProvider interface as is. ?
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.

          So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there? Tan, Wangda and you can share your opinion on this, based on it will do the modifications.

          I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future?

          These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider.

          Isn't AbstractNodeLabelsProvider a good place to do these steps?

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565 . So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there? Tan, Wangda and you can share your opinion on this, based on it will do the modifications. I think once we start marking this script-based provider feature as public, the expected output from the script will automatically become a public interface unless we explicitly say no. We should start thinking about this now to avoid uncertainty in the future? These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider. Isn't AbstractNodeLabelsProvider a good place to do these steps?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda

          1. rebased the patch on top of 3565
          2. Moved common code which was earlier here to YARN-2923, as 2923 jira will be going first
          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda rebased the patch on top of 3565 Moved common code which was earlier here to YARN-2923 , as 2923 jira will be going first
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the review comments Vinod Kumar Vavilapalli,

          SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes

          Actually these are not preffixes, as per Tan, Wangda's comment we had decided have whitelisting for provider : The option will be: yarn.node-labels.nm.provider = "config/script/other-class-name". . These are modifications for it.

          DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration

          Well As per one of the wangda's comment he had mentioned that possible values or default values of configurations had to be kept in YARNConfiguration, hence had placed it here, if required as per your comment can move it to AbstractNodeLabelsProvider

          LOG is not used anywhere

          Are the logs expected when the labels are set in setNodeLabels ? i can add here but anyway there were logs in NodeStatusUpdaterImpl on successfull and unsuccessfull attempts.

          BTW, assuming YARN-3565 goes in first, you will have to make some changes here.

          I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.

          Well was thinking about this while working on YARN-3565, but dint modify the NodeLabelsProvider as currently Labels(currently partitions) which needs to be sent from NM have to be one of RM's CLUSTER NodeLabel set. So exclusiveness need not be sent from NM to RM as currently we support specifying the exclusiveness only during adding clusterNode labels. So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there?
          Tan, Wangda and you can share your opinion on this, based on it will do the modifications.

          Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider.

          Well as per the class reponsibility i understand that NodeStatusUpdaterImpl is not supposed to have it but as it might be expected to be public we had to ensure that

          • For every heartbeat labels are sent across only if modified
          • doing basic validations before sending the modified labels

          These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider.
          Those changes were also from my previous patch, so no hard feelings in taking care of it if req .

          Can you add the documentation for setting this up too too?

          Well was planning to raise jira for updating documentation on top of NodeLabels but documentation for it is not yet completed. If required can just add some pdf here

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the review comments Vinod Kumar Vavilapalli , SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes Actually these are not preffixes, as per Tan, Wangda 's comment we had decided have whitelisting for provider : The option will be: yarn.node-labels.nm.provider = "config/script/other-class-name". . These are modifications for it. DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration Well As per one of the wangda's comment he had mentioned that possible values or default values of configurations had to be kept in YARNConfiguration, hence had placed it here, if required as per your comment can move it to AbstractNodeLabelsProvider LOG is not used anywhere Are the logs expected when the labels are set in setNodeLabels ? i can add here but anyway there were logs in NodeStatusUpdaterImpl on successfull and unsuccessfull attempts. BTW, assuming YARN-3565 goes in first, you will have to make some changes here. I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565 . Well was thinking about this while working on YARN-3565 , but dint modify the NodeLabelsProvider as currently Labels(currently partitions) which needs to be sent from NM have to be one of RM's CLUSTER NodeLabel set. So exclusiveness need not be sent from NM to RM as currently we support specifying the exclusiveness only during adding clusterNode labels. So IMHO if there is plan to make this interface public & stable then would be better do these changes now itself if not it would better done after requirement for constraint labels, so that more clarity on structure would be there? Tan, Wangda and you can share your opinion on this, based on it will do the modifications. Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider. Well as per the class reponsibility i understand that NodeStatusUpdaterImpl is not supposed to have it but as it might be expected to be public we had to ensure that For every heartbeat labels are sent across only if modified doing basic validations before sending the modified labels These needs to be done irrespective of the label provider (system or user's) hence kept it in NodeStatusUpdaterImpl , but if req to be moved out then we need to bring in some intermediate manager(/helper/delegator) class between NodeStatusUpdaterImpl and NodeLabelsProvider. Those changes were also from my previous patch, so no hard feelings in taking care of it if req . Can you add the documentation for setting this up too too? Well was planning to raise jira for updating documentation on top of NodeLabels but documentation for it is not yet completed. If required can just add some pdf here
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Tx for working on this Naganarasimha G R!

          Comments on the patch

          YarnConfiguration

          • The configuration properties should be yarn.nodemanager.node-labels.* instead of yarn.node-labels.nm.*
          • SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes.
          • NODE_LABELS_PROVIDER can be removed as you already have NODE_LABELS_PROVIDER_PREFIX, delete it.
          • None of the prefix constants need to be public. I think it was a mistake all along, we don't need to continue that.
          • DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration
          • Some rename suggestions for the configs
            • DISTRIBUTED_NODE_LABELS_PREFIX -> NM_NODE_LABELS_PREFIX
            • NODE_LABELS_PROVIDER -> NM_NODE_LABELS_PROVIDER
            • Similarly change to NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS
              NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS
              DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS
              DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS
              NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PREFIX
              NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PATH
              NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_SCRIPT_OPTS

          AbstractNodeLabelsProvider

          • LOG is not used anywhere
          • Add @Override annotations to the extended methods from the parent.
          • serviceInit() should call super's serviceInit()
            ScriptBasedNodeLabelsProvider
          • NodeLabelsScriptRunner can access scriptTimeout from the parent class but not scriptArgs. I think you should simply access scriptArgs directly and drop it from NodeLabelsScriptRunner's constructor.
          • I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565.
          • Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider.
            Can you add the documentation for setting this up too too?

          BTW, assuming YARN-3565 goes in first, you will have to make some changes here.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Tx for working on this Naganarasimha G R ! Comments on the patch YarnConfiguration The configuration properties should be yarn.nodemanager.node-labels.* instead of yarn.node-labels.nm.* SCRIPT_NODE_LABELS_PROVIDER and CONFIG_NODE_LABELS_PROVIDER are not needed, delete them, you have separate constants for their prefixes. NODE_LABELS_PROVIDER can be removed as you already have NODE_LABELS_PROVIDER_PREFIX, delete it. None of the prefix constants need to be public. I think it was a mistake all along, we don't need to continue that. DISABLE_NODE_LABELS_PROVIDER_FETCH_TIMER doesn't need to be in YarnConfiguration Some rename suggestions for the configs DISTRIBUTED_NODE_LABELS_PREFIX -> NM_NODE_LABELS_PREFIX NODE_LABELS_PROVIDER -> NM_NODE_LABELS_PROVIDER Similarly change to NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_INTERVAL_MS DEFAULT_NM_NODE_LABELS_PROVIDER_FETCH_TIMEOUT_MS NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PREFIX NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_PATH NM_SCRIPT_BASED_NODE_LABELS_PROVIDER_SCRIPT_OPTS AbstractNodeLabelsProvider LOG is not used anywhere Add @Override annotations to the extended methods from the parent. serviceInit() should call super's serviceInit() ScriptBasedNodeLabelsProvider NodeLabelsScriptRunner can access scriptTimeout from the parent class but not scriptArgs. I think you should simply access scriptArgs directly and drop it from NodeLabelsScriptRunner's constructor. I think the format expected from the command should be more structured. Specifically as we expect more per-label attributes in line with YARN-3565 . Not caused by your patch but worth fixing here. NodeStatusUpdaterImpl shouldn't worry about invalid label-set, previous-valid-labels and label validation. You should move all that functionality into NodeLabelsProvider. Can you add the documentation for setting this up too too? BTW, assuming YARN-3565 goes in first, you will have to make some changes here.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Tan, Wangda, any updates on this patch required ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Tan, Wangda , any updates on this patch required ?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda, can you please take a look @ the reworked patch ... ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , can you please take a look @ the reworked patch ... ?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Tan, Wangda ,
          Findbugs are not related to this jira ...

          Show
          Naganarasimha Naganarasimha G R added a comment - Tan, Wangda , Findbugs are not related to this jira ...
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12709432/YARN-2729.20150404-1.patch
          against trunk revision ef591b1.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7219//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/7219//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7219//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12709432/YARN-2729.20150404-1.patch against trunk revision ef591b1. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7219//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-YARN-Build/7219//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-common.html Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7219//console This message is automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda,
          Uploading the patch with fixing the review comments given earlier...

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , Uploading the patch with fixing the review comments given earlier...
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here).

          agree, i dont mind redoing, as long as its for better reason and i was expecting for changes here anyway.
          For other comments on configuration will get it done,

          I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc. Can you make an abstract class and inherited by ScriptBased?

          I can do this (which i feel is correct), but if we do this then it might not be possible to generalize much NodeHealthSCriptRunner and ScriptBasedNodeLabelsProvider, which i feel should be ok

          checkAndThrowLabelName should be called in NodeStatusUpdaterImpl

          In a way it would be better in NodeStatusUpdaterImpl as we support external class to be a provider, but earlier thought it would not be good for additional checks as part of heart beat flow

          label need to be trim() when called checkAndThrowLabelName(...)

          Not required as checkAndThrowLabelName takes care of it, but missing test case will add it for NodeStatusUpdaterImpl
          Other issues will rework in next patch

          Show
          Naganarasimha Naganarasimha G R added a comment - Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here). agree, i dont mind redoing, as long as its for better reason and i was expecting for changes here anyway. For other comments on configuration will get it done, I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc. Can you make an abstract class and inherited by ScriptBased? I can do this (which i feel is correct), but if we do this then it might not be possible to generalize much NodeHealthSCriptRunner and ScriptBasedNodeLabelsProvider, which i feel should be ok checkAndThrowLabelName should be called in NodeStatusUpdaterImpl In a way it would be better in NodeStatusUpdaterImpl as we support external class to be a provider, but earlier thought it would not be good for additional checks as part of heart beat flow label need to be trim() when called checkAndThrowLabelName(...) Not required as checkAndThrowLabelName takes care of it, but missing test case will add it for NodeStatusUpdaterImpl Other issues will rework in next patch
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch
          against trunk revision 6a6a59d.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7205//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7205//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch against trunk revision 6a6a59d. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7205//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7205//console This message is automatically generated.
          Hide
          leftnoteasy Wangda Tan added a comment -

          Some comments:
          1) Configuration:
          Instead of distributed_node_labels_prefix, do you think is it better to name it : "yarn.node-labels.nm.provider"? The "distributed.node-labels-provider" doesn't clearly mentioned it runs in NM side.

          I don't want to expose class to config unless it is necessary, now we have two options, one is script-based and another is config-based. We can set the two as "white-list", if a given value is not in whitelist, trying to get a class from the name. So the option will be: yarn.node-labels.nm.provider = "config/script/other-class-name".

          Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here).
          If you agree above, the name could be: yarn.node-labels.nm.provider-fetch-interval-ms (and provider-fetch-timeout-ms)

          And script-related options could be:
          yarn.node-labels.nm.provider.script.path/opts

          2) Implementation of ScriptBasedNodeLabelsProvider
          I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc.
          Can you make an abstract class and inherited by ScriptBased?

          DISABLE_TIMER_CONFIG should be a part of YarnConfiguration, all default of configurations should be a part of YarnConfiguration.

          canRun -> something like verifyConfiguredScript, and directly throw exception when something wrong (so that admin can know what really happened, such as file not found, doesn't have execution permission, etc.), and it should be private non-static.

          checkAndThrowLabelName should be called in NodeStatusUpdaterImpl

          label need to be trim() when called checkAndThrowLabelName(...)

          Show
          leftnoteasy Wangda Tan added a comment - Some comments: 1) Configuration: Instead of distributed_node_labels_prefix, do you think is it better to name it : "yarn.node-labels.nm.provider"? The "distributed.node-labels-provider" doesn't clearly mentioned it runs in NM side. I don't want to expose class to config unless it is necessary, now we have two options, one is script-based and another is config-based. We can set the two as "white-list", if a given value is not in whitelist, trying to get a class from the name. So the option will be: yarn.node-labels.nm.provider = "config/script/other-class-name". Revisted interval, I think it's better to make it to be provider configuration instead of script-provider-only configuration. Since config/script will share it (I remember I have some back-and-forth opinions here). If you agree above, the name could be: yarn.node-labels.nm.provider-fetch-interval-ms (and provider-fetch-timeout-ms) And script-related options could be: yarn.node-labels.nm.provider.script.path/opts 2) Implementation of ScriptBasedNodeLabelsProvider I feel like ScriptBased and ConfigBased can share some implementations, they will all init a time task, get interval and run, check timeout (meaningless for config-based), etc. Can you make an abstract class and inherited by ScriptBased? DISABLE_TIMER_CONFIG should be a part of YarnConfiguration, all default of configurations should be a part of YarnConfiguration. canRun -> something like verifyConfiguredScript, and directly throw exception when something wrong (so that admin can know what really happened, such as file not found, doesn't have execution permission, etc.), and it should be private non-static. checkAndThrowLabelName should be called in NodeStatusUpdaterImpl label need to be trim() when called checkAndThrowLabelName(...)
          Hide
          leftnoteasy Wangda Tan added a comment -

          Apparently Jenkins ran wrong tests, rekicked Jenkins.

          Show
          leftnoteasy Wangda Tan added a comment - Apparently Jenkins ran wrong tests, rekicked Jenkins.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Testcase failures not related to my jira

          Show
          Naganarasimha Naganarasimha G R added a comment - Testcase failures not related to my jira
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch
          against trunk revision c94d594.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
          org.apache.hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA

          The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
          org.apache.hadoop.hdfs.TestLeaseRecovery2

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7193//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7193//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708788/YARN-2729.20150402-1.patch against trunk revision c94d594. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover org.apache.hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestPipelinesFailover org.apache.hadoop.hdfs.TestLeaseRecovery2 Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7193//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7193//console This message is automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Attaching a patch to disable the timer and include the missing files

          Show
          Naganarasimha Naganarasimha G R added a comment - Attaching a patch to disable the timer and include the missing files
          Hide
          leftnoteasy Wangda Tan added a comment -

          i dint want to add a new configuration to enable or disable the timer

          Agree,

          I think acceptable values of interval-ms should be -1 (do not launch timer) and > 0, other values should be illegal.

          Show
          leftnoteasy Wangda Tan added a comment - i dint want to add a new configuration to enable or disable the timer Agree, I think acceptable values of interval-ms should be -1 (do not launch timer) and > 0, other values should be illegal.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks Tan, Wangda for the review,

          I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched.

          yes its configurable, but was just trying to avoid another thread of timer if not required and also was checking with you as i was not sure that it would be a better approach to take some decision based on the value configured for interval-ms (0 or -1 to not to launch the timer task) and i dint want to add a new configuration to enable or disable the timer

          You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority).

          yep felt so... hence dint further work on it.

          And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch?

          my mistake will update along with the conclusion for the first point ....

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks Tan, Wangda for the review, I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched. yes its configurable, but was just trying to avoid another thread of timer if not required and also was checking with you as i was not sure that it would be a better approach to take some decision based on the value configured for interval-ms (0 or -1 to not to launch the timer task) and i dint want to add a new configuration to enable or disable the timer You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority). yep felt so... hence dint further work on it. And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch? my mistake will update along with the conclusion for the first point ....
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Naganarasimha G R,
          Sorry for my late response,

          Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ?

          Interval-ms is configurable as we decided, I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched.

          whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

          You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority).

          And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch?

          Show
          leftnoteasy Wangda Tan added a comment - Hi Naganarasimha G R , Sorry for my late response, Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ? Interval-ms is configurable as we decided, I think it will be fine if we don't launch timer (if don't launch timer is just a simple change), or you can just set timer to a very big value that it will never get launched. whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ? You already have a ScriptBasedNodeLabelsProvider, use it now and we can see if there's chance to merge it to NodeHealthScriptRunner in the future (low priority). And did you forgot add "ScriptBasedNodeLabelsProvider" in your latest patch?
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708567/YARN-2729.20150401-1.patch
          against trunk revision 7610925.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

          org.apache.hadoop.yarn.server.nodemanager.TestNodeManager

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7182//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7182//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708567/YARN-2729.20150401-1.patch against trunk revision 7610925. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.TestNodeManager Test results: https://builds.apache.org/job/PreCommit-YARN-Build/7182//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7182//console This message is automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda,
          Not sure the reason for failure in applying the patch ...I am able to do this in local setup (succeded after fuzz offset 65 lines), so i am rebasing it and uploading it.
          Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ?
          Also provide your feedback on earlier discussion :
          whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , Not sure the reason for failure in applying the patch ...I am able to do this in local setup (succeded after fuzz offset 65 lines), so i am rebasing it and uploading it. Also as per Jian Fang he wanted some kind of configuration such that the timer task doesn't run, as their usecase requires updating of node labels only during registration and do not want timer to continuously running for this. So do we need to support this (as unnecessary additional timer thread will be running) ? and if so can we check for interval-ms and decide based on its value (0 or -1 to not to launch the timer task) ? Also provide your feedback on earlier discussion : whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12706372/YARN-2729.20150322-1.patch
          against trunk revision 1a495fb.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7157//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12706372/YARN-2729.20150322-1.patch against trunk revision 1a495fb. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-YARN-Build/7157//console This message is automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda,
          Have rebased the patch based on changes in yarn-2495,
          one open point, whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , Have rebased the patch based on changes in yarn-2495, one open point, whether to do the changes required to make HadoopCommon's NodeHealthScriptRunner reuseable (In seperate jira) and rework on ScriptBasedNodeLabelsProvider ?
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda
          Rebasing the patch
          Removing dependency on yarn-2923,
          Changed configuration names to suit current conf suggestions
          Have made it to fail fast on invalid configurations.

          If the above modifications are fine then will start looking into changes required to make HadoopCommon's NodeHealthScriptRunner to make it reuseable. (In seperate jira)

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda Rebasing the patch Removing dependency on yarn-2923, Changed configuration names to suit current conf suggestions Have made it to fail fast on invalid configurations. If the above modifications are fine then will start looking into changes required to make HadoopCommon's NodeHealthScriptRunner to make it reuseable. (In seperate jira)
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Re basing the patch and incorporating the changes based on yarn-2923 (changes in yarn configuration)

          Show
          Naganarasimha Naganarasimha G R added a comment - Re basing the patch and incorporating the changes based on yarn-2923 (changes in yarn configuration)
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Updating patch with Tan, Wangda's review comments

          Show
          Naganarasimha Naganarasimha G R added a comment - Updating patch with Tan, Wangda 's review comments
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Its dependent on YARN-2495 and once that is commited will enable patch for this

          Show
          Naganarasimha Naganarasimha G R added a comment - Its dependent on YARN-2495 and once that is commited will enable patch for this
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Naganarasimha G R,
          IIRC, the script based patch should be based on YARN-2495, and we should create a script-based labels provider extend NodeLabelsProviderService, correct? But I haven't seen much relationship between this and YARN-2495 besides configuration options.

          Please let me know if I understood incorrectly.

          Thanks,
          Wangda

          Show
          leftnoteasy Wangda Tan added a comment - Hi Naganarasimha G R , IIRC, the script based patch should be based on YARN-2495 , and we should create a script-based labels provider extend NodeLabelsProviderService, correct? But I haven't seen much relationship between this and YARN-2495 besides configuration options. Please let me know if I understood incorrectly. Thanks, Wangda
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12678261/YARN-2729.20141031-1.patch
          against trunk revision d1828d9.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5653//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12678261/YARN-2729.20141031-1.patch against trunk revision d1828d9. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5653//console This message is automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          submitting patch

          Show
          Naganarasimha Naganarasimha G R added a comment - submitting patch
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Updated with review comments and test cases

          Show
          Naganarasimha Naganarasimha G R added a comment - Updated with review comments and test cases
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Naganarasimha G R,
          For 1.
          I think what I meant is just check label name locally in NM.
          If NM register/heartbeat with RM failed with labels, I should have commented in YARN-2495: https://issues.apache.org/jira/browse/YARN-2495?focusedCommentId=14184146&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14184146.

          use a flag say if the last sync about node labels is success or not

          This should be also your proposal

          For 2.
          I think for now, let's keep it simple, I just don't want to change too much for what we have in NodeLabelsManager . As our discussion in YARN-2495, in the future we might need return rejected labels. So we can change this that that time. What do you think?

          Thanks,
          Wangda

          Show
          leftnoteasy Wangda Tan added a comment - Hi Naganarasimha G R , For 1. I think what I meant is just check label name locally in NM. If NM register/heartbeat with RM failed with labels, I should have commented in YARN-2495 : https://issues.apache.org/jira/browse/YARN-2495?focusedCommentId=14184146&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14184146 . use a flag say if the last sync about node labels is success or not This should be also your proposal For 2. I think for now, let's keep it simple, I just don't want to change too much for what we have in NodeLabelsManager . As our discussion in YARN-2495 , in the future we might need return rejected labels. So we can change this that that time. What do you think? Thanks, Wangda
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda,

          And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM.

          Two concerns if we do this:
          1. Two times labels will get validated, once in NM and once in RM(NodeLabelsManager.replaceNodeLabels ). And anyway as the exception message is planned to be propagated till NM and getting logged in NM, would this be required.?
          2. Currently checkAndThrowLabelName is validating and throwing exception for a singe label, i think it would better to accept a collection and show that which all labels in the collection are not valid. So that user need not wait till multiple timer runs to identify all the set of valid labels. so was planning to rename the method to $$ validateLabels(Collection<String> labels) $$ and make it static to get it accessible for (conf & script)NodeLabelsProvider (if this basic validation is required in NM too)

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda , And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM. Two concerns if we do this: 1. Two times labels will get validated, once in NM and once in RM(NodeLabelsManager.replaceNodeLabels ). And anyway as the exception message is planned to be propagated till NM and getting logged in NM, would this be required.? 2. Currently checkAndThrowLabelName is validating and throwing exception for a singe label, i think it would better to accept a collection and show that which all labels in the collection are not valid. So that user need not wait till multiple timer runs to identify all the set of valid labels. so was planning to rename the method to $$ validateLabels(Collection<String> labels) $$ and make it static to get it accessible for (conf & script)NodeLabelsProvider (if this basic validation is required in NM too)
          Hide
          leftnoteasy Wangda Tan added a comment -

          Hi Naganarasimha G R,
          After looked at the patch, and implementation of NodeHealthMonitorExecutor, I think it may not proper to put a "common" class for them to yarn.utils. Reasons are:

          1. We already have a ShellCommandExecutor, that is enough to run a general script, and set timeout, fetch output, etc.
          2. The purpose of NodeHealthScriptRunner and NodeLabelScriptRunner (The name in my mind ) are different: NodeHealthScriptRunner needs do a lot of checks for exception because it need to tell RM what actual happens, but NodeLabelScriptRunner doesn't need so, just a list of successfully fetched labels or no, if failed to run script, log errors to log should be enough.

          So I suggest do to this like NodeHealthScriptRunner, and remove what we don't need, it should be able to,

          1. If the script is failed or timeout or exception happened, and log them properly
          2. Upon successfully execute script, parse output for labels. I think we can define, lines start with "NODE_LABELS:" are node labels, and splited by ",". Like:
            NODE_LABELS:java, windows\n
            ...(other messages)
            NODE_LABELS:gpu,x86
            The result should be {"java", "windows", "gpu", "x86"}

            .
            And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM.

          Does this make sense to you?

          And in addition, NM_SCRIPT_LABELS_PROVIDER_PREFIX -> NM_NODE_LABELS_SCRIPT_PROVIDER_PREFIX is more clear to me, similar to others

          Thanks,
          Wangda

          Show
          leftnoteasy Wangda Tan added a comment - Hi Naganarasimha G R , After looked at the patch, and implementation of NodeHealthMonitorExecutor, I think it may not proper to put a "common" class for them to yarn.utils. Reasons are: We already have a ShellCommandExecutor, that is enough to run a general script, and set timeout, fetch output, etc. The purpose of NodeHealthScriptRunner and NodeLabelScriptRunner (The name in my mind ) are different: NodeHealthScriptRunner needs do a lot of checks for exception because it need to tell RM what actual happens, but NodeLabelScriptRunner doesn't need so, just a list of successfully fetched labels or no, if failed to run script, log errors to log should be enough. So I suggest do to this like NodeHealthScriptRunner, and remove what we don't need, it should be able to, If the script is failed or timeout or exception happened, and log them properly Upon successfully execute script, parse output for labels. I think we can define, lines start with "NODE_LABELS:" are node labels, and splited by ",". Like: NODE_LABELS:java, windows\n ...(other messages) NODE_LABELS:gpu,x86 The result should be {"java", "windows", "gpu", "x86"} . And you need make checkAndThrowLabelName in CommonsNodeLabelsManager public, check the labels if valid before send to RM. Does this make sense to you? And in addition, NM_SCRIPT_LABELS_PROVIDER_PREFIX -> NM_NODE_LABELS_SCRIPT_PROVIDER_PREFIX is more clear to me, similar to others Thanks, Wangda
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Tan, Wangda
          As per our discussion I have done the scriptNodeLabelsPRovider similar to the NodeHealthCheckerService. And moved the yarn configuration changes related to the script into this jira itself. Please review

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Tan, Wangda As per our discussion I have done the scriptNodeLabelsPRovider similar to the NodeHealthCheckerService. And moved the yarn configuration changes related to the script into this jira itself. Please review
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Attaching the WIP patch for this part of the issue...

          Show
          Naganarasimha Naganarasimha G R added a comment - Attaching the WIP patch for this part of the issue...

            People

            • Assignee:
              Naganarasimha Naganarasimha G R
              Reporter:
              Naganarasimha Naganarasimha G R
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development