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.

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

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

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

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

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

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

I committed this. Thanks, Dick!

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

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