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

rumen makes a task with a null type when one of the task lines is truncated

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: tools/rumen
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Rumen was used to produce a job trace, but the job trace contained a LoggedTask that had a null taskType. This appears to happen when a Task line is truncated.

      We should not put the LoggedTask in the trace at all when this happens.

        Activity

        Hide
        Dick King added a comment -

        This patch caluses a task to not appear in a job trace if we don't know what its type is.

        Also, under the old code, if the task creation line is present but the finish line is truncated, the taskType becomes null. This will cause the task to be marked with its type if that happens.

        Show
        Dick King added a comment - This patch caluses a task to not appear in a job trace if we don't know what its type is. Also, under the old code, if the task creation line is present but the finish line is truncated, the taskType becomes null. This will cause the task to be marked with its type if that happens.
        Hide
        Hong Tang added a comment -
        -      task.setTaskType(typ);
        +      if (!taskAlreadyLogged || typ != null) {
        +        task.setTaskType(typ);
        +      }
        

        Shouldn't it be just:

        -      task.setTaskType(typ);
        +      if (typ != null) {
        +        task.setTaskType(typ);
        +      }
        

        Otherwise, you would still the task type if taskAlreadyLogged==true but typ is null, and then later be ignored.

        Also -1 on modifying existing tests to test bug fixes. It'd be better to have a standalone negative test case to test this logic instead of piggybacking the testing with another test.

        Show
        Hong Tang added a comment - - task.setTaskType(typ); + if (!taskAlreadyLogged || typ != null) { + task.setTaskType(typ); + } Shouldn't it be just: - task.setTaskType(typ); + if (typ != null) { + task.setTaskType(typ); + } Otherwise, you would still the task type if taskAlreadyLogged==true but typ is null, and then later be ignored. Also -1 on modifying existing tests to test bug fixes. It'd be better to have a standalone negative test case to test this logic instead of piggybacking the testing with another test.
        Hide
        Dick King added a comment -

        This is a revised version of my previous patch. I made the following changes:

        1: I looked at the code change suggested in the previous comment and the code context, and made a different change.

        2: I did revise the unit test to make a separate test for proper functioning with a job tracker log that has truncated task lines.

        -dk

        Show
        Dick King added a comment - This is a revised version of my previous patch. I made the following changes: 1: I looked at the code change suggested in the previous comment and the code context, and made a different change. 2: I did revise the unit test to make a separate test for proper functioning with a job tracker log that has truncated task lines. -dk
        Hide
        Hong Tang added a comment -

        Patch includes quite a few unnecessary white space or format changes. I removed them and uploaded the new patch.

        Show
        Hong Tang added a comment - Patch includes quite a few unnecessary white space or format changes. I removed them and uploaded the new patch.
        Hide
        Dick King added a comment -

        The patch appears to make lots of changes to HadoopLogsAnalyzer but it doesn't. The only hunk that represents a code change is at line 1046.

        The patch will fail the audit because there are three new files that don't have an apache license banner. These three files cannot contain a license banner because they are an input and two reference outputs for a unit case.

        -dk

        Show
        Dick King added a comment - The patch appears to make lots of changes to HadoopLogsAnalyzer but it doesn't. The only hunk that represents a code change is at line 1046. The patch will fail the audit because there are three new files that don't have an apache license banner. These three files cannot contain a license banner because they are an input and two reference outputs for a unit case. -dk
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12419792/MAPREDUCE-986--2009-09-16--1015.patch
        against trunk revision 815628.

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

        +1 tests included. The patch appears to include 12 new or modified tests.

        +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 warnings.

        -1 release audit. The applied patch generated 223 release audit warnings (more than the trunk's current 220 warnings).

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/testReport/
        Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/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/12419792/MAPREDUCE-986--2009-09-16--1015.patch against trunk revision 815628. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified tests. +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 warnings. -1 release audit. The applied patch generated 223 release audit warnings (more than the trunk's current 220 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/38/console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12419797/mapredoce-986-removed-white-space.patch
        against trunk revision 815628.

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

        +1 tests included. The patch appears to include 9 new or modified tests.

        +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 warnings.

        -1 release audit. The applied patch generated 223 release audit warnings (more than the trunk's current 220 warnings).

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/testReport/
        Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/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/12419797/mapredoce-986-removed-white-space.patch against trunk revision 815628. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +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 warnings. -1 release audit. The applied patch generated 223 release audit warnings (more than the trunk's current 220 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/91/console This message is automatically generated.
        Hide
        Chris Douglas added a comment -

        I committed this. Thanks, Dick!

        Show
        Chris Douglas added a comment - I committed this. Thanks, Dick!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #74 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/74/)
        . Fix Rumen to work with truncated task lines. Contributed by Dick King

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #74 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/74/ ) . Fix Rumen to work with truncated task lines. Contributed by Dick King
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #112 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/112/)
        . Fix Rumen to work with truncated task lines. Contributed by Dick King

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #112 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/112/ ) . Fix Rumen to work with truncated task lines. Contributed by Dick King

          People

          • Assignee:
            Dick King
            Reporter:
            Dick King
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development