Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-2372

TaskLogAppender mechanism shouldn't be set in log4j.properties

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: task
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The TaskLogAppender log4j appender relies on using log4j.properties to pass in some Java system properties into properties of the logger. This is problematic since we've often found that users have customized log4j.properties and don't upgrade it when they upgrade the version of Hadoop.

      Since this is really an internal mechanism of how the task runner passes task info to the TLA, we shouldn't rely on these settings in log4j.properties at all. Rather, we should just get the system properties directly from System.getProperty.

      1. mapreduce-2372.txt
        3 kB
        Todd Lipcon
      2. mapreduce-2372.txt
        4 kB
        Todd Lipcon
      3. mapreduce-2372.txt
        4 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #685 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk/685/)
          MAPREDUCE-2372. TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125082
          Files :

          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskLogAppender.java
          • /hadoop/mapreduce/trunk/CHANGES.txt
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #685 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk/685/ ) MAPREDUCE-2372 . TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125082 Files : /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskLogAppender.java /hadoop/mapreduce/trunk/CHANGES.txt /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #56 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/56/)
          MAPREDUCE-2372. TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125081
          Files :

          • /hadoop/mapreduce/branches/branch-0.22/CHANGES.txt
          • /hadoop/mapreduce/branches/branch-0.22/src/java/org/apache/hadoop/mapred/TaskRunner.java
          • /hadoop/mapreduce/branches/branch-0.22/src/java/org/apache/hadoop/mapred/TaskLogAppender.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #56 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/56/ ) MAPREDUCE-2372 . TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125081 Files : /hadoop/mapreduce/branches/branch-0.22/CHANGES.txt /hadoop/mapreduce/branches/branch-0.22/src/java/org/apache/hadoop/mapred/TaskRunner.java /hadoop/mapreduce/branches/branch-0.22/src/java/org/apache/hadoop/mapred/TaskLogAppender.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #686 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/686/)
          MAPREDUCE-2372. TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125082
          Files :

          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskLogAppender.java
          • /hadoop/mapreduce/trunk/CHANGES.txt
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #686 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/686/ ) MAPREDUCE-2372 . TaskLogAppender mechanism shouldn't be set up in log4j.properties. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125082 Files : /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskLogAppender.java /hadoop/mapreduce/trunk/CHANGES.txt /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java
          Hide
          Todd Lipcon added a comment -

          Committed to trunk and branch 22. Thanks for reviewing, Tom.

          Show
          Todd Lipcon added a comment - Committed to trunk and branch 22. Thanks for reviewing, Tom.
          Hide
          Tom White added a comment -

          +1 to keeping the current default in TaskLog.getTaskLogLength().

          Show
          Tom White added a comment - +1 to keeping the current default in TaskLog.getTaskLogLength().
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12479511/mapreduce-2372.txt
          against trunk revision 1103993.

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

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

          +1 core tests. The patch passed core unit tests.

          -1 contrib tests. The patch failed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12479511/mapreduce-2372.txt against trunk revision 1103993. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/258//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          patch which switches to default 0

          Show
          Todd Lipcon added a comment - patch which switches to default 0
          Hide
          Todd Lipcon added a comment -

          was just looking this over before commit and decided that the default of "100" for the logsize property isn't for any particularly good reason. Instead it should probably be "0", which disables the queueing/truncation ability and is the default used by TaskLog.getTaskLogLength

          Show
          Todd Lipcon added a comment - was just looking this over before commit and decided that the default of "100" for the logsize property isn't for any particularly good reason. Instead it should probably be "0", which disables the queueing/truncation ability and is the default used by TaskLog.getTaskLogLength
          Hide
          Tom White added a comment -

          +1 This looks like a good change to me.

          Show
          Tom White added a comment - +1 This looks like a good change to me.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12473365/mapreduce-2372.txt
          against trunk revision 1079072.

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

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

          +1 core tests. The patch passed core unit tests.

          -1 contrib tests. The patch failed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12473365/mapreduce-2372.txt against trunk revision 1079072. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/136//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          Fix findbugs warning by synchronizing access to all the member variables of TLA

          Show
          Todd Lipcon added a comment - Fix findbugs warning by synchronizing access to all the member variables of TLA
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12473212/mapreduce-2372.txt
          against trunk revision 1079072.

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

          -1 tests included. 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 javadoc. The javadoc tool appears to have generated 1 warning messages.

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

          -1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.

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

          +1 core tests. The patch passed core unit tests.

          -1 contrib tests. The patch failed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12473212/mapreduce-2372.txt against trunk revision 1079072. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/134//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          Here's a patch which allows the settings to come directly from system properties.

          Unfortunately, TLA isn't covered by any test cases yet, so nothing to change there.

          We will need a patch on Common to remove the settings from log4j.properties as well.

          Show
          Todd Lipcon added a comment - Here's a patch which allows the settings to come directly from system properties. Unfortunately, TLA isn't covered by any test cases yet, so nothing to change there. We will need a patch on Common to remove the settings from log4j.properties as well.

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development