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

Context.setStatus() and progress() api are ignored

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.1, 0.22.0
    • Component/s: task
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change
    • Release Note:
      Moved the api public Counter getCounter(Enum<?> counterName), public Counter getCounter(String groupName, String counterName) from org.apache.hadoop.mapreduce.TaskInputOutputContext to org.apache.hadoop.mapreduce.TaskAttemptContext

      Description

      TaskAttemptContext.setStatus() and progress() were overriden in TaskInputOutputContext, inbranch 0.20, to call the underlying reporter apis. But the methods are no more over-riden in TaskInputOutputContextImpl after MAPREDUCE-954.

      1. patch-1905-1.txt
        19 kB
        Amareshwari Sriramadasu
      2. patch-1905.txt
        3 kB
        Amareshwari Sriramadasu

        Issue Links

          Activity

          Hide
          Amareshwari Sriramadasu added a comment -

          Patch with the fix. Also includes a regression test.

          Show
          Amareshwari Sriramadasu added a comment - Patch with the fix. Also includes a regression test.
          Hide
          Hadoop QA added a comment -

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

          Show
          Hadoop QA added a comment - -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.
          Hide
          Amareshwari Sriramadasu added a comment -

          Hudson failed running all the tests. Re-submitting.

          Show
          Amareshwari Sriramadasu added a comment - Hudson failed running all the tests. Re-submitting.
          Hide
          Hadoop QA added a comment -

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

          Show
          Hadoop QA added a comment - -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.
          Hide
          Hadoop QA added a comment -

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

          Show
          Hadoop QA added a comment - -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.
          Hide
          Amareshwari Sriramadasu added a comment -

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

          Show
          Amareshwari Sriramadasu added a comment - 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).
          Hide
          Vinod Kumar Vavilapalli added a comment -

          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.

          Show
          Vinod Kumar Vavilapalli added a comment - 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.
          Hide
          Amareshwari Sriramadasu added a comment -

          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,

          {Input/Output}

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

          Owen?

          Show
          Amareshwari Sriramadasu added a comment - 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, {Input/Output} Format cannot increment counters. Though there is a hack used in LineRecordReader : type cast TaskAttemptContext to MapContext for accessing counter, which is ugly. Owen?
          Hide
          Tom White added a comment -

          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

          Show
          Tom White added a comment - 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
          Hide
          Amareshwari Sriramadasu added a comment -

          Thanks Tom.
          Will upload a patch with the changes.

          Show
          Amareshwari Sriramadasu added a comment - Thanks Tom. Will upload a patch with the changes.
          Hide
          Amareshwari Sriramadasu added a comment -

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

          Show
          Amareshwari Sriramadasu added a comment - Patch moving the reporter into TaskAttemptContextImpl and moving getCounter methods from TaskInputOutputContext to TaskAttemptContext.
          Hide
          Amareshwari Sriramadasu added a comment -

          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.

          Show
          Amareshwari Sriramadasu added a comment - 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.
          Hide
          Tom White added a comment -

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

          Show
          Tom White added a comment - +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.
          Hide
          Amareshwari Sriramadasu added a comment -

          Thanks Tom for the review.

          I just committed this to trunk and branch 0.21.

          Show
          Amareshwari Sriramadasu added a comment - Thanks Tom for the review. I just committed this to trunk and branch 0.21.

            People

            • Assignee:
              Amareshwari Sriramadasu
              Reporter:
              Amareshwari Sriramadasu
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development