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

ContainersAggregated CPU resource utilization reports negative usage in first few heartbeats

    Details

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

      Description

      NodeManager reports ContainerAggregated CPU resource utilization as -ve value in first few heartbeats cycles. I added a new debug print and received below values from heartbeats.

      INFO org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.ContainersMonitorImpl: ContainersResource Utilization : CpuTrackerUsagePercent : -1.0 
      INFO org.apache.hadoop.yarn.server.nodemanager.containermanager.monitor.ContainersMonitorImpl:ContainersResource Utilization :  CpuTrackerUsagePercent : 198.94598
      

      Its better we send 0 as CPU usage rather than sending a negative values in heartbeats eventhough its happening in only first few heartbeats.

      1. 0001-YARN-4308.patch
        2 kB
        Sunil G
      2. 0002-YARN-4308.patch
        3 kB
        Sunil G
      3. 0003-YARN-4308.patch
        6 kB
        Sunil G
      4. 0004-YARN-4308.patch
        6 kB
        Sunil G
      5. 0005-YARN-4308.patch
        10 kB
        Sunil G
      6. 0006-YARN-4308.patch
        13 kB
        Sunil G
      7. 0007-YARN-4308.patch
        13 kB
        Sunil G
      8. 0008-YARN-4308.patch
        14 kB
        Sunil G
      9. 0009-YARN-4308.patch
        14 kB
        Sunil G
      10. 0010-YARN-4308.patch
        15 kB
        Sunil G

        Issue Links

          Activity

          Hide
          sunilg Sunil G added a comment -

          Thanks NGarla_Unused for the review and commit. Thanks Karthik Kambatla for the initial patch and thanks Junping Du, Daniel Templeton for the review.

          Show
          sunilg Sunil G added a comment - Thanks NGarla_Unused for the review and commit. Thanks Karthik Kambatla for the initial patch and thanks Junping Du , Daniel Templeton for the review.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9934 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9934/)
          YARN-4308. ContainersAggregated CPU resource utilization reports (naganarasimha_gr: rev 1500a0a3009e453c9f05a93df7a78b4e185eef30)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/WindowsBasedProcessTree.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/MockResourceCalculatorProcessTree.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/MockCPUResourceCalculatorProcessTree.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainersMonitorResourceChange.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9934 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9934/ ) YARN-4308 . ContainersAggregated CPU resource utilization reports (naganarasimha_gr: rev 1500a0a3009e453c9f05a93df7a78b4e185eef30) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/WindowsBasedProcessTree.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/MockResourceCalculatorProcessTree.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/MockCPUResourceCalculatorProcessTree.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/ContainersMonitorImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ResourceCalculatorProcessTree.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/monitor/TestContainersMonitorResourceChange.java
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Committed it to trunk & branch2. Thanks Sunil G for working on this jira and Karthik Kambatla ,Daniel Templeton & Junping Du for the reviews.

          Show
          Naganarasimha Naganarasimha G R added a comment - Committed it to trunk & branch2. Thanks Sunil G for working on this jira and Karthik Kambatla , Daniel Templeton & Junping Du for the reviews.
          Hide
          templedf Daniel Templeton added a comment -

          +1 from me.

          Show
          templedf Daniel Templeton added a comment - +1 from me.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          thanks for the patch Sunil G,

          Its not very much of a strong reason, but it seems good to take it separate.

          Agree its not too strong reason but i am ok with it.

          Overall patch looks fine if no other person has any concern will commit it!

          Show
          Naganarasimha Naganarasimha G R added a comment - thanks for the patch Sunil G , Its not very much of a strong reason, but it seems good to take it separate. Agree its not too strong reason but i am ok with it. Overall patch looks fine if no other person has any concern will commit it!
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 32s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 7m 4s trunk passed
          +1 compile 2m 8s trunk passed
          +1 checkstyle 0m 38s trunk passed
          +1 mvnsite 1m 0s trunk passed
          +1 mvneclipse 0m 24s trunk passed
          +1 findbugs 1m 44s trunk passed
          +1 javadoc 0m 47s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 47s the patch passed
          +1 compile 2m 9s the patch passed
          +1 javac 2m 9s the patch passed
          -1 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 82 unchanged - 3 fixed = 83 total (was 85)
          +1 mvnsite 0m 57s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          +1 findbugs 2m 0s the patch passed
          +1 javadoc 0m 47s the patch passed
          +1 unit 2m 16s hadoop-yarn-common in the patch passed.
          +1 unit 11m 21s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          36m 59s



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 32s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 7m 4s trunk passed +1 compile 2m 8s trunk passed +1 checkstyle 0m 38s trunk passed +1 mvnsite 1m 0s trunk passed +1 mvneclipse 0m 24s trunk passed +1 findbugs 1m 44s trunk passed +1 javadoc 0m 47s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 47s the patch passed +1 compile 2m 9s the patch passed +1 javac 2m 9s the patch passed -1 checkstyle 0m 37s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 82 unchanged - 3 fixed = 83 total (was 85) +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. +1 findbugs 2m 0s the patch passed +1 javadoc 0m 47s the patch passed +1 unit 2m 16s hadoop-yarn-common in the patch passed. +1 unit 11m 21s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 36m 59s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808623/0010-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f98c3dbce36c 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e620530 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11870/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11870/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11870/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thanks Daniel Templeton. I have added messages for assert conditions. There were couple of methods in MockCPUResourceCalculatorProcessTree which were not needed for this test. I have removed them too as part of this new patch.
          NGarla_Unused, thank you for the thoughts. I had a similar line of thoughts before choosing this new Mock class for CPU resource util class.

          • There were 4 more methods which needed to be overridden along with getCPUPercentage. So a separate mock class was looking more cleaner and readable.
          • Currently we made some comments and java doc to point out that -1 has to be returned only in the first run where data is not enough to produce CPU usage. Still any new impl's can try to come with cases where -1 may be needed to return in b/w cycles also. So a separate mock class can help in writing all these specific test logics as needed. This is just done to make a CPU specific test class so that any mock work can be done as common.

          Its not very much of a strong reason,but it seems good to take it separate. More thoughts are welcome

          Show
          sunilg Sunil G added a comment - Thanks Daniel Templeton . I have added messages for assert conditions. There were couple of methods in MockCPUResourceCalculatorProcessTree which were not needed for this test. I have removed them too as part of this new patch. NGarla_Unused , thank you for the thoughts. I had a similar line of thoughts before choosing this new Mock class for CPU resource util class. There were 4 more methods which needed to be overridden along with getCPUPercentage . So a separate mock class was looking more cleaner and readable. Currently we made some comments and java doc to point out that -1 has to be returned only in the first run where data is not enough to produce CPU usage. Still any new impl's can try to come with cases where -1 may be needed to return in b/w cycles also. So a separate mock class can help in writing all these specific test logics as needed. This is just done to make a CPU specific test class so that any mock work can be done as common. Its not very much of a strong reason,but it seems good to take it separate. More thoughts are welcome
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Hi Sunil G,
          Overall the changes looks good but we could have avoided MockCPUResourceCalculatorProcessTree and just used Mockito.when along with Mockito.thenReturn(Value1,value2...). thoughts?

          Show
          Naganarasimha Naganarasimha G R added a comment - Hi Sunil G , Overall the changes looks good but we could have avoided MockCPUResourceCalculatorProcessTree and just used Mockito.when along with Mockito.thenReturn(Value1,value2...) . thoughts?
          Hide
          templedf Daniel Templeton added a comment -

          Thanks, Sunil G! Tests look good. It would be nice to add messages to your asserts. It would also be nice to add javadocs to the constructor and non-overridden methods in MockCPUResourceCalculatorProcessTree.

          Show
          templedf Daniel Templeton added a comment - Thanks, Sunil G ! Tests look good. It would be nice to add messages to your asserts. It would also be nice to add javadocs to the constructor and non-overridden methods in MockCPUResourceCalculatorProcessTree .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 29s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 11s Maven dependency ordering for branch
          +1 mvninstall 6m 24s trunk passed
          +1 compile 2m 1s trunk passed
          +1 checkstyle 0m 37s trunk passed
          +1 mvnsite 0m 57s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 1m 38s trunk passed
          +1 javadoc 0m 46s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 47s the patch passed
          +1 compile 2m 0s the patch passed
          +1 javac 2m 0s the patch passed
          -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 82 unchanged - 3 fixed = 83 total (was 85)
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 52s the patch passed
          +1 javadoc 0m 43s the patch passed
          +1 unit 2m 10s hadoop-yarn-common in the patch passed.
          +1 unit 11m 0s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          34m 58s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808233/0009-YARN-4308.patch
          JIRA Issue YARN-4308
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9668391b94b5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 106234d
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11850/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11850/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11850/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 29s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 11s Maven dependency ordering for branch +1 mvninstall 6m 24s trunk passed +1 compile 2m 1s trunk passed +1 checkstyle 0m 37s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 1m 38s trunk passed +1 javadoc 0m 46s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 47s the patch passed +1 compile 2m 0s the patch passed +1 javac 2m 0s the patch passed -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 82 unchanged - 3 fixed = 83 total (was 85) +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 52s the patch passed +1 javadoc 0m 43s the patch passed +1 unit 2m 10s hadoop-yarn-common in the patch passed. +1 unit 11m 0s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 34m 58s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808233/0009-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9668391b94b5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 106234d Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11850/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11850/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11850/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Fixed few checkstyle issues.

          Show
          sunilg Sunil G added a comment - Fixed few checkstyle issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 5m 56s trunk passed
          +1 compile 1m 54s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 29s trunk passed
          +1 javadoc 0m 44s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 44s the patch passed
          +1 compile 1m 53s the patch passed
          +1 javac 1m 53s the patch passed
          -1 checkstyle 0m 32s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 82 unchanged - 3 fixed = 85 total (was 85)
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 44s the patch passed
          +1 javadoc 0m 39s the patch passed
          +1 unit 2m 6s hadoop-yarn-common in the patch passed.
          +1 unit 10m 50s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          33m 15s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808201/0008-YARN-4308.patch
          JIRA Issue YARN-4308
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5b4bad3d414a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 106234d
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11848/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11848/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11848/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 5m 56s trunk passed +1 compile 1m 54s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 29s trunk passed +1 javadoc 0m 44s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 44s the patch passed +1 compile 1m 53s the patch passed +1 javac 1m 53s the patch passed -1 checkstyle 0m 32s hadoop-yarn-project/hadoop-yarn: The patch generated 3 new + 82 unchanged - 3 fixed = 85 total (was 85) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 44s the patch passed +1 javadoc 0m 39s the patch passed +1 unit 2m 6s hadoop-yarn-common in the patch passed. +1 unit 10m 50s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 33m 15s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808201/0008-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5b4bad3d414a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 106234d Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11848/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11848/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11848/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thanks Daniel Templeton for pointing out java doc issue.
          Handled all comments and I have some points to add to the sleep issue which you mentioned.

          MonitoringThread iterates through the process tree of all running containers and sets its utilization. It also checks for memory over utilization and kills such containers. However in our current scenario, we are verifying only CPU values. I also wanted to avoid these sleeps in first place and wanted to verify based on some events or processed values. However we have only containerResourceUtilization value to look for as a change and a default value of ResourceUtilization.newInstance(0, 0, 0.0f) is set already. So if CPU readings are coming as 0, this will be still 0. Hence I can do this check only for the test case which I added as CPU value of 50 was returned by MockCPUResourceCalculatorProcessTree.
          We can see whether it can be generalized for similar cases in future.

          Show
          sunilg Sunil G added a comment - Thanks Daniel Templeton for pointing out java doc issue. Handled all comments and I have some points to add to the sleep issue which you mentioned. MonitoringThread iterates through the process tree of all running containers and sets its utilization. It also checks for memory over utilization and kills such containers. However in our current scenario, we are verifying only CPU values. I also wanted to avoid these sleeps in first place and wanted to verify based on some events or processed values. However we have only containerResourceUtilization value to look for as a change and a default value of ResourceUtilization.newInstance(0, 0, 0.0f) is set already. So if CPU readings are coming as 0, this will be still 0. Hence I can do this check only for the test case which I added as CPU value of 50 was returned by MockCPUResourceCalculatorProcessTree . We can see whether it can be generalized for similar cases in future.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks, Sunil G. Looks like checkstyle caught something I should have: you're missing javadocs on the MockCPUResourceCalculatorProcessTree class. Since there's another patch coming, I have a few more nits to pick that I just noticed:

          • In MockResourceCalculatorProcessTree, you have:
            private long cpuPercentage = 0;
            ...
            @Override
            public float getCpuUsagePercent() {
              return this.cpuPercentage;
            }
          

          Should it just return 0 (like you had originally) instead of declaring a member variable that is never changed?

          In the test you sleep for 200ms for the next cycle. Is there a way to wait for a concrete event instead of sleeping and hoping? I'm worried about adding another flakey test to the list.

          Show
          templedf Daniel Templeton added a comment - Thanks, Sunil G . Looks like checkstyle caught something I should have: you're missing javadocs on the MockCPUResourceCalculatorProcessTree class. Since there's another patch coming, I have a few more nits to pick that I just noticed: In MockResourceCalculatorProcessTree , you have: private long cpuPercentage = 0; ... @Override public float getCpuUsagePercent() { return this .cpuPercentage; } Should it just return 0 (like you had originally) instead of declaring a member variable that is never changed? In the test you sleep for 200ms for the next cycle. Is there a way to wait for a concrete event instead of sleeping and hoping? I'm worried about adding another flakey test to the list.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 26s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 10s Maven dependency ordering for branch
          +1 mvninstall 5m 59s trunk passed
          +1 compile 1m 55s trunk passed
          +1 checkstyle 0m 34s trunk passed
          +1 mvnsite 0m 53s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 31s trunk passed
          +1 javadoc 0m 42s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 44s the patch passed
          +1 compile 1m 52s the patch passed
          +1 javac 1m 52s the patch passed
          -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 83 unchanged - 2 fixed = 85 total (was 85)
          +1 mvnsite 0m 49s the patch passed
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 44s the patch passed
          +1 javadoc 0m 39s the patch passed
          +1 unit 2m 5s hadoop-yarn-common in the patch passed.
          +1 unit 10m 52s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          33m 14s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807884/0007-YARN-4308.patch
          JIRA Issue YARN-4308
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c75862020153 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 97e2449
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11833/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11833/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11833/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 26s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 5m 59s trunk passed +1 compile 1m 55s trunk passed +1 checkstyle 0m 34s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 31s trunk passed +1 javadoc 0m 42s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 44s the patch passed +1 compile 1m 52s the patch passed +1 javac 1m 52s the patch passed -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 83 unchanged - 2 fixed = 85 total (was 85) +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 44s the patch passed +1 javadoc 0m 39s the patch passed +1 unit 2m 5s hadoop-yarn-common in the patch passed. +1 unit 10m 52s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 33m 14s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807884/0007-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c75862020153 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 97e2449 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11833/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11833/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11833/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Daniel Templeton, Thank you very much. Extremely sorry for that debug log, my bad. I added to debug some tests.

          Attaching new patch by removing unwanted logs, and also fixed checkstyle/javac warnings. Kindly help to check the same.

          Show
          sunilg Sunil G added a comment - Daniel Templeton , Thank you very much. Extremely sorry for that debug log, my bad. I added to debug some tests. Attaching new patch by removing unwanted logs, and also fixed checkstyle/javac warnings. Kindly help to check the same.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks for adding the test, Sunil G. Looks generally good. You probably want to remove this, though:

          +            LOG.info("For me Sunil: " + cpuUsagePercentPerCore);
          
          Show
          templedf Daniel Templeton added a comment - Thanks for adding the test, Sunil G . Looks generally good. You probably want to remove this, though: + LOG.info( "For me Sunil: " + cpuUsagePercentPerCore);
          Hide
          sunilg Sunil G added a comment -

          I will fix the warning and checkstyle issue after a round of review with a new patch, so I can address if any comments are there. Thank you. cc/NGarla_Unused Daniel Templeton.

          Show
          sunilg Sunil G added a comment - I will fix the warning and checkstyle issue after a round of review with a new patch, so I can address if any comments are there. Thank you. cc/ NGarla_Unused Daniel Templeton .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 7s Maven dependency ordering for branch
          +1 mvninstall 6m 29s trunk passed
          +1 compile 2m 2s trunk passed
          +1 checkstyle 0m 41s trunk passed
          +1 mvnsite 0m 55s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 38s trunk passed
          +1 javadoc 0m 45s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 55s the patch passed
          +1 compile 2m 1s the patch passed
          -1 javac 2m 1s hadoop-yarn-project_hadoop-yarn generated 1 new + 33 unchanged - 0 fixed = 34 total (was 33)
          -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 4 new + 84 unchanged - 1 fixed = 88 total (was 85)
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 56s the patch passed
          +1 javadoc 0m 41s the patch passed
          +1 unit 2m 6s hadoop-yarn-common in the patch passed.
          +1 unit 10m 55s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          34m 47s



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 7s Maven dependency ordering for branch +1 mvninstall 6m 29s trunk passed +1 compile 2m 2s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 38s trunk passed +1 javadoc 0m 45s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 55s the patch passed +1 compile 2m 1s the patch passed -1 javac 2m 1s hadoop-yarn-project_hadoop-yarn generated 1 new + 33 unchanged - 0 fixed = 34 total (was 33) -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 4 new + 84 unchanged - 1 fixed = 88 total (was 85) +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 56s the patch passed +1 javadoc 0m 41s the patch passed +1 unit 2m 6s hadoop-yarn-common in the patch passed. +1 unit 10m 55s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 34m 47s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807716/0006-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b56e59058633 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / aadb77e Default Java 1.8.0_91 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-YARN-Build/11816/artifact/patchprocess/diff-compile-javac-hadoop-yarn-project_hadoop-yarn.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11816/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11816/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11816/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          I guess I missed one file in earlier patch. Reattaching an updated patch.

          Show
          sunilg Sunil G added a comment - I guess I missed one file in earlier patch. Reattaching an updated patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 9s Maven dependency ordering for branch
          +1 mvninstall 6m 31s trunk passed
          +1 compile 2m 3s trunk passed
          +1 checkstyle 0m 43s trunk passed
          +1 mvnsite 0m 55s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 38s trunk passed
          +1 javadoc 0m 45s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          -1 mvninstall 0m 22s hadoop-yarn-server-nodemanager in the patch failed.
          -1 compile 1m 1s hadoop-yarn in the patch failed.
          -1 javac 1m 1s hadoop-yarn in the patch failed.
          -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 84 unchanged - 1 fixed = 86 total (was 85)
          -1 mvnsite 0m 21s hadoop-yarn-server-nodemanager in the patch failed.
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 0m 10s hadoop-yarn-server-nodemanager in the patch failed.
          +1 javadoc 0m 48s the patch passed
          +1 unit 2m 6s hadoop-yarn-common in the patch passed.
          -1 unit 0m 23s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          22m 35s



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 31s trunk passed +1 compile 2m 3s trunk passed +1 checkstyle 0m 43s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 38s trunk passed +1 javadoc 0m 45s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch -1 mvninstall 0m 22s hadoop-yarn-server-nodemanager in the patch failed. -1 compile 1m 1s hadoop-yarn in the patch failed. -1 javac 1m 1s hadoop-yarn in the patch failed. -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 84 unchanged - 1 fixed = 86 total (was 85) -1 mvnsite 0m 21s hadoop-yarn-server-nodemanager in the patch failed. +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 10s hadoop-yarn-server-nodemanager in the patch failed. +1 javadoc 0m 48s the patch passed +1 unit 2m 6s hadoop-yarn-common in the patch passed. -1 unit 0m 23s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 22m 35s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807707/0005-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e38d3ec69b25 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / aadb77e Default Java 1.8.0_91 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn.txt checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11813/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11813/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11813/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Hi Daniel Templeton NGarla_Unused

          I have added a test case to check whether UNAVAILABLE return value for CPU percentage is handled properly in ContainerMonitorImpl. Pls help to check the same.

          Show
          sunilg Sunil G added a comment - Hi Daniel Templeton NGarla_Unused I have added a test case to check whether UNAVAILABLE return value for CPU percentage is handled properly in ContainerMonitorImpl. Pls help to check the same.
          Hide
          templedf Daniel Templeton added a comment -

          Sunil G, I don't think just fixing the existing tests to not break with the change is enough. I would prefer it if we could test both what happens when the cpuUsagePercent is 0 and when it's -1. Right now we're not testing that the -1's are skipped correctly.

          Show
          templedf Daniel Templeton added a comment - Sunil G , I don't think just fixing the existing tests to not break with the change is enough. I would prefer it if we could test both what happens when the cpuUsagePercent is 0 and when it's -1. Right now we're not testing that the -1's are skipped correctly.
          Hide
          sunilg Sunil G added a comment -

          Hi Daniel Templeton

          NGarla_Unused also had a same concern. And earlier i tried to explain with this comment.. With MockResourceCalculatorProcessTree#getCpuUsagePercent, I am overriding the default CPU usage from -1 to 0. If I am not overloading, then this will always be skipped in ContainreMonitor. Actual;y existing tests were covering this scenario already. Do you see whether we need one more test for same?. Pls help to share your thoughts.

          Show
          sunilg Sunil G added a comment - Hi Daniel Templeton NGarla_Unused also had a same concern. And earlier i tried to explain with this comment. . With MockResourceCalculatorProcessTree#getCpuUsagePercent , I am overriding the default CPU usage from -1 to 0. If I am not overloading, then this will always be skipped in ContainreMonitor . Actual;y existing tests were covering this scenario already. Do you see whether we need one more test for same?. Pls help to share your thoughts.
          Hide
          templedf Daniel Templeton added a comment -

          It doesn't look like the patch includes a test to confirm that negative values are properly ignored.

          Also, the earlier conversation was about logging at DEBUG level. Looks like we're at INFO level now. I prefer INFO, but I want to make sure that was intended.

          Show
          templedf Daniel Templeton added a comment - It doesn't look like the patch includes a test to confirm that negative values are properly ignored. Also, the earlier conversation was about logging at DEBUG level. Looks like we're at INFO level now. I prefer INFO, but I want to make sure that was intended.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Sunil G patch LGTM, if no more comments will commit this patch.
          cc Daniel Templeton

          Show
          Naganarasimha Naganarasimha G R added a comment - Sunil G patch LGTM, if no more comments will commit this patch. cc Daniel Templeton
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 6m 33s trunk passed
          +1 compile 1m 55s trunk passed
          +1 checkstyle 0m 36s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 32s trunk passed
          +1 javadoc 0m 45s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 0m 45s the patch passed
          +1 compile 1m 53s the patch passed
          +1 javac 1m 53s the patch passed
          -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83)
          +1 mvnsite 0m 50s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 43s the patch passed
          +1 javadoc 0m 44s the patch passed
          +1 unit 2m 4s hadoop-yarn-common in the patch passed.
          +1 unit 11m 25s hadoop-yarn-server-nodemanager in the patch passed.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          34m 28s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805256/0004-YARN-4308.patch
          JIRA Issue YARN-4308
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c2bb8ec14a74 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 757050f
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11592/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11592/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/11592/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 6m 33s trunk passed +1 compile 1m 55s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 32s trunk passed +1 javadoc 0m 45s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 0m 45s the patch passed +1 compile 1m 53s the patch passed +1 javac 1m 53s the patch passed -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 43s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 2m 4s hadoop-yarn-common in the patch passed. +1 unit 11m 25s hadoop-yarn-server-nodemanager in the patch passed. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 34m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805256/0004-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c2bb8ec14a74 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 757050f Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11592/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11592/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11592/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          sunilg Sunil G added a comment -

          Thanks NGarla_Unused and Daniel Templeton

          Updating a new patch addressing the comments. Now using java doc comment in ResourceCalculatorProcessTree and its child classes. (not adding comments in test classes). Pls help to check the same and kindly let me know if any issues.

          Show
          sunilg Sunil G added a comment - Thanks NGarla_Unused and Daniel Templeton Updating a new patch addressing the comments. Now using java doc comment in ResourceCalculatorProcessTree and its child classes. (not adding comments in test classes). Pls help to check the same and kindly let me know if any issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 25s Maven dependency ordering for branch
          +1 mvninstall 6m 23s trunk passed
          +1 compile 2m 2s trunk passed
          +1 checkstyle 0m 37s trunk passed
          +1 mvnsite 0m 53s trunk passed
          +1 mvneclipse 0m 21s trunk passed
          +1 findbugs 1m 31s trunk passed
          +1 javadoc 0m 46s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 46s the patch passed
          +1 compile 1m 58s the patch passed
          +1 javac 1m 57s the patch passed
          -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83)
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 19s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 47s the patch passed
          +1 javadoc 0m 44s the patch passed
          +1 unit 2m 11s hadoop-yarn-common in the patch passed.
          -1 unit 11m 48s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          35m 23s



          Reason Tests
          Failed junit tests hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 25s Maven dependency ordering for branch +1 mvninstall 6m 23s trunk passed +1 compile 2m 2s trunk passed +1 checkstyle 0m 37s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 21s trunk passed +1 findbugs 1m 31s trunk passed +1 javadoc 0m 46s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 46s the patch passed +1 compile 1m 58s the patch passed +1 javac 1m 57s the patch passed -1 checkstyle 0m 34s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83) +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 19s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 47s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 2m 11s hadoop-yarn-common in the patch passed. -1 unit 11m 48s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 35m 23s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804727/0003-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 255f58d547bd 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1597630 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11544/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11544/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11544/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11544/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11544/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 35s Maven dependency ordering for branch
          +1 mvninstall 8m 46s trunk passed
          +1 compile 2m 46s trunk passed
          +1 checkstyle 0m 44s trunk passed
          +1 mvnsite 1m 11s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 2m 6s trunk passed
          +1 javadoc 1m 0s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 49s the patch passed
          +1 compile 2m 6s the patch passed
          +1 javac 2m 6s the patch passed
          -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83)
          +1 mvnsite 0m 57s the patch passed
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 53s the patch passed
          +1 javadoc 0m 46s the patch passed
          +1 unit 2m 11s hadoop-yarn-common in the patch passed.
          -1 unit 11m 24s hadoop-yarn-server-nodemanager in the patch failed.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          40m 24s



          Reason Tests
          Failed junit tests hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 35s Maven dependency ordering for branch +1 mvninstall 8m 46s trunk passed +1 compile 2m 46s trunk passed +1 checkstyle 0m 44s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 2m 6s trunk passed +1 javadoc 1m 0s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 49s the patch passed +1 compile 2m 6s the patch passed +1 javac 2m 6s the patch passed -1 checkstyle 0m 33s hadoop-yarn-project/hadoop-yarn: patch generated 1 new + 82 unchanged - 1 fixed = 83 total (was 83) +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 53s the patch passed +1 javadoc 0m 46s the patch passed +1 unit 2m 11s hadoop-yarn-common in the patch passed. -1 unit 11m 24s hadoop-yarn-server-nodemanager in the patch failed. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 40m 24s Reason Tests Failed junit tests hadoop.yarn.server.nodemanager.containermanager.monitor.TestContainerMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804727/0003-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a2a27e06128d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 010e6ac Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11534/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/11534/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/11534/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11534/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/11534/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          I'd go one step further and ask that the comments be a javadoc comments and that the javadoc comments be complete javadoc for the methods, i.e. add javadoc comments for the methods that are missing it.

          Show
          templedf Daniel Templeton added a comment - I'd go one step further and ask that the comments be a javadoc comments and that the javadoc comments be complete javadoc for the methods, i.e. add javadoc comments for the methods that are missing it.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the patch Sunil G, Patch seems to be simple and straight fwd, just on nit , can the comment be a javadoc comment if not the link is of no use .

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the patch Sunil G , Patch seems to be simple and straight fwd, just on nit , can the comment be a javadoc comment if not the link is of no use .
          Hide
          sunilg Sunil G added a comment -

          Updating new patch as per the discussion. Kindly help to check the same.

          Show
          sunilg Sunil G added a comment - Updating new patch as per the discussion. Kindly help to check the same.
          Hide
          sunilg Sunil G added a comment -

          Yes. Thanks Karthik Kambatla Daniel Templeton and NGarla_Unused.
          I will update a patch by changing log to info and will update java docs. Thanks for pooling in the thoughts.

          Show
          sunilg Sunil G added a comment - Yes. Thanks Karthik Kambatla Daniel Templeton and NGarla_Unused . I will update a patch by changing log to info and will update java docs. Thanks for pooling in the thoughts.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Sunil G, As most of us agree that metrics is not actually much useful, So can you update the patch with updating javadocs and approp comment, and also i am fine with info logging as its once per container so not much of an impact ?

          Show
          Naganarasimha Naganarasimha G R added a comment - Sunil G , As most of us agree that metrics is not actually much useful, So can you update the patch with updating javadocs and approp comment, and also i am fine with info logging as its once per container so not much of an impact ?
          Hide
          kasha Karthik Kambatla added a comment -

          Given we expect to see -1 exactly once, do we really need metrics for this? Just logging it at INFO level seems innocuous, no?

          Show
          kasha Karthik Kambatla added a comment - Given we expect to see -1 exactly once, do we really need metrics for this? Just logging it at INFO level seems innocuous, no?
          Hide
          templedf Daniel Templeton added a comment -

          I don't have a problem with also adding the metric, but it needs to be well and clearly documented so that it doesn't become yet another knob/dial that people patently ignore for lack of knowing what it does.

          Show
          templedf Daniel Templeton added a comment - I don't have a problem with also adding the metric, but it needs to be well and clearly documented so that it doesn't become yet another knob/dial that people patently ignore for lack of knowing what it does.
          Hide
          sunilg Sunil G added a comment -

          Thanks for weighing in the thoughts Daniel Templeton and NGarla_Unused

          I'm fine with just making sure that anyone hunting reasons for missing reports will trip over a pointer to the likely cause

          Metric can only give a suggestion and a possible error cause. So once this symptom is hit, it produces some indications (runtime metric). And it can help the admin to enable debug log for this class alone (as NGarla_Unused mentioned). I am not very much sure about internally changing log level. Seems like it can be little bit of too much complex snippets to handle a defined scenario (comments/javadoc can define the protocol or standard). So mostly agreeing to NGarla_Unused's view and still thinking that we can have a metric.

          Show
          sunilg Sunil G added a comment - Thanks for weighing in the thoughts Daniel Templeton and NGarla_Unused I'm fine with just making sure that anyone hunting reasons for missing reports will trip over a pointer to the likely cause Metric can only give a suggestion and a possible error cause. So once this symptom is hit, it produces some indications (runtime metric). And it can help the admin to enable debug log for this class alone (as NGarla_Unused mentioned). I am not very much sure about internally changing log level. Seems like it can be little bit of too much complex snippets to handle a defined scenario (comments/javadoc can define the protocol or standard). So mostly agreeing to NGarla_Unused 's view and still thinking that we can have a metric.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for suggesstions and discussions Sunil G & Daniel Templeton,
          IMO issue is not just about missing usage reports, Memory Isolation (controlling overusage) itself gets blocked. So its important to capture in debug logs and javadoc and comment.
          IMHO metric for this would be over engineering for a corner case. so i am ok with documenting properly and commenting this corner case for easy identification. Thoughts?
          In btw log level for a particular class can be dynamically modified so not required to bounce the NM.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for suggesstions and discussions Sunil G & Daniel Templeton , IMO issue is not just about missing usage reports, Memory Isolation (controlling overusage) itself gets blocked. So its important to capture in debug logs and javadoc and comment. IMHO metric for this would be over engineering for a corner case. so i am ok with documenting properly and commenting this corner case for easy identification. Thoughts? In btw log level for a particular class can be dynamically modified so not required to bounce the NM.
          Hide
          templedf Daniel Templeton added a comment -

          If the admin has to enable debug and bounce the daemon, the situation that caused the issue may not survive the bounce. In that case, I don't think adding a metric will be enough of an improvement over javadocs and comments to be worth it. I'm fine with just making sure that anyone hunting reasons for missing reports will trip over a pointer to the likely cause.

          Show
          templedf Daniel Templeton added a comment - If the admin has to enable debug and bounce the daemon, the situation that caused the issue may not survive the bounce. In that case, I don't think adding a metric will be enough of an improvement over javadocs and comments to be worth it. I'm fine with just making sure that anyone hunting reasons for missing reports will trip over a pointer to the likely cause.
          Hide
          sunilg Sunil G added a comment -

          Daniel Templeton, I think admin has to come in and enable debug log. Metric will give only an indication. So may be it's better to enable debug manually for detailed investigation. Thoughts?

          Show
          sunilg Sunil G added a comment - Daniel Templeton , I think admin has to come in and enable debug log. Metric will give only an indication. So may be it's better to enable debug manually for detailed investigation. Thoughts?
          Hide
          templedf Daniel Templeton added a comment -

          Interesting idea. When you say, "enable debug log," do you mean the admin can do that or that you'd do it automatically in the code?

          Show
          templedf Daniel Templeton added a comment - Interesting idea. When you say, "enable debug log," do you mean the admin can do that or that you'd do it automatically in the code?
          Hide
          sunilg Sunil G added a comment -

          I synced up with Varun Vasudev offline.
          Few thoughts. We could add a metric which will increment a counter whenever CPU reports -1 (and we will skip these readings as done in the patch) . And this metric could be exposed from NM end. If this metric value is coming more than 1, then we can enable debug log to check for the reasons.
          Daniel Templeton Karthik Kambatla NGarla_Unused Thoughts?

          Show
          sunilg Sunil G added a comment - I synced up with Varun Vasudev offline. Few thoughts. We could add a metric which will increment a counter whenever CPU reports -1 (and we will skip these readings as done in the patch) . And this metric could be exposed from NM end. If this metric value is coming more than 1, then we can enable debug log to check for the reasons. Daniel Templeton Karthik Kambatla NGarla_Unused Thoughts?
          Hide
          sunilg Sunil G added a comment -

          Sure. I feel we can weigh in opinion from Karthik Kambatla and NGarla_Unused too. I am fine in either way (documenting and commenting Or restricted warning logging) so it will be good if some more thoughts comes in so that best solution can go in.

          Show
          sunilg Sunil G added a comment - Sure. I feel we can weigh in opinion from Karthik Kambatla and NGarla_Unused too. I am fine in either way (documenting and commenting Or restricted warning logging) so it will be good if some more thoughts comes in so that best solution can go in.
          Hide
          templedf Daniel Templeton added a comment -

          If you're not going to let the user know about persistent missed reports, then leave a wide trail of breadcrumbs for the person who has to debug it. Putting it in the JavaDoc is a good first step. Maybe also drop a comment into the code that calls the method.

          Show
          templedf Daniel Templeton added a comment - If you're not going to let the user know about persistent missed reports, then leave a wide trail of breadcrumbs for the person who has to debug it. Putting it in the JavaDoc is a good first step. Maybe also drop a comment into the code that calls the method.
          Hide
          sunilg Sunil G added a comment -

          Yes. debug log is already present, my bad.

          Even if right now the only time a negative value comes back is on the first report, that doesn't mean it won't change later.

          I agree your thought. CpuTimeTracker doesnt have a protocol/standard defined when to return -1 or 0 or other values. So there are chances that this can be changed too in future. But I am thinking in covering this proposed INFO log code from test case point if view also. Because after skipping n times, we have to log one warning and this cycle has to continue. So this code snippet also to be covered via a test case. Is it fine if we make a note in CpuTimeTracker for its behavior or its expected return code as java doc?. I am fine in either way, but was thinking about the real usecase for now.

          Show
          sunilg Sunil G added a comment - Yes. debug log is already present, my bad. Even if right now the only time a negative value comes back is on the first report, that doesn't mean it won't change later. I agree your thought. CpuTimeTracker doesnt have a protocol/standard defined when to return -1 or 0 or other values. So there are chances that this can be changed too in future. But I am thinking in covering this proposed INFO log code from test case point if view also. Because after skipping n times, we have to log one warning and this cycle has to continue. So this code snippet also to be covered via a test case. Is it fine if we make a note in CpuTimeTracker for its behavior or its expected return code as java doc?. I am fine in either way, but was thinking about the real usecase for now.
          Hide
          templedf Daniel Templeton added a comment -

          There's already a debug log on a miss in the patch.

          Even if right now the only time a negative value comes back is on the first report, that doesn't mean it won't change later. My spider sense says that the chance of the reports going away permanently with no sign as to why is bad. We're talking about futures, though, so I'm willing to accept your assertion that this change can't possible create a customer support case, but I reserve the right to an I-told-you-so later if it does.

          Show
          templedf Daniel Templeton added a comment - There's already a debug log on a miss in the patch. Even if right now the only time a negative value comes back is on the first report, that doesn't mean it won't change later. My spider sense says that the chance of the reports going away permanently with no sign as to why is bad. We're talking about futures, though, so I'm willing to accept your assertion that this change can't possible create a customer support case, but I reserve the right to an I-told-you-so later if it does.
          Hide
          sunilg Sunil G added a comment -

          Thanks Daniel Templeton for sharing the thoughts.
          I have checked the possibilities of getting -ve values from CpuTimeTracker . As I see it, we can get negative only first time and I was not seeing other cases. In that case, considering the skipping happen only once, do we need a INFO log there ? I think I can add a debug log if that code is hit. But I am not very sure whether we need log after "n" hits, because it may hit only first time. Could you pls correct me If i missed somthing .

          Show
          sunilg Sunil G added a comment - Thanks Daniel Templeton for sharing the thoughts. I have checked the possibilities of getting -ve values from CpuTimeTracker . As I see it, we can get negative only first time and I was not seeing other cases. In that case, considering the skipping happen only once, do we need a INFO log there ? I think I can add a debug log if that code is hit. But I am not very sure whether we need log after "n" hits, because it may hit only first time. Could you pls correct me If i missed somthing .
          Hide
          templedf Daniel Templeton added a comment -

          I think it would make sense to test that the negative values are properly ignored.

          I saw that Karthik Kambatla said the pathological case of always getting a negative value should not occur, but I'm a still little concerned about that case. If it happens, there will be no externally visible signs as to why the reports are being skipped. Taking the daemon down to turn on debugging may well change the state, leaving a confused end user. Is there a way that we can drop an obvious flag in the logs if the issue persists? Like maybe if we skip n reports in a row, log a warning?

          Show
          templedf Daniel Templeton added a comment - I think it would make sense to test that the negative values are properly ignored. I saw that Karthik Kambatla said the pathological case of always getting a negative value should not occur, but I'm a still little concerned about that case. If it happens, there will be no externally visible signs as to why the reports are being skipped. Taking the daemon down to turn on debugging may well change the state, leaving a confused end user. Is there a way that we can drop an obvious flag in the logs if the issue persists? Like maybe if we skip n reports in a row, log a warning?
          Hide
          sunilg Sunil G added a comment -

          Hi Naganarasimha G R
          Thank you for triggering jenkins. I have made a change in one test case to cover this scenario. Earlier they were expecting -1 there. Do you feel we need to have a case for -1 value for CPU and ensure that we skip it? Because we were using mock framework and if I do not have the suggested change in the Mock class, then it will skip all readings.

          Show
          sunilg Sunil G added a comment - Hi Naganarasimha G R Thank you for triggering jenkins. I have made a change in one test case to cover this scenario. Earlier they were expecting -1 there. Do you feel we need to have a case for -1 value for CPU and ensure that we skip it? Because we were using mock framework and if I do not have the suggested change in the Mock class, then it will skip all readings.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 56s trunk passed
          +1 compile 0m 27s trunk passed with JDK v1.8.0_92
          +1 compile 0m 26s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 51s trunk passed
          +1 javadoc 0m 17s trunk passed with JDK v1.8.0_92
          +1 javadoc 0m 22s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 24s the patch passed
          +1 compile 0m 18s the patch passed with JDK v1.8.0_92
          +1 javac 0m 18s the patch passed
          +1 compile 0m 23s the patch passed with JDK v1.7.0_95
          +1 javac 0m 23s the patch passed
          -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: patch generated 1 new + 22 unchanged - 1 fixed = 23 total (was 23)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 59s the patch passed
          +1 javadoc 0m 16s the patch passed with JDK v1.8.0_92
          +1 javadoc 0m 20s the patch passed with JDK v1.7.0_95
          +1 unit 11m 1s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_92.
          +1 unit 11m 37s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          37m 54s



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

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 56s trunk passed +1 compile 0m 27s trunk passed with JDK v1.8.0_92 +1 compile 0m 26s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 51s trunk passed +1 javadoc 0m 17s trunk passed with JDK v1.8.0_92 +1 javadoc 0m 22s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 24s the patch passed +1 compile 0m 18s the patch passed with JDK v1.8.0_92 +1 javac 0m 18s the patch passed +1 compile 0m 23s the patch passed with JDK v1.7.0_95 +1 javac 0m 23s the patch passed -1 checkstyle 0m 13s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: patch generated 1 new + 22 unchanged - 1 fixed = 23 total (was 23) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 59s the patch passed +1 javadoc 0m 16s the patch passed with JDK v1.8.0_92 +1 javadoc 0m 20s the patch passed with JDK v1.7.0_95 +1 unit 11m 1s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.8.0_92. +1 unit 11m 37s hadoop-yarn-server-nodemanager in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 37m 54s Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12795882/0002-YARN-4308.patch JIRA Issue YARN-4308 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d183c125478f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a5fed8b Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_92 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/11219/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/11219/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/11219/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Have triggered the jenkins, also Sunil G are you planning to write any test case?

          Show
          Naganarasimha Naganarasimha G R added a comment - Have triggered the jenkins, also Sunil G are you planning to write any test case?
          Hide
          kasha Karthik Kambatla added a comment -

          I had recently reviewed ResourceCalculatorProcessTree patches. Don't remember any cases where it could be negative in other cases.

          Show
          kasha Karthik Kambatla added a comment - I had recently reviewed ResourceCalculatorProcessTree patches. Don't remember any cases where it could be negative in other cases.
          Hide
          Naganarasimha Naganarasimha G R added a comment -

          Thanks for the patch Karthik Kambatla & Sunil G, it LGTM.
          But just one query is there any possibility that cpuUsagePercentPerCore is reported as -1 other than the initial run (like if the stats are not available in particular OS or any other reason) ? if so then there is possibility that Memory monitoring will never happen. From my side did a walk through on the ResourceCalculatorProcessTree and the related code, based on the code did not find any such flows but it would be good if some one involved during the earlier code of ResourceCalculatorProcessTree reviews and confirms.

          Show
          Naganarasimha Naganarasimha G R added a comment - Thanks for the patch Karthik Kambatla & Sunil G , it LGTM. But just one query is there any possibility that cpuUsagePercentPerCore is reported as -1 other than the initial run (like if the stats are not available in particular OS or any other reason) ? if so then there is possibility that Memory monitoring will never happen. From my side did a walk through on the ResourceCalculatorProcessTree and the related code, based on the code did not find any such flows but it would be good if some one involved during the earlier code of ResourceCalculatorProcessTree reviews and confirms.
          Hide
          sunilg Sunil G added a comment -

          I also tested this by adding some more debug logs. I am not seeing any -ve values as I saw earlier. Looks fine for me.

          2016-04-16 22:36:21,127 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 0.0
          2016-04-16 22:36:22,129 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 0.0
          2016-04-16 22:36:23,132 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 4.509
          
          Show
          sunilg Sunil G added a comment - I also tested this by adding some more debug logs. I am not seeing any -ve values as I saw earlier. Looks fine for me. 2016-04-16 22:36:21,127 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 0.0 2016-04-16 22:36:22,129 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 0.0 2016-04-16 22:36:23,132 ERROR org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode: ContainersResource Utilization : CpuTrackerUsagePercent : 4.509
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for updating the patch, Sunil. The patch looks good to me. Will let someone else review/+1.

          Show
          kasha Karthik Kambatla added a comment - Thanks for updating the patch, Sunil. The patch looks good to me. Will let someone else review/+1.
          Hide
          sunilg Sunil G added a comment -

          Thank you Karthik Kambatla.
          Yes, I think your patch can go in w/o any change. I am re-attaching the same here. Only making some test case changes to make sure tests are passing.

          Thanks once again for sharing the thoughts and patch.

          Show
          sunilg Sunil G added a comment - Thank you Karthik Kambatla . Yes, I think your patch can go in w/o any change. I am re-attaching the same here. Only making some test case changes to make sure tests are passing. Thanks once again for sharing the thoughts and patch.
          Hide
          kasha Karthik Kambatla added a comment -

          I would prefer skipping reporting a container's utilization if CPU is unavailable, since we would likely get it the next turn. I posted a patch on YARN-4885. Can we go along those lines? Sunil G - if that is acceptable, can you update the patch here accordingly?

          Show
          kasha Karthik Kambatla added a comment - I would prefer skipping reporting a container's utilization if CPU is unavailable, since we would likely get it the next turn. I posted a patch on YARN-4885 . Can we go along those lines? Sunil G - if that is acceptable, can you update the patch here accordingly?
          Hide
          sunilg Sunil G added a comment -

          Hi Junping Du , Varun Vasudev
          Recently we had few discussion in one of the ATS metrics jira YARN-4172 regarding -1 handling for CPU usage.

          Agreeing that we need to send -1 when there are no reading available, I would like to point 2 cases here;
          1. When cpu sample is taken for first time, current code snippet in CpuTimeTracker is sending -1. In such cases, its debatable that whether we can send -1 for this case or not. May be we could start with 0 or even we can wait for a cycle to report back.
          2. If CpuTimeTracker#getCpuTrackerUsagePercent returns -1, we can send the reading as it is back to caller. There is no need to operate on same. ResourceCalculatorProcessTree.UNAVAILABLE can be returned as CPU usage.

          If thoughts are same, I can update a new patch.

          Show
          sunilg Sunil G added a comment - Hi Junping Du , Varun Vasudev Recently we had few discussion in one of the ATS metrics jira YARN-4172 regarding -1 handling for CPU usage. Agreeing that we need to send -1 when there are no reading available, I would like to point 2 cases here; 1. When cpu sample is taken for first time , current code snippet in CpuTimeTracker is sending -1. In such cases, its debatable that whether we can send -1 for this case or not. May be we could start with 0 or even we can wait for a cycle to report back. 2. If CpuTimeTracker#getCpuTrackerUsagePercent returns -1, we can send the reading as it is back to caller. There is no need to operate on same. ResourceCalculatorProcessTree.UNAVAILABLE can be returned as CPU usage. If thoughts are same, I can update a new patch.
          Hide
          sunilg Sunil G added a comment -
          Show
          sunilg Sunil G added a comment - cc/ Varun Vasudev
          Hide
          sunilg Sunil G added a comment -

          Hi Junping Du
          Could you please help to see the above case. I can also handle this case from the user end. We could just reset res-usage for CPU as n/a or 0 before sending any resource usage to client/UI. But still for first time, I feel that we can return 0 rather that negative value. In other cases, sending a -ve value is fine (provided we need not do any more modification or calculation based on that negative value).
          For eg: calculation of milliVcoresUsed in ContainerMonitorImpl

          Show
          sunilg Sunil G added a comment - Hi Junping Du Could you please help to see the above case. I can also handle this case from the user end. We could just reset res-usage for CPU as n/a or 0 before sending any resource usage to client/UI. But still for first time, I feel that we can return 0 rather that negative value. In other cases, sending a -ve value is fine (provided we need not do any more modification or calculation based on that negative value). For eg: calculation of milliVcoresUsed in ContainerMonitorImpl
          Hide
          sunilg Sunil G added a comment -

          Thanks Karthik Kambatla and Junping Du for clarifying the same.
          As part of YARN-4292, we were trying to get the ResourceUtilization from Nodes through REST api. And I came across getting -ve CPU usage for Containers ResourceUtilization at the start time when containers were just allocated. I thought it may confuse user, hence raised the same. Showing a snippet from the REST o/p (proposed sample o/p)

          nodePhysicalMemoryMB: 4641
          nodeVirtualMemoryMB: 0
          nodeCPUUsage: 10.576282501220703
          containersPhysicalMemoryMB: 1297
          containersVirtualMemoryMB: 0
          containersCPUUsage: -1.925473
          

          But I understood the discussion happened in YARN-3304, about showing 0 may give an assumption that all metrics works fine. Yes, Its correct.
          But here I think scenario is slightly different. When CpuTimeTracker#getCpuTrackerUsagePercent is called for first time, lastSampleTime will not be available. Hence the method returns -ve values always for first time. and this propagates till RM through heartbeat as -ve value and some more calculations were also happened during this for the metric such as the calculation of milliVcoresUsed in ContainerMonitorImpl. I think some specific handling can be done for this as this will happen always first time compared to a genuine non-availability of metric. How do you feel?

           public float getCpuTrackerUsagePercent() {
              if (lastSampleTime == UNAVAILABLE ||
                  lastSampleTime > sampleTime) {
                // lastSampleTime > sampleTime may happen when the system time is changed
                lastSampleTime = sampleTime;
                lastCumulativeCpuTime = cumulativeCpuTime;
                return cpuUsage;
              }
          ...
          
          Show
          sunilg Sunil G added a comment - Thanks Karthik Kambatla and Junping Du for clarifying the same. As part of YARN-4292 , we were trying to get the ResourceUtilization from Nodes through REST api. And I came across getting -ve CPU usage for Containers ResourceUtilization at the start time when containers were just allocated. I thought it may confuse user, hence raised the same. Showing a snippet from the REST o/p (proposed sample o/p) nodePhysicalMemoryMB: 4641 nodeVirtualMemoryMB: 0 nodeCPUUsage: 10.576282501220703 containersPhysicalMemoryMB: 1297 containersVirtualMemoryMB: 0 containersCPUUsage: -1.925473 But I understood the discussion happened in YARN-3304 , about showing 0 may give an assumption that all metrics works fine. Yes, Its correct. But here I think scenario is slightly different. When CpuTimeTracker#getCpuTrackerUsagePercent is called for first time, lastSampleTime will not be available. Hence the method returns -ve values always for first time. and this propagates till RM through heartbeat as -ve value and some more calculations were also happened during this for the metric such as the calculation of milliVcoresUsed in ContainerMonitorImpl . I think some specific handling can be done for this as this will happen always first time compared to a genuine non-availability of metric. How do you feel? public float getCpuTrackerUsagePercent() { if (lastSampleTime == UNAVAILABLE || lastSampleTime > sampleTime) { // lastSampleTime > sampleTime may happen when the system time is changed lastSampleTime = sampleTime; lastCumulativeCpuTime = cumulativeCpuTime; return cpuUsage; } ...
          Hide
          djp Junping Du added a comment -

          I think the long discussion was happening in YARN-3304. As most people agreed, "-1" is used to represent resource is unavailable for tracking to differentiate with zero resource.

          Show
          djp Junping Du added a comment - I think the long discussion was happening in YARN-3304 . As most people agreed, "-1" is used to represent resource is unavailable for tracking to differentiate with zero resource.
          Hide
          kasha Karthik Kambatla added a comment -

          Don't recall all the details, but that was intentional.

          Anubhav Dhoot/Junping Du - there was a lot of back and forth, do you guys remember the details?

          Show
          kasha Karthik Kambatla added a comment - Don't recall all the details, but that was intentional. Anubhav Dhoot / Junping Du - there was a lot of back and forth, do you guys remember the details?
          Hide
          sunilg Sunil G added a comment -

          Attaching a patch to handle this corner case.

          Show
          sunilg Sunil G added a comment - Attaching a patch to handle this corner case.
          Hide
          sunilg Sunil G added a comment -

          In CpuTimeTrackerCpuTimeTracker, Below code snippet can send UNAVAILABLE for first time as lastSampleTime will be UNAVAILABLE for first time calculation.

          This caused for a -ve return value and caused NM to send a -ve CPU usages in its heartbeats for a brief time

            public float getCpuTrackerUsagePercent() {
              if (lastSampleTime == UNAVAILABLE ||
                  lastSampleTime > sampleTime) {
                // lastSampleTime > sampleTime may happen when the system time is changed
                lastSampleTime = sampleTime;
                lastCumulativeCpuTime = cumulativeCpuTime;
                return cpuUsage;
              }
          ....
          ....
          
          Show
          sunilg Sunil G added a comment - In CpuTimeTrackerCpuTimeTracker , Below code snippet can send UNAVAILABLE for first time as lastSampleTime will be UNAVAILABLE for first time calculation. This caused for a -ve return value and caused NM to send a -ve CPU usages in its heartbeats for a brief time public float getCpuTrackerUsagePercent() { if (lastSampleTime == UNAVAILABLE || lastSampleTime > sampleTime) { // lastSampleTime > sampleTime may happen when the system time is changed lastSampleTime = sampleTime; lastCumulativeCpuTime = cumulativeCpuTime; return cpuUsage; } .... ....

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development