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

The application attempt's diagnostic message should have a maximum size

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: resourcemanager
    • Labels:
      None

      Description

      We've found through experience that the diagnostic message can grow unbounded. I've seen attempts that have diagnostic messages over 1MB. Since the message is stored in the state store, it's a bad idea to allow the message to grow unbounded. Instead, there should be a property that sets a maximum size on the message.

      I suspect that some of the ZK state store issues we've seen in the past were due to the size of the diagnostic messages and not to the size of the classpath, as is the current prevailing opinion.

      An open question is how best to prune the message once it grows too large. Should we

      1. truncate the tail,
      2. truncate the head,
      3. truncate the middle,
      4. add another property to make the behavior selectable, or
      5. none of the above?
      1. YARN-6125.000.patch
        18 kB
        Andras Piros
      2. YARN-6125.001.patch
        17 kB
        Andras Piros
      3. YARN-6125.002.patch
        20 kB
        Andras Piros
      4. YARN-6125.003.patch
        20 kB
        Andras Piros
      5. YARN-6125.004.patch
        28 kB
        Andras Piros
      6. YARN-6125.005.patch
        17 kB
        Andras Piros
      7. YARN-6125.006.patch
        17 kB
        Andras Piros
      8. YARN-6125.007.patch
        17 kB
        Andras Piros
      9. YARN-6125.008.patch
        17 kB
        Andras Piros
      10. YARN-6125.009.patch
        23 kB
        Andras Piros

        Activity

        Hide
        imstefanlee stefanlee added a comment -
        Show
        imstefanlee stefanlee added a comment - Daniel Templeton thanks
        Hide
        templedf Daniel Templeton added a comment -

        Because BoundedAppender has no dependency on RMAppAttemptImpl. It's an inner class because it's a bespoke class that makes to attempt to account for use cases outside the needs of RMAppAttemptImpl.

        Show
        templedf Daniel Templeton added a comment - Because BoundedAppender has no dependency on RMAppAttemptImpl . It's an inner class because it's a bespoke class that makes to attempt to account for use cases outside the needs of RMAppAttemptImpl .
        Hide
        imstefanlee stefanlee added a comment -

        thanks for this jira, i have a doubt that why class BounderAppender is static?Daniel Templeton Andras Piros

        Show
        imstefanlee stefanlee added a comment - thanks for this jira, i have a doubt that why class BounderAppender is static ? Daniel Templeton Andras Piros
        Hide
        templedf Daniel Templeton added a comment -

        Committed to trunk and branch-2.

        Show
        templedf Daniel Templeton added a comment - Committed to trunk and branch-2.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11275 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11275/)
        YARN-6125. The application attempt's diagnostic message should have a (templedf: rev c7a36e613053ec8b111146004b887c2f13535469)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestBoundedAppender.java
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptImplDiagnostics.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/pom.xml
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11275 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11275/ ) YARN-6125 . The application attempt's diagnostic message should have a (templedf: rev c7a36e613053ec8b111146004b887c2f13535469) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestBoundedAppender.java (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptImplDiagnostics.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/pom.xml (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Andras Piros! LGTM +1. I'll let this sit until tomorrow in case anyone else wants to pile on with comments.

        Show
        templedf Daniel Templeton added a comment - Thanks, Andras Piros ! LGTM +1. I'll let this sit until tomorrow in case anyone else wants to pile on with comments.
        Hide
        andras.piros Andras Piros added a comment -

        The tests TestRMRestart, TestDelegationTokenRenewer, and TestFairSchedulerPreemption are passing locally and seem unrelated.

        Show
        andras.piros Andras Piros added a comment - The tests TestRMRestart , TestDelegationTokenRenewer , and TestFairSchedulerPreemption are passing locally and seem unrelated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 45s 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 1m 25s Maven dependency ordering for branch
        +1 mvninstall 18m 52s trunk passed
        +1 compile 10m 49s trunk passed
        +1 checkstyle 1m 5s trunk passed
        +1 mvnsite 2m 32s trunk passed
        +1 mvneclipse 1m 17s trunk passed
        +1 findbugs 4m 17s trunk passed
        +1 javadoc 1m 58s trunk passed
        0 mvndep 0m 12s Maven dependency ordering for patch
        +1 mvninstall 1m 57s the patch passed
        +1 compile 8m 10s the patch passed
        +1 javac 8m 10s the patch passed
        +1 checkstyle 1m 0s the patch passed
        +1 mvnsite 2m 25s the patch passed
        +1 mvneclipse 1m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 5s The patch has no ill-formed XML file.
        +1 findbugs 4m 39s the patch passed
        +1 javadoc 1m 57s the patch passed
        +1 unit 0m 42s hadoop-yarn-api in the patch passed.
        +1 unit 3m 0s hadoop-yarn-common in the patch passed.
        -1 unit 64m 59s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 1m 14s The patch does not generate ASF License warnings.
        144m 3s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerPreemption
          hadoop.yarn.server.resourcemanager.TestRMRestart
          hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852973/YARN-6125.009.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 5b33bd3662a6 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / a136936
        Default Java 1.8.0_121
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14975/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14975/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14975/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 45s 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 1m 25s Maven dependency ordering for branch +1 mvninstall 18m 52s trunk passed +1 compile 10m 49s trunk passed +1 checkstyle 1m 5s trunk passed +1 mvnsite 2m 32s trunk passed +1 mvneclipse 1m 17s trunk passed +1 findbugs 4m 17s trunk passed +1 javadoc 1m 58s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 1m 57s the patch passed +1 compile 8m 10s the patch passed +1 javac 8m 10s the patch passed +1 checkstyle 1m 0s the patch passed +1 mvnsite 2m 25s the patch passed +1 mvneclipse 1m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 5s The patch has no ill-formed XML file. +1 findbugs 4m 39s the patch passed +1 javadoc 1m 57s the patch passed +1 unit 0m 42s hadoop-yarn-api in the patch passed. +1 unit 3m 0s hadoop-yarn-common in the patch passed. -1 unit 64m 59s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 1m 14s The patch does not generate ASF License warnings. 144m 3s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerPreemption   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852973/YARN-6125.009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 5b33bd3662a6 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a136936 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/14975/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14975/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14975/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Daniel Templeton addressing all the mentioned issues.

        Show
        andras.piros Andras Piros added a comment - Daniel Templeton addressing all the mentioned issues.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Andras Piros. A few more comments.

        • The BoundedAppender.toString() method is missing a javadoc summary
        • While you're making changes, maybe the error message in getDiagnosticsLimitKCOrThrow() should be a constant.
        • It just occurred to me to wonder if there's a sensible way to test this patch in action, by testing the RMAppAttemptImpl...
        Show
        templedf Daniel Templeton added a comment - Thanks, Andras Piros . A few more comments. The BoundedAppender.toString() method is missing a javadoc summary While you're making changes, maybe the error message in getDiagnosticsLimitKCOrThrow() should be a constant. It just occurred to me to wonder if there's a sensible way to test this patch in action, by testing the RMAppAttemptImpl ...
        Hide
        andras.piros Andras Piros added a comment - - edited

        The unit test TestRMRestart#testFinishedAppRemovalAfterRMRestart is unrelated. It passes locally both when run separately and as part of the whole unit test series TestRMRestart three times in a row, when patch 008 has been applied to current trunk.

        Show
        andras.piros Andras Piros added a comment - - edited The unit test TestRMRestart#testFinishedAppRemovalAfterRMRestart is unrelated. It passes locally both when run separately and as part of the whole unit test series TestRMRestart three times in a row, when patch 008 has been applied to current trunk .
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 1m 2s 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 26s Maven dependency ordering for branch
        +1 mvninstall 14m 16s trunk passed
        +1 compile 8m 57s trunk passed
        +1 checkstyle 0m 58s trunk passed
        +1 mvnsite 2m 13s trunk passed
        +1 mvneclipse 1m 0s trunk passed
        +1 findbugs 4m 20s trunk passed
        +1 javadoc 1m 39s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 50s the patch passed
        +1 compile 7m 21s the patch passed
        +1 javac 7m 21s the patch passed
        +1 checkstyle 0m 56s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)
        +1 mvnsite 2m 14s the patch passed
        +1 mvneclipse 0m 59s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 3s The patch has no ill-formed XML file.
        +1 findbugs 4m 29s the patch passed
        +1 javadoc 1m 28s the patch passed
        +1 unit 0m 34s hadoop-yarn-api in the patch passed.
        +1 unit 2m 34s hadoop-yarn-common in the patch passed.
        -1 unit 42m 46s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 38s The patch does not generate ASF License warnings.
        109m 41s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852172/YARN-6125.008.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux ea3e9bbae5c7 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 07a5184
        Default Java 1.8.0_121
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14886/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14886/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14886/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 1m 2s 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 26s Maven dependency ordering for branch +1 mvninstall 14m 16s trunk passed +1 compile 8m 57s trunk passed +1 checkstyle 0m 58s trunk passed +1 mvnsite 2m 13s trunk passed +1 mvneclipse 1m 0s trunk passed +1 findbugs 4m 20s trunk passed +1 javadoc 1m 39s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 50s the patch passed +1 compile 7m 21s the patch passed +1 javac 7m 21s the patch passed +1 checkstyle 0m 56s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318) +1 mvnsite 2m 14s the patch passed +1 mvneclipse 0m 59s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 3s The patch has no ill-formed XML file. +1 findbugs 4m 29s the patch passed +1 javadoc 1m 28s the patch passed +1 unit 0m 34s hadoop-yarn-api in the patch passed. +1 unit 2m 34s hadoop-yarn-common in the patch passed. -1 unit 42m 46s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 38s The patch does not generate ASF License warnings. 109m 41s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852172/YARN-6125.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux ea3e9bbae5c7 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 07a5184 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/14886/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14886/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14886/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 54s Maven dependency ordering for branch
        +1 mvninstall 13m 24s trunk passed
        +1 compile 7m 48s trunk passed
        +1 checkstyle 0m 47s trunk passed
        +1 mvnsite 1m 46s trunk passed
        +1 mvneclipse 0m 56s trunk passed
        +1 findbugs 3m 14s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 20s the patch passed
        +1 compile 5m 52s the patch passed
        +1 javac 5m 52s the patch passed
        +1 checkstyle 0m 46s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)
        +1 mvnsite 1m 44s the patch passed
        +1 mvneclipse 0m 54s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 3m 43s the patch passed
        +1 javadoc 1m 24s the patch passed
        +1 unit 0m 32s hadoop-yarn-api in the patch passed.
        +1 unit 2m 29s hadoop-yarn-common in the patch passed.
        -1 unit 40m 9s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 34s The patch does not generate ASF License warnings.
        98m 59s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852166/YARN-6125.007.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 6100330f4897 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 07a5184
        Default Java 1.8.0_121
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14885/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14885/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14885/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 54s Maven dependency ordering for branch +1 mvninstall 13m 24s trunk passed +1 compile 7m 48s trunk passed +1 checkstyle 0m 47s trunk passed +1 mvnsite 1m 46s trunk passed +1 mvneclipse 0m 56s trunk passed +1 findbugs 3m 14s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 5m 52s the patch passed +1 javac 5m 52s the patch passed +1 checkstyle 0m 46s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318) +1 mvnsite 1m 44s the patch passed +1 mvneclipse 0m 54s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 3m 43s the patch passed +1 javadoc 1m 24s the patch passed +1 unit 0m 32s hadoop-yarn-api in the patch passed. +1 unit 2m 29s hadoop-yarn-common in the patch passed. -1 unit 40m 9s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 98m 59s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12852166/YARN-6125.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 6100330f4897 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 07a5184 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/14885/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14885/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14885/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Javadoc for methods done. Renaming KB to KC w/ explanation in yarn-default.xml done.

        Show
        andras.piros Andras Piros added a comment - Javadoc for methods done. Renaming KB to KC w/ explanation in yarn-default.xml done.
        Hide
        templedf Daniel Templeton added a comment -

        My original consideration was that we should distinguish between the current length (after truncate without header and ellipsis) and the total length.

        Fair point. You should document that clearly in a javadoc header.

        switched to "chars" also for docs and all messages

        Almost. YarnConfiguration and yarn-default.xml still talk about kB.

        Show
        templedf Daniel Templeton added a comment - My original consideration was that we should distinguish between the current length (after truncate without header and ellipsis) and the total length. Fair point. You should document that clearly in a javadoc header. switched to "chars" also for docs and all messages Almost. YarnConfiguration and yarn-default.xml still talk about kB.
        Hide
        templedf Daniel Templeton added a comment -

        I agree on #3. I thought I had deleted that comment after thinking more about it.

        Show
        templedf Daniel Templeton added a comment - I agree on #3. I thought I had deleted that comment after thinking more about it.
        Hide
        andras.piros Andras Piros added a comment -

        Latest greatest thing.

        Show
        andras.piros Andras Piros added a comment - Latest greatest thing.
        Hide
        andras.piros Andras Piros added a comment - - edited

        Daniel Templeton latest code review comment responses:

        1. log and exception messages:
          1. done, except for "chars"
          2. done
          3. done
          4. see next one
        2. decided to use only append() and remove appendToDiagnosticsSafely()
        3. as we don't want to count message header and ellipses against limit, I'd stick to the current implementation as the header is of variable length. checkAndCut() can remain as simple as check whether messages would extend beyond limit or not
        4. done
        5. done. My original consideration was that we should distinguish between the current length (after truncate without header and ellipsis) and the total length. Anyway, renamed
        6. switched to "chars" also for docs and all messages
        Show
        andras.piros Andras Piros added a comment - - edited Daniel Templeton latest code review comment responses: log and exception messages: done, except for "chars" done done see next one decided to use only append() and remove appendToDiagnosticsSafely() as we don't want to count message header and ellipses against limit , I'd stick to the current implementation as the header is of variable length. checkAndCut() can remain as simple as check whether messages would extend beyond limit or not done done. My original consideration was that we should distinguish between the current length (after truncate without header and ellipsis) and the total length. Anyway, renamed switched to "chars" also for docs and all messages
        Hide
        templedf Daniel Templeton added a comment -

        Looking good, Andras Piros. A few more comments:

        • On the log and exception messages:
          • "Diagnostic messages truncated, showing last (%d) chars out of (%d) in total:" would be better without the parentheses, and "in total" is redundant. Should "chars" be "bytes?"
          • Instead of "<property> (<value>) should be positive," I'd prefer "The value of <property> should be a positive integer: <value>."
          • "<value> should be a positive integer. Exception message: <message>," I'd prefer that same message: "The value of <property> should be a positive integer: <value>." The exception message on a NFE is seldom useful.
          • With, "There was a problem appending a diagnostics message to an RM app attempt. Here is the message: <message>," I'd leave out the last sentence and reword a little, "There was an issue updating the diagnostic message for RM app attempt <id>: <message>" But, see my next point...
        • In appendToDiagnosticsSafely(), while IndexOutOfBoundsException is theoretically possible, it can't happen in practice. I'm not sure why the IllegalStateException is there. Is it safe to drop this method completely in favor of just calling append()?
        • In BoundedAppender, I think it would be simpler to always just prepend the header to the string directly instead of adding it in the toString().
        • It would be good to have some javadocs to explain why the BoundedAppender is there and how it's intended to function.
        • Any reason not to name currentLength() just {length()}}?
        • The docs talk about bytes, but the messages and code deal in characters. Not always the same thing. It's easier to talk about bytes, but easier to code for characters. Let's pick one or the other.
        Show
        templedf Daniel Templeton added a comment - Looking good, Andras Piros . A few more comments: On the log and exception messages: "Diagnostic messages truncated, showing last (%d) chars out of (%d) in total:" would be better without the parentheses, and "in total" is redundant. Should "chars" be "bytes?" Instead of "<property> (<value>) should be positive," I'd prefer "The value of <property> should be a positive integer: <value>." "<value> should be a positive integer. Exception message: <message>," I'd prefer that same message: "The value of <property> should be a positive integer: <value>." The exception message on a NFE is seldom useful. With, "There was a problem appending a diagnostics message to an RM app attempt. Here is the message: <message>," I'd leave out the last sentence and reword a little, "There was an issue updating the diagnostic message for RM app attempt <id>: <message>" But, see my next point... In appendToDiagnosticsSafely() , while IndexOutOfBoundsException is theoretically possible, it can't happen in practice. I'm not sure why the IllegalStateException is there. Is it safe to drop this method completely in favor of just calling append() ? In BoundedAppender , I think it would be simpler to always just prepend the header to the string directly instead of adding it in the toString() . It would be good to have some javadocs to explain why the BoundedAppender is there and how it's intended to function. Any reason not to name currentLength() just {length()}}? The docs talk about bytes, but the messages and code deal in characters. Not always the same thing. It's easier to talk about bytes, but easier to code for characters. Let's pick one or the other.
        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 52s Maven dependency ordering for branch
        +1 mvninstall 12m 49s trunk passed
        +1 compile 7m 46s trunk passed
        +1 checkstyle 0m 49s trunk passed
        +1 mvnsite 1m 45s trunk passed
        +1 mvneclipse 0m 57s trunk passed
        +1 findbugs 3m 15s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 20s the patch passed
        +1 compile 6m 20s the patch passed
        +1 javac 6m 20s the patch passed
        +1 checkstyle 0m 48s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318)
        +1 mvnsite 1m 54s the patch passed
        +1 mvneclipse 0m 50s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 4m 8s the patch passed
        +1 javadoc 1m 24s the patch passed
        +1 unit 0m 33s hadoop-yarn-api in the patch passed.
        +1 unit 2m 38s hadoop-yarn-common in the patch passed.
        +1 unit 39m 44s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 28s The patch does not generate ASF License warnings.
        98m 40s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851768/YARN-6125.006.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux b052774b037e 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 37b4acf
        Default Java 1.8.0_121
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14869/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14869/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 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 52s Maven dependency ordering for branch +1 mvninstall 12m 49s trunk passed +1 compile 7m 46s trunk passed +1 checkstyle 0m 49s trunk passed +1 mvnsite 1m 45s trunk passed +1 mvneclipse 0m 57s trunk passed +1 findbugs 3m 15s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 6m 20s the patch passed +1 javac 6m 20s the patch passed +1 checkstyle 0m 48s hadoop-yarn-project/hadoop-yarn: The patch generated 0 new + 316 unchanged - 2 fixed = 316 total (was 318) +1 mvnsite 1m 54s the patch passed +1 mvneclipse 0m 50s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 4m 8s the patch passed +1 javadoc 1m 24s the patch passed +1 unit 0m 33s hadoop-yarn-api in the patch passed. +1 unit 2m 38s hadoop-yarn-common in the patch passed. +1 unit 39m 44s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 98m 40s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851768/YARN-6125.006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux b052774b037e 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 37b4acf Default Java 1.8.0_121 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14869/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14869/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Small Javadoc fix.

        Show
        andras.piros Andras Piros added a comment - Small Javadoc fix.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 22s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 45s Maven dependency ordering for branch
        +1 mvninstall 14m 1s trunk passed
        +1 compile 9m 5s trunk passed
        +1 checkstyle 0m 49s trunk passed
        +1 mvnsite 1m 54s trunk passed
        +1 mvneclipse 0m 55s trunk passed
        +1 findbugs 3m 35s trunk passed
        +1 javadoc 1m 28s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 26s the patch passed
        +1 compile 6m 48s the patch passed
        +1 javac 6m 48s the patch passed
        -0 checkstyle 0m 50s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 316 unchanged - 2 fixed = 317 total (was 318)
        +1 mvnsite 1m 50s the patch passed
        +1 mvneclipse 0m 54s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 3m 53s the patch passed
        +1 javadoc 1m 20s the patch passed
        +1 unit 0m 34s hadoop-yarn-api in the patch passed.
        +1 unit 2m 37s hadoop-yarn-common in the patch passed.
        -1 unit 40m 44s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 29s The patch does not generate ASF License warnings.
        103m 19s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851552/YARN-6125.005.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 61b1acf0a7b5 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 2007e0c
        Default Java 1.8.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14858/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14858/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14858/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14858/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 45s Maven dependency ordering for branch +1 mvninstall 14m 1s trunk passed +1 compile 9m 5s trunk passed +1 checkstyle 0m 49s trunk passed +1 mvnsite 1m 54s trunk passed +1 mvneclipse 0m 55s trunk passed +1 findbugs 3m 35s trunk passed +1 javadoc 1m 28s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 6m 48s the patch passed +1 javac 6m 48s the patch passed -0 checkstyle 0m 50s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 316 unchanged - 2 fixed = 317 total (was 318) +1 mvnsite 1m 50s the patch passed +1 mvneclipse 0m 54s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 3m 53s the patch passed +1 javadoc 1m 20s the patch passed +1 unit 0m 34s hadoop-yarn-api in the patch passed. +1 unit 2m 37s hadoop-yarn-common in the patch passed. -1 unit 40m 44s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 103m 19s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851552/YARN-6125.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 61b1acf0a7b5 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2007e0c Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14858/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14858/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14858/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14858/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Refactored new functionality to nested class RMAppAttemptImpl.BoundedAppender, got rid of the unnecessary stuff.

        Show
        andras.piros Andras Piros added a comment - Refactored new functionality to nested class RMAppAttemptImpl.BoundedAppender , got rid of the unnecessary stuff.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 25s Maven dependency ordering for branch
        +1 mvninstall 13m 5s trunk passed
        +1 compile 8m 25s trunk passed
        +1 checkstyle 0m 48s trunk passed
        +1 mvnsite 1m 50s trunk passed
        +1 mvneclipse 0m 57s trunk passed
        +1 findbugs 3m 28s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 21s the patch passed
        +1 compile 6m 54s the patch passed
        +1 javac 6m 54s the patch passed
        -0 checkstyle 0m 47s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 316 unchanged - 2 fixed = 318 total (was 318)
        +1 mvnsite 1m 42s the patch passed
        +1 mvneclipse 0m 53s the patch passed
        -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 xml 0m 2s The patch has no ill-formed XML file.
        -1 findbugs 1m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
        -1 javadoc 0m 31s hadoop-yarn-common in the patch failed.
        +1 unit 0m 30s hadoop-yarn-api in the patch passed.
        +1 unit 2m 26s hadoop-yarn-common in the patch passed.
        -1 unit 39m 41s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 30s The patch does not generate ASF License warnings.
        99m 26s



        Reason Tests
        FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Format string should use %n rather than n in org.apache.hadoop.yarn.util.BoundedAppender.toString() At BoundedAppender.java:rather than n in org.apache.hadoop.yarn.util.BoundedAppender.toString() At BoundedAppender.java:[line 259]
        Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851489/YARN-6125.004.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 185557f03586 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / deb368b
        Default Java 1.8.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/whitespace-eol.txt
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.html
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14852/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14852/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 25s Maven dependency ordering for branch +1 mvninstall 13m 5s trunk passed +1 compile 8m 25s trunk passed +1 checkstyle 0m 48s trunk passed +1 mvnsite 1m 50s trunk passed +1 mvneclipse 0m 57s trunk passed +1 findbugs 3m 28s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 6m 54s the patch passed +1 javac 6m 54s the patch passed -0 checkstyle 0m 47s hadoop-yarn-project/hadoop-yarn: The patch generated 2 new + 316 unchanged - 2 fixed = 318 total (was 318) +1 mvnsite 1m 42s the patch passed +1 mvneclipse 0m 53s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 2s The patch has no ill-formed XML file. -1 findbugs 1m 14s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) -1 javadoc 0m 31s hadoop-yarn-common in the patch failed. +1 unit 0m 30s hadoop-yarn-api in the patch passed. +1 unit 2m 26s hadoop-yarn-common in the patch passed. -1 unit 39m 41s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 99m 26s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common   Format string should use %n rather than n in org.apache.hadoop.yarn.util.BoundedAppender.toString() At BoundedAppender.java:rather than n in org.apache.hadoop.yarn.util.BoundedAppender.toString() At BoundedAppender.java: [line 259] Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12851489/YARN-6125.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 185557f03586 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / deb368b Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.html javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14852/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14852/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14852/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Daniel Templeton it's actually always a good idea to throw out code if the feature can remain... but the header message should always change because of the current and total character counts appended, as well as the specified limit. So in any case, either in a separate class like BoundedAppender or within RMAppAttemptImpl we have to take care of that pieces of state.

        I'm for keeping the separate class, makes code more readable for me.

        Show
        andras.piros Andras Piros added a comment - Daniel Templeton it's actually always a good idea to throw out code if the feature can remain... but the header message should always change because of the current and total character counts appended, as well as the specified limit . So in any case, either in a separate class like BoundedAppender or within RMAppAttemptImpl we have to take care of that pieces of state. I'm for keeping the separate class, makes code more readable for me.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks for the update, Andras Piros. Now that we're just truncating the top, do we still need the full bounded appender? Seems like the RM could just truncate the message and reapply the header after each append. Because we're trimming to the exact length, once we start truncating, the additional header will make sure every append ends up truncating.

        Show
        templedf Daniel Templeton added a comment - Thanks for the update, Andras Piros . Now that we're just truncating the top, do we still need the full bounded appender? Seems like the RM could just truncate the message and reapply the header after each append. Because we're trimming to the exact length, once we start truncating, the additional header will make sure every append ends up truncating.
        Hide
        andras.piros Andras Piros added a comment -

        Good. Daniel Templeton neither the header nor the ellipses will be counted against the limit.

        Show
        andras.piros Andras Piros added a comment - Good. Daniel Templeton neither the header nor the ellipses will be counted against the limit.
        Hide
        templedf Daniel Templeton added a comment -

        Works for me. Are you planning to count the header against the limit? I'd say maybe you shouldn't.

        Show
        templedf Daniel Templeton added a comment - Works for me. Are you planning to count the header against the limit? I'd say maybe you shouldn't.
        Hide
        jlowe Jason Lowe added a comment -

        Seems fine to me.

        Show
        jlowe Jason Lowe added a comment - Seems fine to me.
        Hide
        andras.piros Andras Piros added a comment -

        Thanks Jason Lowe and Daniel Templeton for your ideas!

        So to summarize, when truncating:

        • oldest message first (FIFO)
        • header stating how many bytes / characters showing from how many total ones
        • last truncated message is truncated from the head, keeping its last chars
        • ellipses at the head of the last message truncated

        Example:

        Diagnostic messages truncated, showing last N chars out of M chars in total:
        ...rter than the previour one
        message3 this is even shorter
        message4 the shortest one
        
        Show
        andras.piros Andras Piros added a comment - Thanks Jason Lowe and Daniel Templeton for your ideas! So to summarize, when truncating: oldest message first (FIFO) header stating how many bytes / characters showing from how many total ones last truncated message is truncated from the head, keeping its last chars ellipses at the head of the last message truncated Example: Diagnostic messages truncated, showing last N chars out of M chars in total: ...rter than the previour one message3 this is even shorter message4 the shortest one
        Hide
        templedf Daniel Templeton added a comment -

        I would argue that a partial stack trace, whether head or tail, is mostly useless, but with the head you at least get the exception message. It's not that big of a deal, though, because the buffer is big enough to accommodate any reasonable stack trace, and the most likely source of an overflow is repeated messages.

        I'm fine with just doing a blind truncate on the head of the message and leave it to a future JIRA if someone thinks we need to be more clever. Adding a header that says "<n> bytes truncated" would also be helpful.

        Show
        templedf Daniel Templeton added a comment - I would argue that a partial stack trace, whether head or tail, is mostly useless, but with the head you at least get the exception message. It's not that big of a deal, though, because the buffer is big enough to accommodate any reasonable stack trace, and the most likely source of an overflow is repeated messages. I'm fine with just doing a blind truncate on the head of the message and leave it to a future JIRA if someone thinks we need to be more clever. Adding a header that says "<n> bytes truncated" would also be helpful.
        Hide
        jlowe Jason Lowe added a comment -

        I would argue the most important part of many stacktraces isn't the top but rather the last 'Caused by' clause of the stacktrace. Anyway I was under the impression that we were giving a sufficient buffer size to accommodate any reasonable stacktrace. If that's still the case then I'd prefer to keep this simple.

        Show
        jlowe Jason Lowe added a comment - I would argue the most important part of many stacktraces isn't the top but rather the last 'Caused by' clause of the stacktrace. Anyway I was under the impression that we were giving a sufficient buffer size to accommodate any reasonable stacktrace. If that's still the case then I'd prefer to keep this simple.
        Hide
        templedf Daniel Templeton added a comment -

        That is certainly an option. The point that Yufei Gu made is that if a message is a stack trace, the most important stuff is at the top. What Andras Piros is saying is that we should drop the oldest message first, but starting from its tail, not its head. That way we get:

        java.io.IOException: Failed on local exception: java.io.IOException: javax.security.sasl.SaslException: GSS initiate failed [Caused by GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)]; Host Details : local host is: "foo.bar.com/127.0.0.1"; destination host is: "foo.bar.com":8020;
        	at org.apache.hadoop.net.NetUtils.wrapException(NetUtils.java:772)
        	at org.apache.hadoop.ipc.Client.call(Client.java:1476)
        	at org.apache.hadoop.ipc.Client.call(Client.java:1409)
        	at org.apache.hadoop.ipc.ProtobufRpcEngine$Invoker.invoke(ProtobufRpcEngine.java:230)
        	at com.sun.proxy.$Proxy14.getFileInfo(Unknown Source)
        ...

        instead of

        ...
        	at sun.security.jgss.krb5.Krb5MechFactory.getMechanismContext(Krb5MechFactory.java:187)
        	at sun.security.jgss.GSSManagerImpl.getMechanismContext(GSSManagerImpl.java:223)
        	at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:212)
        	at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:179)
        	at com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:193)
        	... 41 more
        Show
        templedf Daniel Templeton added a comment - That is certainly an option. The point that Yufei Gu made is that if a message is a stack trace, the most important stuff is at the top. What Andras Piros is saying is that we should drop the oldest message first, but starting from its tail, not its head. That way we get: java.io.IOException: Failed on local exception: java.io.IOException: javax.security.sasl.SaslException: GSS initiate failed [Caused by GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)]; Host Details : local host is: "foo.bar.com/127.0.0.1"; destination host is: "foo.bar.com":8020; at org.apache.hadoop.net.NetUtils.wrapException(NetUtils.java:772) at org.apache.hadoop.ipc.Client.call(Client.java:1476) at org.apache.hadoop.ipc.Client.call(Client.java:1409) at org.apache.hadoop.ipc.ProtobufRpcEngine$Invoker.invoke(ProtobufRpcEngine.java:230) at com.sun.proxy.$Proxy14.getFileInfo(Unknown Source) ... instead of ... at sun.security.jgss.krb5.Krb5MechFactory.getMechanismContext(Krb5MechFactory.java:187) at sun.security.jgss.GSSManagerImpl.getMechanismContext(GSSManagerImpl.java:223) at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:212) at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:179) at com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:193) ... 41 more
        Hide
        jlowe Jason Lowe added a comment -

        I suppose we could do ellipses, but I'm still a bit confused. Are we both truncating individual messages as they come in as well as the entire diagnostics? I was under the impression we're not worrying about messages but the diagnostics buffer as a whole. Whether they try to fill it with one big message, a few messages, or one character at a time over a zillion calls, I would expect it to look something like this, using the example 1 above as the original diagnostics from the app:

        ...rter than the previour one
        message3 this is even shorter
        message4 the shortest one
        

        Again I think it could be useful to convey to the user how much of the message was truncated (i.e.: showing last N of X bytes) so they have an idea of how much they're missing. But the first cut could just be ellipses at the start and the last N bytes of the diagnostic messages where N is the configured diagnostic limit.

        Show
        jlowe Jason Lowe added a comment - I suppose we could do ellipses, but I'm still a bit confused. Are we both truncating individual messages as they come in as well as the entire diagnostics? I was under the impression we're not worrying about messages but the diagnostics buffer as a whole. Whether they try to fill it with one big message, a few messages, or one character at a time over a zillion calls, I would expect it to look something like this, using the example 1 above as the original diagnostics from the app: ...rter than the previour one message3 this is even shorter message4 the shortest one Again I think it could be useful to convey to the user how much of the message was truncated (i.e.: showing last N of X bytes) so they have an idea of how much they're missing. But the first cut could just be ellipses at the start and the last N bytes of the diagnostic messages where N is the configured diagnostic limit.
        Hide
        templedf Daniel Templeton added a comment -

        My original thought there was something more like:

        <PREVIOUS MESSAGES DELETED>
        message3 this has been one that did not fit totally into the buffer<MESSAGE TAIL TRUNCATED>
        message4 the shortest one

        so that it's extremely clear what happened to the rest of the message. In Andras Piros's example above, I think the ellipses fill that role, making the more explicit tag superfluous. Are ellipses enough, or would a user be potentially confused?

        Show
        templedf Daniel Templeton added a comment - My original thought there was something more like: <PREVIOUS MESSAGES DELETED> message3 this has been one that did not fit totally into the buffer<MESSAGE TAIL TRUNCATED> message4 the shortest one so that it's extremely clear what happened to the rest of the message. In Andras Piros 's example above, I think the ellipses fill that role, making the more explicit tag superfluous. Are ellipses enough, or would a user be potentially confused?
        Hide
        jlowe Jason Lowe added a comment -

        I think it's not necessary to distinguish between message truncation and overall truncation. We can simply tack on a header stating the diagnostics were truncated when reporting. For example, "Application diagnostics too large, only showing the last N bytes:" A potentially useful enhancement could be to track the total number of diagnostics bytes even though we're not keeping all those bytes around, e.g.: "App diagnostics too large, only showing last N bytes of T bytes total:" We may have to do something similar anyway to know when the diagnostics were truncated.

        Show
        jlowe Jason Lowe added a comment - I think it's not necessary to distinguish between message truncation and overall truncation. We can simply tack on a header stating the diagnostics were truncated when reporting. For example, "Application diagnostics too large, only showing the last N bytes:" A potentially useful enhancement could be to track the total number of diagnostics bytes even though we're not keeping all those bytes around, e.g.: "App diagnostics too large, only showing last N bytes of T bytes total:" We may have to do something similar anyway to know when the diagnostics were truncated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s 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 44s Maven dependency ordering for branch
        +1 mvninstall 12m 52s trunk passed
        +1 compile 8m 16s trunk passed
        +1 checkstyle 0m 48s trunk passed
        +1 mvnsite 1m 46s trunk passed
        +1 mvneclipse 0m 54s trunk passed
        +1 findbugs 3m 19s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 21s the patch passed
        +1 compile 6m 7s the patch passed
        +1 javac 6m 7s the patch passed
        +1 checkstyle 0m 47s the patch passed
        +1 mvnsite 1m 43s the patch passed
        +1 mvneclipse 0m 53s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 2s The patch has no ill-formed XML file.
        +1 findbugs 3m 42s the patch passed
        -1 javadoc 0m 34s hadoop-yarn-common in the patch failed.
        +1 unit 0m 32s hadoop-yarn-api in the patch passed.
        +1 unit 2m 26s hadoop-yarn-common in the patch passed.
        -1 unit 54m 36s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 29s The patch does not generate ASF License warnings.
        113m 4s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority
          hadoop.yarn.server.resourcemanager.TestApplicationMasterService
          hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter
          hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA
          hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher
          hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor
          hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart
          hadoop.yarn.server.resourcemanager.TestResourceTrackerService
          hadoop.yarn.server.resourcemanager.TestContainerResourceUsage
          hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairScheduler
          hadoop.yarn.server.resourcemanager.TestDecommissioningNodesWatcher
          hadoop.yarn.server.resourcemanager.TestSignalContainer
          hadoop.yarn.server.resourcemanager.rmapp.TestNodesListManager
          hadoop.yarn.server.resourcemanager.TestRMRestart
          hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart
          hadoop.yarn.server.resourcemanager.rmapp.attempt.TestRMAppAttemptTransitions
          hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel
          hadoop.yarn.server.resourcemanager.TestApplicationCleanup
          hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
          hadoop.yarn.server.resourcemanager.TestRM



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850719/YARN-6125.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux c3e44fe488fb 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 0914fcc
        Default Java 1.8.0_121
        findbugs v3.0.0
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14814/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14814/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14814/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14814/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s 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 44s Maven dependency ordering for branch +1 mvninstall 12m 52s trunk passed +1 compile 8m 16s trunk passed +1 checkstyle 0m 48s trunk passed +1 mvnsite 1m 46s trunk passed +1 mvneclipse 0m 54s trunk passed +1 findbugs 3m 19s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 6m 7s the patch passed +1 javac 6m 7s the patch passed +1 checkstyle 0m 47s the patch passed +1 mvnsite 1m 43s the patch passed +1 mvneclipse 0m 53s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 3m 42s the patch passed -1 javadoc 0m 34s hadoop-yarn-common in the patch failed. +1 unit 0m 32s hadoop-yarn-api in the patch passed. +1 unit 2m 26s hadoop-yarn-common in the patch passed. -1 unit 54m 36s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 113m 4s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority   hadoop.yarn.server.resourcemanager.TestApplicationMasterService   hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter   hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA   hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher   hadoop.yarn.server.resourcemanager.rmapp.TestApplicationLifetimeMonitor   hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart   hadoop.yarn.server.resourcemanager.TestResourceTrackerService   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairScheduler   hadoop.yarn.server.resourcemanager.TestDecommissioningNodesWatcher   hadoop.yarn.server.resourcemanager.TestSignalContainer   hadoop.yarn.server.resourcemanager.rmapp.TestNodesListManager   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart   hadoop.yarn.server.resourcemanager.rmapp.attempt.TestRMAppAttemptTransitions   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel   hadoop.yarn.server.resourcemanager.TestApplicationCleanup   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer   hadoop.yarn.server.resourcemanager.TestRM Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850719/YARN-6125.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux c3e44fe488fb 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0914fcc Default Java 1.8.0_121 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14814/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14814/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14814/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14814/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment - - edited
        • ExpectedException.none() as factory method, and ExpectedException general usage description can be found here. Basically if you don't expect any Exception s to be thrown, you don't call expect(), expectMessage() or expectCause(). Yes, you can always test exceptional behavior w/ try-catch idiom instead of ExpectedException
        • got rid of Lists.newLinkedList() call
        • got rid of that final variable

        Posting the patch addressing those issues but not yet the truncate behavior.

        Talked to Daniel Templeton and Yufei Gu offline about truncate characteristics, here is one possible solution:

        1. when a message is being appended, and it doesn't fit into the buffer, trim message body to buffer size (minus header prefixes) preserving its head and add a prefix header telling users it's been partially truncated
        2. when a message is being appended, and it fits into the buffer:
          1. past messages are deleted in FIFO order
          2. for all the ones deleted we put a common prefix header stating there have been messages left out
          3. for the last one truncated we trim the message body and add a prefix header stating part of its tail has been left out

        Some example scenarios, considering four consecutive append() calls w/ different messages each:

        1. nothing truncated:
          message1 this is a very long message that fits into the buffer
          message2 this is a lot shorter than the previour one
          message3 this is even shorter
          message4 the shortest one
          
        2. only the very last message has been partially truncated:
          message1 this is a very long message that fits into the buffer
          message2 this is a lot shorter than the previour one
          message3 this is even shorter
          <MESSAGE TAIL TRUNCATED>message4 this has been one that did not fit totally into the buffer...
          
        3. only the very first message has been partially truncated:
          <MESSAGE TAIL TRUNCATED>message1 this has been one that did not fit totally into the buffer...
          message2 this is a very long message that fits into the buffer
          message3 this is a lot shorter than the previour one
          message4 the shortest one
          
        4. the first several ones have been deleted, the previous one has been truncated partially:
          <PREVIOUS MESSAGES DELETED>
          <MESSAGE TAIL TRUNCATED>message3 this has been one that did not fit totally into the buffer...
          message4 the shortest one
          
        5. all the previous ones have been deleted, plus the last one has been truncated:
          <PREVIOUS MESSAGES DELETED>
          <MESSAGE TAIL TRUNCATED>message4 this has been one that did not fit totally into the buffer...
          

        Jason Lowe, Karthik Kambatla, Varun Vasudev what is your opinion about the proposal?

        Show
        andras.piros Andras Piros added a comment - - edited ExpectedException.none() as factory method, and ExpectedException general usage description can be found here . Basically if you don't expect any Exception s to be thrown, you don't call expect() , expectMessage() or expectCause() . Yes, you can always test exceptional behavior w/ try-catch idiom instead of ExpectedException got rid of Lists.newLinkedList() call got rid of that final variable Posting the patch addressing those issues but not yet the truncate behavior. Talked to Daniel Templeton and Yufei Gu offline about truncate characteristics, here is one possible solution: when a message is being appended, and it doesn't fit into the buffer, trim message body to buffer size (minus header prefixes) preserving its head and add a prefix header telling users it's been partially truncated when a message is being appended, and it fits into the buffer: past messages are deleted in FIFO order for all the ones deleted we put a common prefix header stating there have been messages left out for the last one truncated we trim the message body and add a prefix header stating part of its tail has been left out Some example scenarios, considering four consecutive append() calls w/ different messages each: nothing truncated: message1 this is a very long message that fits into the buffer message2 this is a lot shorter than the previour one message3 this is even shorter message4 the shortest one only the very last message has been partially truncated: message1 this is a very long message that fits into the buffer message2 this is a lot shorter than the previour one message3 this is even shorter <MESSAGE TAIL TRUNCATED>message4 this has been one that did not fit totally into the buffer... only the very first message has been partially truncated: <MESSAGE TAIL TRUNCATED>message1 this has been one that did not fit totally into the buffer... message2 this is a very long message that fits into the buffer message3 this is a lot shorter than the previour one message4 the shortest one the first several ones have been deleted, the previous one has been truncated partially: <PREVIOUS MESSAGES DELETED> <MESSAGE TAIL TRUNCATED>message3 this has been one that did not fit totally into the buffer... message4 the shortest one all the previous ones have been deleted, plus the last one has been truncated: <PREVIOUS MESSAGES DELETED> <MESSAGE TAIL TRUNCATED>message4 this has been one that did not fit totally into the buffer... Jason Lowe , Karthik Kambatla , Varun Vasudev what is your opinion about the proposal?
        Hide
        templedf Daniel Templeton added a comment -

        the pom.xml dependency reordering was necessary in order to get ExpectedException working.

        What is the net effect of declaring that no exception is expected? Is it any different from not declaring anything?

        Lists.newLinkedList() comes from Guava, which I like.

        The issue isn't Guava. The issue is that it was created to replace new LinkedList<Integer>() (in this case), but as of Java 7 there's the diamond operator. If you read the javadoc for newList(), it says not to use it with Java 7 or later.

        I like final stuff (I know I'm weird), please specify where I should make variables / fields non-final

        I was thinking specifically of

            final int inputLength = csq.length();

        inputLength is only ever used as a parameter, so declaring it final is kinda overkill.

        While you're working on the patch, another architectural concern has occurred to me. Assume I have a 64k limit. I append a message that's 65,530 bytes long. I then append a message that's 10 bytes long. The current implementation will drop the first message completely. Seems like it might be better to only drop the first line of the first message, just enough to trim it down. A partial message may be better than no message. Granted, it's probably not hugely useful to only have the tail of a message, but from a user's perspective it's less unexpected to get a partial message than to drop a large message entirely. One could make the same case for just cutting it down to the exact length instead of minding line boundaries... Jason Lowe, Karthik Kambatla, Varun Vasudev, Yufei Gu, any thoughts?

        Oh, one more thing... Can we add a header that shows the content was trimmed? If we're not just clipping to the buffer size, it would be useful to have a way to notify the user that the message is not complete. In most cases it will be, so we want to be clear when it isn't.

        Show
        templedf Daniel Templeton added a comment - the pom.xml dependency reordering was necessary in order to get ExpectedException working. What is the net effect of declaring that no exception is expected? Is it any different from not declaring anything? Lists.newLinkedList() comes from Guava, which I like. The issue isn't Guava. The issue is that it was created to replace new LinkedList<Integer>() (in this case), but as of Java 7 there's the diamond operator. If you read the javadoc for newList() , it says not to use it with Java 7 or later. I like final stuff (I know I'm weird), please specify where I should make variables / fields non-final I was thinking specifically of final int inputLength = csq.length(); inputLength is only ever used as a parameter, so declaring it final is kinda overkill. While you're working on the patch, another architectural concern has occurred to me. Assume I have a 64k limit. I append a message that's 65,530 bytes long. I then append a message that's 10 bytes long. The current implementation will drop the first message completely. Seems like it might be better to only drop the first line of the first message, just enough to trim it down. A partial message may be better than no message. Granted, it's probably not hugely useful to only have the tail of a message, but from a user's perspective it's less unexpected to get a partial message than to drop a large message entirely. One could make the same case for just cutting it down to the exact length instead of minding line boundaries... Jason Lowe , Karthik Kambatla , Varun Vasudev , Yufei Gu , any thoughts? Oh, one more thing... Can we add a header that shows the content was trimmed? If we're not just clipping to the buffer size, it would be useful to have a way to notify the user that the message is not complete. In most cases it will be, so we want to be clear when it isn't.
        Hide
        andras.piros Andras Piros added a comment - - edited

        Daniel Templeton thanks for the review!

        My thoughts on the comments:

        1. done
        2. done
        3. the pom.xml dependency reordering was necessary in order to get ExpectedException working. There are other parts of hadoop that employ the same thing (notably, hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-nodemanager/pom.xml), essentially to make sure the correct Hamcrest version is on the classpath. One day switching to Mockito 2.x should solve this problem
        4. Lists.newLinkedList() comes from Guava, which I like. But nevermind, changed to use new LinkedList<>()
        5. done
        6. done
        7. done
        8. I like final stuff (I know I'm weird), please specify where I should make variables / fields non-final
        9. WIP
        10. done
        11. done
        12. done
        13. done
        14. done
        Show
        andras.piros Andras Piros added a comment - - edited Daniel Templeton thanks for the review! My thoughts on the comments: done done the pom.xml dependency reordering was necessary in order to get ExpectedException working. There are other parts of hadoop that employ the same thing (notably, hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-nodemanager/pom.xml ), essentially to make sure the correct Hamcrest version is on the classpath. One day switching to Mockito 2.x should solve this problem Lists.newLinkedList() comes from Guava, which I like. But nevermind, changed to use new LinkedList<>() done done done I like final stuff (I know I'm weird), please specify where I should make variables / fields non- final WIP done done done done done
        Hide
        templedf Daniel Templeton added a comment -

        Thanks for the patch. Some comments:

        • In the property, let's use "limit" instead of "capacity." It sounds firmer.
        • Instead of DEFAULT_APP_ATTEMPT_DIAGNOSTICS_CAPACITY_BYTES, I think DEFAULT_APP_ATTEMPT_DIAGNOSTICS_CAPACITY_KB might be better. Then you can default to 64 instead of 65536.
        • I can't comment on the POM change. Anyone else want to comment?
        • Lists.newLinkedList() is a Java 6 thing. Just use the LinkedList constructor directly.
        • I don't think we need the default constructor.
        • You should do parameter validation in RMAppAttemptImpl before passing the capacity to the constructor. You can offer a more informative error message that way.
        • ensureNull() should probably be ensureNotNull().
        • I don't see any good reason to have inputLength be final.
        • Your checks at the beginning of cutAtLeast(), checkAndCut(), and append will bring down the RM if they're violated. That's bad. You should probably log and ignore instead, probably at the level of the RM, instead of in the appender. Except for checkAndCut(), where you should either trim down the new message to fit or allow the oversized message to be appended. Probably the former.
        • Your last append should probably just call the first append, and the first one should probably call the second one.
        • Just leave out the javadoc on the overridden methods instead of stubbing it out.
        • The description in yarn-default.xml, the description should maybe be "Defines the maximum capacity of the diagnostic message for each application attempt, in bytes. When using ZooKeeper to store application state behavior, it's important to limit the size of the diagnostic messages to prevent YARN from overwhelming ZooKeeper. In cases where yarn.resourcemanager.state-store.max-completed-applications is set to a large number, it may be desirable to reduce the value of this property to limit the total data stored."
        • No need for the empty setup() in the test.
        • initWithPositiveCapacitySuccess() should assert something.
        Show
        templedf Daniel Templeton added a comment - Thanks for the patch. Some comments: In the property, let's use "limit" instead of "capacity." It sounds firmer. Instead of DEFAULT_APP_ATTEMPT_DIAGNOSTICS_CAPACITY_BYTES , I think DEFAULT_APP_ATTEMPT_DIAGNOSTICS_CAPACITY_KB might be better. Then you can default to 64 instead of 65536. I can't comment on the POM change. Anyone else want to comment? Lists.newLinkedList() is a Java 6 thing. Just use the LinkedList constructor directly. I don't think we need the default constructor. You should do parameter validation in RMAppAttemptImpl before passing the capacity to the constructor. You can offer a more informative error message that way. ensureNull() should probably be ensureNotNull() . I don't see any good reason to have inputLength be final. Your checks at the beginning of cutAtLeast() , checkAndCut() , and append will bring down the RM if they're violated. That's bad. You should probably log and ignore instead, probably at the level of the RM, instead of in the appender. Except for checkAndCut() , where you should either trim down the new message to fit or allow the oversized message to be appended. Probably the former. Your last append should probably just call the first append, and the first one should probably call the second one. Just leave out the javadoc on the overridden methods instead of stubbing it out. The description in yarn-default.xml , the description should maybe be "Defines the maximum capacity of the diagnostic message for each application attempt, in bytes. When using ZooKeeper to store application state behavior, it's important to limit the size of the diagnostic messages to prevent YARN from overwhelming ZooKeeper. In cases where yarn.resourcemanager.state-store.max-completed-applications is set to a large number, it may be desirable to reduce the value of this property to limit the total data stored." No need for the empty setup() in the test. initWithPositiveCapacitySuccess() should assert something.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 48s Maven dependency ordering for branch
        +1 mvninstall 15m 58s trunk passed
        +1 compile 8m 51s trunk passed
        +1 checkstyle 0m 55s trunk passed
        +1 mvnsite 2m 1s trunk passed
        +1 mvneclipse 1m 5s trunk passed
        +1 findbugs 4m 3s trunk passed
        +1 javadoc 1m 42s trunk passed
        0 mvndep 0m 13s Maven dependency ordering for patch
        +1 mvninstall 1m 46s the patch passed
        +1 compile 8m 1s the patch passed
        +1 javac 8m 1s the patch passed
        -0 checkstyle 0m 57s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 318 unchanged - 0 fixed = 319 total (was 318)
        +1 mvnsite 1m 59s the patch passed
        +1 mvneclipse 1m 4s the patch passed
        -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
        +1 xml 0m 3s The patch has no ill-formed XML file.
        +1 findbugs 4m 17s the patch passed
        -1 javadoc 0m 37s hadoop-yarn-common in the patch failed.
        +1 unit 0m 34s hadoop-yarn-api in the patch passed.
        +1 unit 2m 38s hadoop-yarn-common in the patch passed.
        +1 unit 41m 45s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 33s The patch does not generate ASF License warnings.
        109m 49s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850317/YARN-6125.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 8862700e7735 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 258991d
        Default Java 1.8.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/whitespace-eol.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14802/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14802/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 48s Maven dependency ordering for branch +1 mvninstall 15m 58s trunk passed +1 compile 8m 51s trunk passed +1 checkstyle 0m 55s trunk passed +1 mvnsite 2m 1s trunk passed +1 mvneclipse 1m 5s trunk passed +1 findbugs 4m 3s trunk passed +1 javadoc 1m 42s trunk passed 0 mvndep 0m 13s Maven dependency ordering for patch +1 mvninstall 1m 46s the patch passed +1 compile 8m 1s the patch passed +1 javac 8m 1s the patch passed -0 checkstyle 0m 57s hadoop-yarn-project/hadoop-yarn: The patch generated 1 new + 318 unchanged - 0 fixed = 319 total (was 318) +1 mvnsite 1m 59s the patch passed +1 mvneclipse 1m 4s the patch passed -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 3s The patch has no ill-formed XML file. +1 findbugs 4m 17s the patch passed -1 javadoc 0m 37s hadoop-yarn-common in the patch failed. +1 unit 0m 34s hadoop-yarn-api in the patch passed. +1 unit 2m 38s hadoop-yarn-common in the patch passed. +1 unit 41m 45s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 109m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850317/YARN-6125.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 8862700e7735 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 258991d Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14802/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14802/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14802/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment - - edited

        CheckStyle, Javadoc, and code format modifications.

        Note that the three failing tests TestRMRestart, TestResourceTrackerService, and TestFSAppStarvation are passing locally, and are unrelated.

        Show
        andras.piros Andras Piros added a comment - - edited CheckStyle, Javadoc, and code format modifications. Note that the three failing tests TestRMRestart , TestResourceTrackerService , and TestFSAppStarvation are passing locally, and are unrelated.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 15s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        0 mvndep 0m 8s Maven dependency ordering for branch
        +1 mvninstall 12m 27s trunk passed
        +1 compile 7m 29s trunk passed
        +1 checkstyle 0m 47s trunk passed
        +1 mvnsite 1m 43s trunk passed
        +1 mvneclipse 0m 56s trunk passed
        +1 findbugs 3m 12s trunk passed
        +1 javadoc 1m 23s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 21s the patch passed
        +1 compile 6m 45s the patch passed
        +1 javac 6m 45s the patch passed
        -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 202 new + 318 unchanged - 0 fixed = 520 total (was 318)
        +1 mvnsite 1m 41s the patch passed
        +1 mvneclipse 0m 54s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 3s The patch has no ill-formed XML file.
        +1 findbugs 3m 33s the patch passed
        -1 javadoc 0m 34s hadoop-yarn-common in the patch failed.
        +1 unit 0m 31s hadoop-yarn-api in the patch passed.
        +1 unit 2m 24s hadoop-yarn-common in the patch passed.
        -1 unit 39m 20s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 29s The patch does not generate ASF License warnings.
        96m 25s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation
          hadoop.yarn.server.resourcemanager.TestRMRestart
          hadoop.yarn.server.resourcemanager.TestResourceTrackerService



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:a9ad5d6
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850117/YARN-6125.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 3b2d758ed5cc 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 11e44bd
        Default Java 1.8.0_121
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14797/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14797/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 12m 27s trunk passed +1 compile 7m 29s trunk passed +1 checkstyle 0m 47s trunk passed +1 mvnsite 1m 43s trunk passed +1 mvneclipse 0m 56s trunk passed +1 findbugs 3m 12s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 6m 45s the patch passed +1 javac 6m 45s the patch passed -0 checkstyle 0m 49s hadoop-yarn-project/hadoop-yarn: The patch generated 202 new + 318 unchanged - 0 fixed = 520 total (was 318) +1 mvnsite 1m 41s the patch passed +1 mvneclipse 0m 54s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 3s The patch has no ill-formed XML file. +1 findbugs 3m 33s the patch passed -1 javadoc 0m 34s hadoop-yarn-common in the patch failed. +1 unit 0m 31s hadoop-yarn-api in the patch passed. +1 unit 2m 24s hadoop-yarn-common in the patch passed. -1 unit 39m 20s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 96m 25s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFSAppStarvation   hadoop.yarn.server.resourcemanager.TestRMRestart   hadoop.yarn.server.resourcemanager.TestResourceTrackerService Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850117/YARN-6125.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 3b2d758ed5cc 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 11e44bd Default Java 1.8.0_121 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/14797/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14797/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn Console output https://builds.apache.org/job/PreCommit-YARN-Build/14797/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Patch based on apache/trunk instead of apache/master.

        Show
        andras.piros Andras Piros added a comment - Patch based on apache/trunk instead of apache/master .
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        -1 patch 0m 5s YARN-6125 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



        Subsystem Report/Notes
        JIRA Issue YARN-6125
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850111/YARN-6125.000.patch
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/14796/console
        Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s YARN-6125 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue YARN-6125 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12850111/YARN-6125.000.patch Console output https://builds.apache.org/job/PreCommit-YARN-Build/14796/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        andras.piros Andras Piros added a comment -

        Initial patch, containing a BoundedAppender with configurable capacity (defaulting to 65536) to be used with RMAppAttemptImpl.diagnostics.

        Show
        andras.piros Andras Piros added a comment - Initial patch, containing a BoundedAppender with configurable capacity (defaulting to 65536 ) to be used with RMAppAttemptImpl.diagnostics .
        Hide
        templedf Daniel Templeton added a comment -

        Agreed. I don't think we need to be too draconian on the limit. 64k seems like a reasonable limit. It's big enough that nothing reasonable should overrun it, but a hell of a lot better than 1MB.

        Show
        templedf Daniel Templeton added a comment - Agreed. I don't think we need to be too draconian on the limit. 64k seems like a reasonable limit. It's big enough that nothing reasonable should overrun it, but a hell of a lot better than 1MB.
        Hide
        jlowe Jason Lowe added a comment -

        So I assume the tail end of the diagnostics would have been just as good as the first? If so, my vote is to roll this out initially with preserving the tail end. We'll have to come up with a reasonable default limit so most stacktraces/errors fit to avoid making things hard to debug by default.

        Show
        jlowe Jason Lowe added a comment - So I assume the tail end of the diagnostics would have been just as good as the first? If so, my vote is to roll this out initially with preserving the tail end. We'll have to come up with a reasonable default limit so most stacktraces/errors fit to avoid making things hard to debug by default.
        Hide
        templedf Daniel Templeton added a comment -

        In the case that I've gotten my hands on the message contents, it was a repeated stack trace from Spark, one for each of many, many retries.

        Show
        templedf Daniel Templeton added a comment - In the case that I've gotten my hands on the message contents, it was a repeated stack trace from Spark, one for each of many, many retries.
        Hide
        jlowe Jason Lowe added a comment -

        For the huge examples that have been encountered so far, what would have worked best for them? Are they simply a gigantic stacktrace, an accumulation of independent diagnostic messages, or potentially recurring, redundant messages for the same error? I normally would tend to lean towards preserving the tail end of the message with the assumption that the most recent error would be logged there, but of course there could be cascading errors and the beginning would be better.

        That's why I'm hoping the real-world examples help shape the direction here. I'd rather not add yet another config that either nobody sets or knows how to set correctly. If we do add a config then the next question is whether that config should be app-specific (e.g.: app framework A's diagnostic approach works best with preserving the end, but preserving the beginning is better for B, etc.).

        Show
        jlowe Jason Lowe added a comment - For the huge examples that have been encountered so far, what would have worked best for them? Are they simply a gigantic stacktrace, an accumulation of independent diagnostic messages, or potentially recurring, redundant messages for the same error? I normally would tend to lean towards preserving the tail end of the message with the assumption that the most recent error would be logged there, but of course there could be cascading errors and the beginning would be better. That's why I'm hoping the real-world examples help shape the direction here. I'd rather not add yet another config that either nobody sets or knows how to set correctly. If we do add a config then the next question is whether that config should be app-specific (e.g.: app framework A's diagnostic approach works best with preserving the end, but preserving the beginning is better for B, etc.).

          People

          • Assignee:
            andras.piros Andras Piros
            Reporter:
            templedf Daniel Templeton
          • Votes:
            0 Vote for this issue
            Watchers:
            13 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development