Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.1
    • Component/s: tasktracker
    • Labels:
      None

      Description

      MAPREDUCE-4970 introduced incompatible change and causes syslog to be missing from tasktracker on old clusters which just have log4j.properties configured

      1. MAPREDUCE-5148.patch
        0.5 kB
        Arun C Murthy
      2. Screenshot_MAPREDUCE-5148.png
        72 kB
        Yesha Vora

        Activity

        Hide
        Yesha Vora added a comment -

        Screen Shot

        Show
        Yesha Vora added a comment - Screen Shot
        Hide
        Arun C Murthy added a comment -

        Looks like the newly introduced task-log4j.properties is missing.

        It's unfortunate, but I think we should just document MAPREDUCE-4970 and be ok with it.

        Alejandro Abdelnur - Are you fine with this?

        Show
        Arun C Murthy added a comment - Looks like the newly introduced task-log4j.properties is missing. It's unfortunate, but I think we should just document MAPREDUCE-4970 and be ok with it. Alejandro Abdelnur - Are you fine with this?
        Hide
        Alejandro Abdelnur added a comment -

        I agree with Arun. Plus, given than the message in the logs is clear folks will have a clue what to fix.

        Show
        Alejandro Abdelnur added a comment - I agree with Arun. Plus, given than the message in the logs is clear folks will have a clue what to fix.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        We should definitely mark MAPREDUCE-4970 as an incompatible change.

        Also, I think, we can maintain the compatability with little effort - tasks should read both log4j.properties and task-log4j.properties. That should do, right?

        Show
        Vinod Kumar Vavilapalli added a comment - We should definitely mark MAPREDUCE-4970 as an incompatible change. Also, I think, we can maintain the compatability with little effort - tasks should read both log4j.properties and task-log4j.properties. That should do, right?
        Hide
        Sandy Ryza added a comment -

        I ran a tar build and the task-log4j.properties showed up (and syslog appeared correctly). Is there a packaging/upgrade process that updates code but not config files?

        Show
        Sandy Ryza added a comment - I ran a tar build and the task-log4j.properties showed up (and syslog appeared correctly). Is there a packaging/upgrade process that updates code but not config files?
        Hide
        Arun C Murthy added a comment -

        Yesha Vora finally helped me understand the issue. The problem is that if there is an existing install, and people just upgrade the hadoop-core jar it breaks them and is, as such, a problem.

        Maybe a simple fix is to include task-log4j.properties in the hadoop-core jar?

        Something like:

        diff --git build.xml build.xml
        index a87513c..d5a3859 100644
        --- build.xml
        +++ build.xml
        @@ -762,7 +762,7 @@
               <tarfileset dir="bin" mode="755"/>
             </tar>
             <property name="jar.properties.list"
        -      value="commons-logging.properties, log4j.properties, hadoop-metrics.properties"/>
        +      value="commons-logging.properties, log4j.properties, hadoop-metrics.properties, task-log4j.properties"/>
             <jar jarfile="${build.dir}/${core.final.name}.jar"
                  basedir="${build.classes}">
               <manifest>
        
        Show
        Arun C Murthy added a comment - Yesha Vora finally helped me understand the issue. The problem is that if there is an existing install, and people just upgrade the hadoop-core jar it breaks them and is, as such, a problem. Maybe a simple fix is to include task-log4j.properties in the hadoop-core jar? Something like: diff --git build.xml build.xml index a87513c..d5a3859 100644 --- build.xml +++ build.xml @@ -762,7 +762,7 @@ <tarfileset dir="bin" mode="755"/> </tar> <property name="jar.properties.list" - value="commons-logging.properties, log4j.properties, hadoop-metrics.properties"/> + value="commons-logging.properties, log4j.properties, hadoop-metrics.properties, task-log4j.properties"/> <jar jarfile="${build.dir}/${core.final.name}.jar" basedir="${build.classes}"> <manifest>
        Hide
        Arun C Murthy added a comment -

        Uploading a candidate patch for discussion.

        Show
        Arun C Murthy added a comment - Uploading a candidate patch for discussion.
        Hide
        Arun C Murthy added a comment -

        Thanks Yesha Vora for being persistent and patient!

        Show
        Arun C Murthy added a comment - Thanks Yesha Vora for being persistent and patient!
        Hide
        Arun C Murthy added a comment -

        Sigh, nevermind that patch doesn't work.

        Show
        Arun C Murthy added a comment - Sigh, nevermind that patch doesn't work.
        Hide
        Arun C Murthy added a comment -

        Spoke too soon, the patch works as expected. I was confused running from a non-tarball i.e. dev-mode build which actually didn't get hadoop-core*.jar on classpath of the MR child task.

        I think the patch is good to go.

        Thoughts?

        Show
        Arun C Murthy added a comment - Spoke too soon, the patch works as expected. I was confused running from a non-tarball i.e. dev-mode build which actually didn't get hadoop-core*.jar on classpath of the MR child task. I think the patch is good to go. Thoughts?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12582268/MAPREDUCE-5148.patch
        against trunk revision .

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

        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3588//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/12582268/MAPREDUCE-5148.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3588//console This message is automatically generated.
        Hide
        Arun C Murthy added a comment -

        Btw, for future reference - we need to ensure that the code has right defaults for all configs i.e. in this case it would be required to ensure TaskTracker sets correct defaults for all properties in task-log4j.properties.

        Furthermore, task-log4j.properties has a bunch of stuff which shouldn't have been blindly copied from the main log4j.properties such as definitions for jobsummary log etc.

        We should minimize contents of that file and, as above, ensure that all properties defined in task-log4j.properties are correctly represented in code too.

        Show
        Arun C Murthy added a comment - Btw, for future reference - we need to ensure that the code has right defaults for all configs i.e. in this case it would be required to ensure TaskTracker sets correct defaults for all properties in task-log4j.properties. Furthermore, task-log4j.properties has a bunch of stuff which shouldn't have been blindly copied from the main log4j.properties such as definitions for jobsummary log etc. We should minimize contents of that file and, as above, ensure that all properties defined in task-log4j.properties are correctly represented in code too.
        Hide
        Giridharan Kesavan added a comment -

        +1 patch looks good. Thanks Arun

        Show
        Giridharan Kesavan added a comment - +1 patch looks good. Thanks Arun
        Hide
        Arun C Murthy added a comment -

        Thanks for the review Giri. I just committed this.

        Show
        Arun C Murthy added a comment - Thanks for the review Giri. I just committed this.
        Hide
        Yesha Vora added a comment -

        Thanks Arun, I verified the fix. It looks good.

        Show
        Yesha Vora added a comment - Thanks Arun, I verified the fix. It looks good.

          People

          • Assignee:
            Arun C Murthy
            Reporter:
            Yesha Vora
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development