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

AMLauncher#createAMContainerLaunchContext() should not log the command to be launched indiscriminately

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.7.2
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: resourcemanager
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Introduces a new configuration property, yarn.resourcemanager.amlauncher.log.command. If this property is set to true, then the AM command being launched will be masked in the RM log.

      Description

      The command could contain sensitive information, such as keystore passwords or AWS credentials or other. Instead of logging it as INFO, we should log it as DEBUG and include a property to disable logging it at all. Logging it to a different logger would also be viable and may create a smaller administrative footprint.

      1. YARN-5549.branch-2.001.patch
        5 kB
        Daniel Templeton
      2. YARN-5549.006.patch
        5 kB
        Daniel Templeton
      3. YARN-5549.005.patch
        7 kB
        Daniel Templeton
      4. YARN-5549.004.patch
        1 kB
        Daniel Templeton
      5. YARN-5549.003.patch
        7 kB
        Daniel Templeton
      6. YARN-5549.002.patch
        7 kB
        Daniel Templeton
      7. YARN-5549.001.patch
        7 kB
        Daniel Templeton

        Activity

        Hide
        templedf Daniel Templeton added a comment -

        Here's a patch. I decided that a separate logger wouldn't make sense unless it were for the whole of the AMLauncher class, which is superfluous since the loggers can be configured at the class level. Configuring the AMLauncher logger not to log is too heavy-handed of a solution, though, so this JIRA is still needed.

        In the case that the command line logging is disabled, I still log a message, just without the risky data, to minimize admin confusion.

        I also did a tiny bit of cleanup. I can't help myself.

        Show
        templedf Daniel Templeton added a comment - Here's a patch. I decided that a separate logger wouldn't make sense unless it were for the whole of the AMLauncher class, which is superfluous since the loggers can be configured at the class level. Configuring the AMLauncher logger not to log is too heavy-handed of a solution, though, so this JIRA is still needed. In the case that the command line logging is disabled, I still log a message, just without the risky data, to minimize admin confusion. I also did a tiny bit of cleanup. I can't help myself.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        0 mvndep 0m 10s Maven dependency ordering for branch
        +1 mvninstall 8m 5s trunk passed
        +1 compile 2m 40s trunk passed
        +1 checkstyle 0m 42s trunk passed
        +1 mvnsite 1m 42s trunk passed
        +1 mvneclipse 0m 45s trunk passed
        +1 findbugs 3m 15s trunk passed
        +1 javadoc 1m 12s trunk passed
        0 mvndep 0m 11s Maven dependency ordering for patch
        +1 mvninstall 1m 20s the patch passed
        +1 compile 2m 17s the patch passed
        +1 javac 2m 17s the patch passed
        +1 checkstyle 0m 39s the patch passed
        +1 mvnsite 1m 30s the patch passed
        +1 mvneclipse 0m 36s the patch passed
        -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 3m 8s the patch passed
        -1 javadoc 0m 15s hadoop-yarn-api in the patch failed.
        +1 unit 0m 22s hadoop-yarn-api in the patch passed.
        +1 unit 2m 17s hadoop-yarn-common in the patch passed.
        +1 unit 37m 36s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        71m 2s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825509/YARN-5549.001.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 14828933a68e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 1360bd2
        Default Java 1.8.0_101
        findbugs v3.0.0
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12897/artifact/patchprocess/whitespace-eol.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12897/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12897/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/12897/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 10s Maven dependency ordering for branch +1 mvninstall 8m 5s trunk passed +1 compile 2m 40s trunk passed +1 checkstyle 0m 42s trunk passed +1 mvnsite 1m 42s trunk passed +1 mvneclipse 0m 45s trunk passed +1 findbugs 3m 15s trunk passed +1 javadoc 1m 12s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 2m 17s the patch passed +1 javac 2m 17s the patch passed +1 checkstyle 0m 39s the patch passed +1 mvnsite 1m 30s the patch passed +1 mvneclipse 0m 36s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 8s the patch passed -1 javadoc 0m 15s hadoop-yarn-api in the patch failed. +1 unit 0m 22s hadoop-yarn-api in the patch passed. +1 unit 2m 17s hadoop-yarn-common in the patch passed. +1 unit 37m 36s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 71m 2s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825509/YARN-5549.001.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 14828933a68e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1360bd2 Default Java 1.8.0_101 findbugs v3.0.0 whitespace https://builds.apache.org/job/PreCommit-YARN-Build/12897/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/12897/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12897/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/12897/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Patch to address Jenkins issues.

        Show
        templedf Daniel Templeton added a comment - Patch to address Jenkins issues.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 19s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        0 mvndep 0m 9s Maven dependency ordering for branch
        +1 mvninstall 6m 44s trunk passed
        +1 compile 2m 18s trunk passed
        +1 checkstyle 0m 40s trunk passed
        +1 mvnsite 1m 34s trunk passed
        +1 mvneclipse 0m 42s trunk passed
        +1 findbugs 2m 50s trunk passed
        +1 javadoc 1m 6s trunk passed
        0 mvndep 0m 10s Maven dependency ordering for patch
        +1 mvninstall 1m 17s the patch passed
        +1 compile 2m 16s the patch passed
        +1 javac 2m 16s the patch passed
        +1 checkstyle 0m 38s the patch passed
        +1 mvnsite 1m 28s the patch passed
        +1 mvneclipse 0m 36s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 3m 10s the patch passed
        +1 javadoc 1m 0s the patch passed
        +1 unit 0m 23s hadoop-yarn-api in the patch passed.
        +1 unit 2m 15s hadoop-yarn-common in the patch passed.
        -1 unit 37m 31s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        68m 17s



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



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825525/YARN-5549.002.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux ad2bb92e1d68 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 1360bd2
        Default Java 1.8.0_101
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-YARN-Build/12900/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12900/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/12900/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/12900/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 44s trunk passed +1 compile 2m 18s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 34s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 2m 50s trunk passed +1 javadoc 1m 6s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 17s the patch passed +1 compile 2m 16s the patch passed +1 javac 2m 16s the patch passed +1 checkstyle 0m 38s the patch passed +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 10s the patch passed +1 javadoc 1m 0s the patch passed +1 unit 0m 23s hadoop-yarn-api in the patch passed. +1 unit 2m 15s hadoop-yarn-common in the patch passed. -1 unit 37m 31s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 68m 17s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825525/YARN-5549.002.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux ad2bb92e1d68 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 1360bd2 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-YARN-Build/12900/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/12900/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/12900/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/12900/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rchiang Ray Chiang added a comment -

        I don't have any objections to moving the command message to DEBUG.

        Anyone else?

        Show
        rchiang Ray Chiang added a comment - I don't have any objections to moving the command message to DEBUG. Anyone else?
        Hide
        templedf Daniel Templeton added a comment -

        I just adjusted the wording to make it sound less like the new property can be applied on a per-job basis.

        Show
        templedf Daniel Templeton added a comment - I just adjusted the wording to make it sound less like the new property can be applied on a per-job basis.
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        I don't have any objections to moving the command message to DEBUG.

        +1. Let's just move it to DEBUG or better just remove it - there is no need for a config option here.

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - I don't have any objections to moving the command message to DEBUG. +1. Let's just move it to DEBUG or better just remove it - there is no need for a config option here.
        Hide
        templedf Daniel Templeton added a comment -

        Good point. I'd much prefer not to add a new config param, especially for something as trivial as this. If the command information is needed for debugging, it's available from the job.xml file.

        Show
        templedf Daniel Templeton added a comment - Good point. I'd much prefer not to add a new config param, especially for something as trivial as this. If the command information is needed for debugging, it's available from the job.xml file.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 17m 23s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        0 mvndep 0m 56s Maven dependency ordering for branch
        +1 mvninstall 7m 26s trunk passed
        +1 compile 2m 28s trunk passed
        +1 checkstyle 0m 41s trunk passed
        +1 mvnsite 1m 38s trunk passed
        +1 mvneclipse 0m 43s trunk passed
        +1 findbugs 3m 0s trunk passed
        +1 javadoc 1m 5s trunk passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 22s the patch passed
        +1 compile 2m 23s the patch passed
        +1 javac 2m 23s the patch passed
        +1 checkstyle 0m 40s the patch passed
        +1 mvnsite 1m 31s the patch passed
        +1 mvneclipse 0m 37s 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 21s the patch passed
        +1 javadoc 1m 2s the patch passed
        +1 unit 0m 24s hadoop-yarn-api in the patch passed.
        +1 unit 2m 18s hadoop-yarn-common in the patch passed.
        +1 unit 37m 33s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        87m 57s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825707/YARN-5549.003.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 4c1da263b003 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 / 8b7adf4
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12907/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/12907/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 17m 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 56s Maven dependency ordering for branch +1 mvninstall 7m 26s trunk passed +1 compile 2m 28s trunk passed +1 checkstyle 0m 41s trunk passed +1 mvnsite 1m 38s trunk passed +1 mvneclipse 0m 43s trunk passed +1 findbugs 3m 0s trunk passed +1 javadoc 1m 5s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 22s the patch passed +1 compile 2m 23s the patch passed +1 javac 2m 23s the patch passed +1 checkstyle 0m 40s the patch passed +1 mvnsite 1m 31s the patch passed +1 mvneclipse 0m 37s 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 21s the patch passed +1 javadoc 1m 2s the patch passed +1 unit 0m 24s hadoop-yarn-api in the patch passed. +1 unit 2m 18s hadoop-yarn-common in the patch passed. +1 unit 37m 33s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 87m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825707/YARN-5549.003.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 4c1da263b003 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 / 8b7adf4 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12907/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/12907/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        rchiang Ray Chiang added a comment -

        +1 for removing it. Makes the review much cleaner.

        Show
        rchiang Ray Chiang added a comment - +1 for removing it. Makes the review much cleaner.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s Docker mode activated.
        +1 @author 0m 1s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 mvninstall 6m 43s trunk passed
        +1 compile 0m 32s trunk passed
        +1 checkstyle 0m 20s trunk passed
        +1 mvnsite 0m 38s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 57s trunk passed
        +1 javadoc 0m 21s trunk passed
        +1 mvninstall 0m 30s the patch passed
        +1 compile 0m 29s the patch passed
        +1 javac 0m 29s the patch passed
        +1 checkstyle 0m 17s the patch passed
        +1 mvnsite 0m 35s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 2s the patch passed
        +1 javadoc 0m 18s the patch passed
        +1 unit 37m 20s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        51m 45s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825722/YARN-5549.004.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 0d132e53d3fa 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 407b519
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12908/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/12908/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 1s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 6m 43s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 20s trunk passed +1 mvnsite 0m 38s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 57s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed +1 checkstyle 0m 17s the patch passed +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 2s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 37m 20s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 51m 45s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825722/YARN-5549.004.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0d132e53d3fa 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 407b519 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12908/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/12908/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        If the command information is needed for debugging, it's available from the job.xml file.

        Sorry to pitch in the middle, IMO its better to log it as debug, as its not always MR job and if required it will be helpful to analyze

        Show
        Naganarasimha Naganarasimha G R added a comment - If the command information is needed for debugging, it's available from the job.xml file. Sorry to pitch in the middle, IMO its better to log it as debug, as its not always MR job and if required it will be helpful to analyze
        Hide
        rchiang Ray Chiang added a comment -

        Naganarasimha G R, so you'd rather stick to version 003 of the patch?

        Vinod Kumar Vavilapalli, are you okay with going back one version, patch-wise?

        Show
        rchiang Ray Chiang added a comment - Naganarasimha G R , so you'd rather stick to version 003 of the patch? Vinod Kumar Vavilapalli , are you okay with going back one version, patch-wise?
        Hide
        templedf Daniel Templeton added a comment -

        With the log message set to debug and guarded by a property, the message is not useful for historical purposes. It will only have value for debugging issues live. Enabling the log message will require a restart of the RM and the submission of new applications.

        By setting the NM cleanup delay, you can get more complete information from the AM launch script. Doing that requires a restart of the NM(s), unless the configured delay is left in place.

        I don't see that this log message provides any information that wouldn't be available through other means. The main difference is whether you're bouncing the RM or the NM to get the info.

        Show
        templedf Daniel Templeton added a comment - With the log message set to debug and guarded by a property, the message is not useful for historical purposes. It will only have value for debugging issues live. Enabling the log message will require a restart of the RM and the submission of new applications. By setting the NM cleanup delay, you can get more complete information from the AM launch script. Doing that requires a restart of the NM(s), unless the configured delay is left in place. I don't see that this log message provides any information that wouldn't be available through other means. The main difference is whether you're bouncing the RM or the NM to get the info.
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        Enabling the log message will require a restart of the RM and the submission of new applications.

        There is a logLevel servlet that lets admins change log-level settings (info / debug etc) for Hadoop daemons while they are running.

        Let's just leave it at DEBUG.

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - Enabling the log message will require a restart of the RM and the submission of new applications. There is a logLevel servlet that lets admins change log-level settings (info / debug etc) for Hadoop daemons while they are running. Let's just leave it at DEBUG.
        Hide
        jlowe Jason Lowe added a comment -

        Totally agree with Daniel that this is only useful for debugging a live issue. However there is a logLevel servlet on all the major daemons which can dynamically change the log level of user-specified loggers, so this can be debugged live without too much hassle on the RM side assuming the issue is repeatable and the user simply needs to re-run it after the log level has been updated.

        As for the other ways to debug it, I agree the NM provides the most details by far. However it can be a real pain on a sizeable cluster to update all NMs for this and it affects all containers on that node rather than the one we're interested in. And if we don't update all the nodes it's hard to know which NM will be used for the next test of the AM. That leaves the job.xml approach, but YARN doesn't require a job.xml for applications. So if we're debugging a custom YARN application that could be challenging if both the RM and NM aren't providing any details as to what command is being launched, the app framework isn't helping either, and the logs from the failed command also aren't sufficient to know what happened.

        I'd like to get rid of the log as long as there's a reasonable alternative to getting the information elsewhere when debugging weird launch failures. Maybe we need a more explicit way for users to debug their own app launch failures? That would be far preferable than having to involve a cluster admin every time. For example there could be a flag in the submission context that causes the NM to log the command line and/or startup script to a separate log file in the YARN log directory for the container. Obviously for another JIRA. Just wanted to point out that updating the log level for live debugging is not something that requires a restart.

        Show
        jlowe Jason Lowe added a comment - Totally agree with Daniel that this is only useful for debugging a live issue. However there is a logLevel servlet on all the major daemons which can dynamically change the log level of user-specified loggers, so this can be debugged live without too much hassle on the RM side assuming the issue is repeatable and the user simply needs to re-run it after the log level has been updated. As for the other ways to debug it, I agree the NM provides the most details by far. However it can be a real pain on a sizeable cluster to update all NMs for this and it affects all containers on that node rather than the one we're interested in. And if we don't update all the nodes it's hard to know which NM will be used for the next test of the AM. That leaves the job.xml approach, but YARN doesn't require a job.xml for applications. So if we're debugging a custom YARN application that could be challenging if both the RM and NM aren't providing any details as to what command is being launched, the app framework isn't helping either, and the logs from the failed command also aren't sufficient to know what happened. I'd like to get rid of the log as long as there's a reasonable alternative to getting the information elsewhere when debugging weird launch failures. Maybe we need a more explicit way for users to debug their own app launch failures? That would be far preferable than having to involve a cluster admin every time. For example there could be a flag in the submission context that causes the NM to log the command line and/or startup script to a separate log file in the YARN log directory for the container. Obviously for another JIRA. Just wanted to point out that updating the log level for live debugging is not something that requires a restart.
        Hide
        templedf Daniel Templeton added a comment -

        Sounds like we're agreed that we should go with patch 3.

        I'll file another JIRA for easier launch debugging. What if we add a launch context setting that has the same effect as setting the cleanup delay? That would give the maximum information with the minimal change.

        Show
        templedf Daniel Templeton added a comment - Sounds like we're agreed that we should go with patch 3. I'll file another JIRA for easier launch debugging. What if we add a launch context setting that has the same effect as setting the cleanup delay? That would give the maximum information with the minimal change.
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        I'd like to get rid of the log as long as there's a reasonable alternative to getting the information elsewhere when debugging weird launch failures. Maybe we need a more explicit way for users to debug their own app launch failures? That would be far preferable than having to involve a cluster admin every time.

        Jason Lowe / Daniel Templeton, I think the best place for this is Timeline Service V2 - for each app, RM / NMs can always dump the launch-commands into ATS V2 per container, with the ACLs set to the app-owner so that only he/she can view it. We can file such a JIRA under the V2 umbrella.

        Sounds like we're agreed that we should go with patch 3.

        Patch 3 minus the configuration, right?

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - I'd like to get rid of the log as long as there's a reasonable alternative to getting the information elsewhere when debugging weird launch failures. Maybe we need a more explicit way for users to debug their own app launch failures? That would be far preferable than having to involve a cluster admin every time. Jason Lowe / Daniel Templeton , I think the best place for this is Timeline Service V2 - for each app, RM / NMs can always dump the launch-commands into ATS V2 per container, with the ACLs set to the app-owner so that only he/she can view it. We can file such a JIRA under the V2 umbrella. Sounds like we're agreed that we should go with patch 3. Patch 3 minus the configuration, right?
        Hide
        jlowe Jason Lowe added a comment -

        Storing launch info in ATSv2 is fine with me and sounds preferable as long as it's easy for the user to get to it via the UI.

        Show
        jlowe Jason Lowe added a comment - Storing launch info in ATSv2 is fine with me and sounds preferable as long as it's easy for the user to get to it via the UI.
        Hide
        templedf Daniel Templeton added a comment -

        I would still argue to keep the config param. There are many reasons why one would want to enable debug logging, all of which can cause the leaking of credentials into the logs. The point of this JIRA is to secure the application command contents. Without the config param, we're only making it a bit more secure, sometimes.

        Show
        templedf Daniel Templeton added a comment - I would still argue to keep the config param. There are many reasons why one would want to enable debug logging, all of which can cause the leaking of credentials into the logs. The point of this JIRA is to secure the application command contents. Without the config param, we're only making it a bit more secure, sometimes.
        Hide
        templedf Daniel Templeton added a comment -

        I just created YARN-5599 and YARN-5600 to help with debugging launch failures in a post-YARN-5549 world. Vinod Kumar Vavilapalli, please correct the description of YARN-5599 to better reflect the reality of ATS if I'm off a bit.

        Show
        templedf Daniel Templeton added a comment - I just created YARN-5599 and YARN-5600 to help with debugging launch failures in a post- YARN-5549 world. Vinod Kumar Vavilapalli , please correct the description of YARN-5599 to better reflect the reality of ATS if I'm off a bit.
        Hide
        kasha Karthik Kambatla added a comment -

        I hate introducing one more config, but looks like it is required here. If the admin turns on debug logging to debug problems, user's credentials are exposed. As long as we are going to drop the config along with the log line in one of these follow-up JIRAs, I am fine with including the config for now. However, for security reasons, the default should probably be OFF. It would help to mention the config along with REDACTED.

        Show
        kasha Karthik Kambatla added a comment - I hate introducing one more config, but looks like it is required here. If the admin turns on debug logging to debug problems, user's credentials are exposed. As long as we are going to drop the config along with the log line in one of these follow-up JIRAs, I am fine with including the config for now. However, for security reasons, the default should probably be OFF. It would help to mention the config along with REDACTED.
        Hide
        templedf Daniel Templeton added a comment -

        OK. Here's a patch based on the previous patch 3 with Karthik Kambatla's feedback applied.

        Show
        templedf Daniel Templeton added a comment - OK. Here's a patch based on the previous patch 3 with Karthik Kambatla 's feedback applied.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 21s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        0 mvndep 0m 9s Maven dependency ordering for branch
        +1 mvninstall 6m 40s trunk passed
        +1 compile 2m 16s trunk passed
        +1 checkstyle 0m 40s trunk passed
        +1 mvnsite 1m 37s trunk passed
        +1 mvneclipse 0m 42s trunk passed
        +1 findbugs 3m 0s trunk passed
        +1 javadoc 1m 9s trunk passed
        0 mvndep 0m 9s Maven dependency ordering for patch
        +1 mvninstall 1m 21s the patch passed
        +1 compile 2m 31s the patch passed
        +1 javac 2m 31s the patch passed
        +1 checkstyle 0m 38s the patch passed
        +1 mvnsite 1m 31s the patch passed
        +1 mvneclipse 0m 36s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 xml 0m 1s The patch has no ill-formed XML file.
        +1 findbugs 3m 19s the patch passed
        +1 javadoc 1m 0s the patch passed
        +1 unit 0m 23s hadoop-yarn-api in the patch passed.
        +1 unit 2m 19s hadoop-yarn-common in the patch passed.
        +1 unit 34m 23s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        65m 55s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826515/YARN-5549.005.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 371b3d2e146b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 85bab5f
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12978/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/12978/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 40s trunk passed +1 compile 2m 16s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 37s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 3m 0s trunk passed +1 javadoc 1m 9s trunk passed 0 mvndep 0m 9s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 2m 31s the patch passed +1 javac 2m 31s the patch passed +1 checkstyle 0m 38s the patch passed +1 mvnsite 1m 31s the patch passed +1 mvneclipse 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 19s the patch passed +1 javadoc 1m 0s the patch passed +1 unit 0m 23s hadoop-yarn-api in the patch passed. +1 unit 2m 19s hadoop-yarn-common in the patch passed. +1 unit 34m 23s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 65m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826515/YARN-5549.005.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 371b3d2e146b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 85bab5f Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12978/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/12978/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        kasha Karthik Kambatla added a comment - - edited

        The patch looks good, except for the following nits:

        1. The static imports make sense, but the rest of the code doesn't do it. Can we leave them out? If we choose to keep it, when logging "REDACTED etc." should use the statically imported version.
        2. Some of the changes are unrelated to the patch. To minimize conflicts, I would like for us to leave these out.
        Show
        kasha Karthik Kambatla added a comment - - edited The patch looks good, except for the following nits: The static imports make sense, but the rest of the code doesn't do it. Can we leave them out? If we choose to keep it, when logging "REDACTED etc." should use the statically imported version. Some of the changes are unrelated to the patch. To minimize conflicts, I would like for us to leave these out.
        Hide
        kasha Karthik Kambatla added a comment -

        Vinod Kumar Vavilapalli, Jason Lowe, NGarla_Unused - are you okay with the approach in the last patch here? I would like to get this in soon.

        Show
        kasha Karthik Kambatla added a comment - Vinod Kumar Vavilapalli , Jason Lowe , NGarla_Unused - are you okay with the approach in the last patch here? I would like to get this in soon.
        Hide
        templedf Daniel Templeton added a comment -

        Here's a patch minus my spurious improvements and the static imports.

        Show
        templedf Daniel Templeton added a comment - Here's a patch minus my spurious improvements and the static imports.
        Hide
        jlowe Jason Lowe added a comment -

        +1 lgtm.

        Show
        jlowe Jason Lowe added a comment - +1 lgtm.
        Hide
        rchiang Ray Chiang added a comment -

        I like the actionable log message in v6 better too. +1

        Show
        rchiang Ray Chiang added a comment - I like the actionable log message in v6 better too. +1
        Hide
        kasha Karthik Kambatla added a comment -

        +1 pending Jenkins. Will commit this tomorrow morning (PT) if no one has any objections.

        Show
        kasha Karthik Kambatla added a comment - +1 pending Jenkins. Will commit this tomorrow morning (PT) if no one has any objections.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 13s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        0 mvndep 0m 9s Maven dependency ordering for branch
        +1 mvninstall 6m 45s trunk passed
        +1 compile 2m 21s trunk passed
        +1 checkstyle 0m 40s trunk passed
        +1 mvnsite 1m 35s trunk passed
        +1 mvneclipse 0m 41s trunk passed
        +1 findbugs 2m 52s trunk passed
        +1 javadoc 1m 3s trunk passed
        0 mvndep 0m 8s Maven dependency ordering for patch
        +1 mvninstall 1m 18s the patch passed
        +1 compile 2m 15s the patch passed
        +1 javac 2m 15s the patch passed
        +1 checkstyle 0m 38s the patch passed
        +1 mvnsite 1m 27s the patch passed
        +1 mvneclipse 0m 35s 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 9s the patch passed
        +1 javadoc 0m 58s the patch passed
        +1 unit 0m 23s hadoop-yarn-api in the patch passed.
        +1 unit 2m 16s hadoop-yarn-common in the patch passed.
        +1 unit 33m 59s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        64m 39s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826691/YARN-5549.006.patch
        JIRA Issue YARN-5549
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
        uname Linux 807283a6d847 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 3069df7
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12996/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/12996/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. 0 mvndep 0m 9s Maven dependency ordering for branch +1 mvninstall 6m 45s trunk passed +1 compile 2m 21s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 35s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 2m 52s trunk passed +1 javadoc 1m 3s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 18s the patch passed +1 compile 2m 15s the patch passed +1 javac 2m 15s the patch passed +1 checkstyle 0m 38s the patch passed +1 mvnsite 1m 27s the patch passed +1 mvneclipse 0m 35s 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 9s the patch passed +1 javadoc 0m 58s the patch passed +1 unit 0m 23s hadoop-yarn-api in the patch passed. +1 unit 2m 16s hadoop-yarn-common in the patch passed. +1 unit 33m 59s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 64m 39s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826691/YARN-5549.006.patch JIRA Issue YARN-5549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 807283a6d847 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 3069df7 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/12996/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/12996/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        Naganarasimha Naganarasimha G R added a comment -

        Thanks for the patch Daniel Templeton and Karthik Kambatla for the comments for the config.
        Given that there is already jira raised to ATSV2 to log the command which can be securedly access later makes sense, but i am +0 for this patch because to get this log it requires a restart of the NM to enable the modified config and as Jason Lowe mentioned we will not be sure where the container will be relaunched again in this node on failure. So temporarily if the node has issues this log and config will be helpfull but in case app's command has issues this approach offers no help.

        Other than this patch looks fine !

        Show
        Naganarasimha Naganarasimha G R added a comment - Thanks for the patch Daniel Templeton and Karthik Kambatla for the comments for the config. Given that there is already jira raised to ATSV2 to log the command which can be securedly access later makes sense, but i am +0 for this patch because to get this log it requires a restart of the NM to enable the modified config and as Jason Lowe mentioned we will not be sure where the container will be relaunched again in this node on failure. So temporarily if the node has issues this log and config will be helpfull but in case app's command has issues this approach offers no help. Other than this patch looks fine !
        Hide
        templedf Daniel Templeton added a comment -

        Minor correction, Naganarasimha G R: it requires a restart of the RM, not the NM, which is a lot easier to stomach.

        Show
        templedf Daniel Templeton added a comment - Minor correction, Naganarasimha G R : it requires a restart of the RM, not the NM, which is a lot easier to stomach.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10392 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10392/)
        YARN-5549. AMLauncher#createAMContainerLaunchContext() should not log (rchiang: rev 378f624a392550770d1db33cb4cee3ef7d5facd4)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/amlauncher/AMLauncher.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10392 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10392/ ) YARN-5549 . AMLauncher#createAMContainerLaunchContext() should not log (rchiang: rev 378f624a392550770d1db33cb4cee3ef7d5facd4) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/amlauncher/AMLauncher.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml
        Hide
        rchiang Ray Chiang added a comment -

        Committed to trunk. Thanks Daniel Templeton for the contribution! Thanks Naganarasimha G R, Vinod Kumar Vavilapalli, Jason Lowe, and Karthik Kambatla for your reviews and feedback!

        FYI, I've held off from putting this in branch-2 for now. It might be preferable to wait until YARN-5599 and/or YARN-5600 are done.

        Show
        rchiang Ray Chiang added a comment - Committed to trunk. Thanks Daniel Templeton for the contribution! Thanks Naganarasimha G R , Vinod Kumar Vavilapalli , Jason Lowe , and Karthik Kambatla for your reviews and feedback! FYI, I've held off from putting this in branch-2 for now. It might be preferable to wait until YARN-5599 and/or YARN-5600 are done.
        Hide
        kasha Karthik Kambatla added a comment -

        I actually think we should push this into the next release (2.8) as well to avoid exposing user credentials.

        Show
        kasha Karthik Kambatla added a comment - I actually think we should push this into the next release (2.8) as well to avoid exposing user credentials.
        Hide
        rchiang Ray Chiang added a comment - - edited

        Re-opening until I push through to branch-2.8. Daniel Templeton, please provide a branch-2/branch-2.8 patch when you can. Thanks.

        Show
        rchiang Ray Chiang added a comment - - edited Re-opening until I push through to branch-2.8. Daniel Templeton , please provide a branch-2/branch-2.8 patch when you can. Thanks.
        Hide
        rchiang Ray Chiang added a comment -

        Pushed to branch-2 and branch-2.8.

        Show
        rchiang Ray Chiang added a comment - Pushed to branch-2 and branch-2.8.
        Hide
        andrew.wang Andrew Wang added a comment -

        As a reminder, please set a 3.x fix version when committing too. Thanks!

        Show
        andrew.wang Andrew Wang added a comment - As a reminder, please set a 3.x fix version when committing too. Thanks!

          People

          • Assignee:
            templedf Daniel Templeton
            Reporter:
            templedf Daniel Templeton
          • Votes:
            0 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development