Patch with the fix. Also includes a regression test.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12448480/patch-1905.txt
against trunk revision 959806.

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

+1 tests included. The patch appears to include 3 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 does not increase the total number of release audit warnings.

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

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/280/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/280/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/280/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/280/console

This message is automatically generated.

Hudson failed running all the tests. Re-submitting.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12448480/patch-1905.txt
against trunk revision 959867.

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

+1 tests included. The patch appears to include 3 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 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.

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/282/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/282/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/282/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/282/console

This message is automatically generated.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12448480/patch-1905.txt
against trunk revision 959867.

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

+1 tests included. The patch appears to include 3 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 does not increase the total number of release audit warnings.

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

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

Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/283/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/283/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/283/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/283/console

This message is automatically generated.

Build on 282 says test failure is TestSimulatorDeterministicReplay which, due to MAPREDUCE-1834. Don't know why second build got triggered, many tests failed in that build because of NoClassDefFoundError (not related).

Functionality wise, the patch fixes the current bug and looks fine.

But I sense something wrong. The new TaskAttemptContextImpl doesn't know about reporter even though the old org.apache.hadoop.mapred.TaskAttemptContextImpl knows about it.

• progress() calls are made throughout, but they may be getting reported because either the calls are made on the the old context object (in Task) or directly on the old Reporter class (in Shuffle etc). So, no bug here as of now, but once we try switching to new API context objects in the implementation of Task etc, we may encounter new bugs.
• setStatus() on the new context object are made in several RecordReaders - from the code, it seems these status updates will not get reported because of the lack of reporter.
• New Mapper/Reducer classes also make setStatus() calls but on TaskInputOutputContextImpl which, though knows about the reporter, doesn't override the method from TaskAttemptContextImpl. This will be fixed by the current patch.

May be, then, the correct and complete solution is to let TaskAttemptContextImpl know about reporter (via constructor), do setStatus() and progress() in TaskAttemptContextImpl itself and not override them in TaskInputOutputContextImpl? This will be slightly more changes, so we can confirm from someone who knows this area well.

I agree with Vinod on the above comment that TaskAttemptContextImpl should know about reporter.

Also, reporter methods in TaskInputOutputContext i.e getCounter() methods should be moved to TaskAttemptContext interface. Currently,

Format cannot increment counters. Though there is a hack used in LineRecordReader : type cast TaskAttemptContext to MapContext for accessing counter, which is ugly.

Owen?

This patch certainly solves the problem but I agree with Vinod and Amareshwari that we should fix this properly by moving the reporter into TaskAttemptContextImpl.

Also, reporter methods in TaskInputOutputContext i.e getCounter() methods should be moved to TaskAttemptContext interface.

+1

Thanks Tom.
Will upload a patch with the changes.

Patch moving the reporter into TaskAttemptContextImpl and moving getCounter methods from TaskInputOutputContext to TaskAttemptContext.

Ran test-patch and ant test on both trunk and branch 0.21.
On branch 0.21, TestDelegationTokenRenewal and TestControlledMapReduceJob failed due to MAPREDUCE-1835 and MAPREDUCE-2121. All other tests passed.
The test failures are in trunk are all existing failures.

+1

This changes the TaskInputOutputContext and TaskAttemptContext interfaces, but in a way that doesn't affect end user applications. Also, the interfaces are marked Evolving, so this change is permitted. We should add a release note though.

Thanks Tom for the review.

I just committed this to trunk and branch 0.21.