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

TaskRunner.prepare() and close() can be removed

    Details

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

      Description

      TaskRunner.prepare() and close() methods call only mapOutputFile.removeAll(). The removeAll() call is a always a no-op in prepare(), because the directory is always empty during start up of the task. The removeAll() call in close() is useless, because it is followed by a attempt directory cleanup. Since the map output files are in attempt directory, the call to close() is useless.
      After MAPREDUCE-842, these calls are under TaskTracker space, passing the wrong conf. Now, the calls do not make sense at all.
      I think we can remove the methods.

      1. patch-1662.txt
        4 kB
        Amareshwari Sriramadasu

        Issue Links

          Activity

          Amareshwari Sriramadasu created issue -
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch removes TaskRunner.prepare().
          ReduceTaskRunner.close() sets a status "closed" on the Progress object. So, patch does not remove TaskRunner.close(), but removes mapOutputFile.removeAll() from close() methods in MapTaskRunner and ReduceTaskRunner.

          Show
          Amareshwari Sriramadasu added a comment - Patch removes TaskRunner.prepare(). ReduceTaskRunner.close() sets a status "closed" on the Progress object. So, patch does not remove TaskRunner.close(), but removes mapOutputFile.removeAll() from close() methods in MapTaskRunner and ReduceTaskRunner.
          Amareshwari Sriramadasu made changes -
          Field Original Value New Value
          Attachment patch-1662.txt [ 12440478 ]
          Amareshwari Sriramadasu made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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 passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/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/12440478/patch-1662.txt against trunk revision 929712. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/87/console This message is automatically generated.
          Vinod Kumar Vavilapalli made changes -
          Link This issue is related to MAPREDUCE-1635 [ MAPREDUCE-1635 ]
          Hide
          Vinod Kumar Vavilapalli added a comment -

          We found this while working on MAPREDUCE-1635.

          Show
          Vinod Kumar Vavilapalli added a comment - We found this while working on MAPREDUCE-1635 .
          Hide
          Amareshwari Sriramadasu added a comment -

          -1 tests included.

          No new tests added because patch removes dead code.

          -1 core tests.

          The test failure TestMapReduceLazyOutput.testLazyOutput is not related to the patch. The test failed because of ZipExceptions (MAPREDUCE-1642).
          The same test passed on my machine.

          Show
          Amareshwari Sriramadasu added a comment - -1 tests included. No new tests added because patch removes dead code. -1 core tests. The test failure TestMapReduceLazyOutput.testLazyOutput is not related to the patch. The test failed because of ZipExceptions ( MAPREDUCE-1642 ). The same test passed on my machine.
          Hide
          Chris Douglas added a comment -

          +1

          I committed this. Thanks, Amareshwari!

          Show
          Chris Douglas added a comment - +1 I committed this. Thanks, Amareshwari!
          Chris Douglas made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Assignee Amareshwari Sriramadasu [ amareshwari ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #324 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/324/)
          MAPREDUCE-1662. Remove unused methods from TaskRunner. Contributed by Amareshwari Sriramadasu

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #324 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/324/ ) MAPREDUCE-1662 . Remove unused methods from TaskRunner. Contributed by Amareshwari Sriramadasu
          Liyin Liang made changes -
          Link This issue is related to MAPREDUCE-1247 [ MAPREDUCE-1247 ]
          Konstantin Shvachko made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development