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

Hadoop does not close output file / does not call Mapper.cleanup if exception in map

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1-win, 2.0.3-alpha, 1.1.2
    • Fix Version/s: 1.2.0, 1-win, 2.1.0-beta
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour.
      Show
      Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour.
    • Target Version/s:

      Description

      Find this in Pig unit test TestStore under Windows. There are dangling files because map does not close the file when exception happens in map(). In Windows, Hadoop will not remove a file if it is not closed. This happens in reduce() as well.

      1. HADOOP-8904-1.patch
        4 kB
        Daniel Dai
      2. HADOOP-23-2.patch
        4 kB
        Daniel Dai
      3. MAPREDUCE-4737.patch
        8 kB
        Arun C Murthy
      4. MAPREDUCE-4737_branch1.patch
        17 kB
        Arun C Murthy
      5. MAPREDUCE-4737.patch
        19 kB
        Arun C Murthy
      6. MAPREDUCE-4737_branch1.patch
        18 kB
        Arun C Murthy
      7. MAPREDUCE-4737.patch
        19 kB
        Arun C Murthy

        Activity

        Matt Foley made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Matt Foley added a comment -

        Closed upon release of Hadoop 1.2.0.

        Show
        Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1410 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1410/)
        MAPREDUCE-4737. Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556)

        Result = SUCCESS
        acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1410 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1410/ ) MAPREDUCE-4737 . Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1383 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1383/)
        MAPREDUCE-4737. Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556)

        Result = FAILURE
        acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1383 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1383/ ) MAPREDUCE-4737 . Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556) Result = FAILURE acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Yarn-trunk #194 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/194/)
        MAPREDUCE-4737. Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556)

        Result = SUCCESS
        acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Show
        Hudson added a comment - Integrated in Hadoop-Yarn-trunk #194 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/194/ ) MAPREDUCE-4737 . Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-trunk-Commit #3655 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3655/)
        MAPREDUCE-4737. Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556)

        Result = SUCCESS
        acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Show
        Hudson added a comment - Integrated in Hadoop-trunk-Commit #3655 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3655/ ) MAPREDUCE-4737 . Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour. Contributed by Arun C. Murthy. (Revision 1471556) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1471556 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/MapTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Mapper.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Reducer.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestMapperReducerCleanup.java
        Arun C Murthy made changes -
        Release Note Ensure that mapreduce APIs are semantically consistent with mapred API w.r.t Mapper.cleanup and Reducer.cleanup; in the sense that cleanup is now called even if there is an error. The old mapred API already ensures that Mapper.close and Reducer.close are invoked during error handling. Note that it is an incompatible change, however end-users can override Mapper.run and Reducer.run to get the old (inconsistent) behaviour.
        Fix Version/s 1.2.0 [ 12321661 ]
        Fix Version/s 1-win [ 12321744 ]
        Fix Version/s 2.0.5-beta [ 12324032 ]
        Resolution Fixed [ 1 ]
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Incompatible change,Reviewed [ 10342, 10343 ]
        Hide
        Arun C Murthy added a comment -

        I just committed this. Thanks Chris Nauroth & Tom White for the reviews!

        Show
        Arun C Murthy added a comment - I just committed this. Thanks Chris Nauroth & Tom White for the reviews!
        Hide
        Arun C Murthy added a comment -

        FYI - I've tested to ensure that the new unit test fails on without the changes to the core.

        Show
        Arun C Murthy added a comment - FYI - I've tested to ensure that the new unit test fails on without the changes to the core.
        Hide
        Arun C Murthy added a comment -

        Thanks for the review Tom.

        Arun, can you mark this as an incompatible change and add some documentation to the release notes field.

        Of course.

        Show
        Arun C Murthy added a comment - Thanks for the review Tom. Arun, can you mark this as an incompatible change and add some documentation to the release notes field. Of course.
        Hide
        Tom White added a comment -

        This looks good to me. Arun, can you mark this as an incompatible change and add some documentation to the release notes field. The semantics of cleanup in the new MR API are being changed to be consistent with the old API, but users may retain the existing semantics by overriding the run() method in the Mapper or Reducer.

        Show
        Tom White added a comment - This looks good to me. Arun, can you mark this as an incompatible change and add some documentation to the release notes field. The semantics of cleanup in the new MR API are being changed to be consistent with the old API, but users may retain the existing semantics by overriding the run() method in the Mapper or Reducer.
        Hide
        Chris Nauroth added a comment -

        +1 for the patch. Thanks for the explanation, Arun. It makes sense.

        Daniel Dai, would you want to try the new patch with Pig, since the original intent was to fix a unit test over there?

        Show
        Chris Nauroth added a comment - +1 for the patch. Thanks for the explanation, Arun. It makes sense. Daniel Dai , would you want to try the new patch with Pig, since the original intent was to fix a unit test over there?
        Hide
        Arun C Murthy added a comment -

        Thanks for reviewing it Chris.

        Is it necessary to close in the try block, considering that close is guaranteed to happen again in the finally block? Also, why bother to set local variables in and collector to null? Attempting to encourage garbage collection to clean them up faster? Something else?

        More for correctness.

        The issue is that I don't want to ignore exceptions during in.close() in normal sequence of action. However, if an exception occurred in, say, in.close() in try block (i.e. then we fall into the finally block), then we should ignore exceptions during collector.close(); hence closeQuietly(collector).

        Also, closeQuietly is coded to ensure we don't call in.close or collector.close in both try and finally block, hence the games with setting them to null in try block.

        Makes sense?

        Show
        Arun C Murthy added a comment - Thanks for reviewing it Chris. Is it necessary to close in the try block, considering that close is guaranteed to happen again in the finally block? Also, why bother to set local variables in and collector to null? Attempting to encourage garbage collection to clean them up faster? Something else? More for correctness. The issue is that I don't want to ignore exceptions during in.close() in normal sequence of action. However, if an exception occurred in, say, in.close() in try block (i.e. then we fall into the finally block), then we should ignore exceptions during collector.close(); hence closeQuietly(collector). Also, closeQuietly is coded to ensure we don't call in.close or collector.close in both try and finally block, hence the games with setting them to null in try block. Makes sense?
        Hide
        Chris Nauroth added a comment -

        The new patches look good, and the new tests are very helpful. I ran some tests on Windows using both trunk and branch-1-win, and they passed.

        I have a question on this part:

            try {
              runner.run(in, new OldOutputCollector(collector, conf), reporter);
              mapPhase.complete();
              // start the sort phase only if there are reducers
              if (numReduceTasks > 0) {
                setPhase(TaskStatus.Phase.SORT);
              }
              statusUpdate(umbilical);
              collector.flush();
              
              in.close();
              in = null;
              
              collector.close();
              collector = null;
            } finally {
              closeQuietly(in);
              closeQuietly(collector);
            }
        

        Is it necessary to close in the try block, considering that close is guaranteed to happen again in the finally block? Also, why bother to set local variables in and collector to null? Attempting to encourage garbage collection to clean them up faster? Something else?

        Show
        Chris Nauroth added a comment - The new patches look good, and the new tests are very helpful. I ran some tests on Windows using both trunk and branch-1-win, and they passed. I have a question on this part: try { runner.run(in, new OldOutputCollector(collector, conf), reporter); mapPhase.complete(); // start the sort phase only if there are reducers if (numReduceTasks > 0) { setPhase(TaskStatus.Phase.SORT); } statusUpdate(umbilical); collector.flush(); in.close(); in = null ; collector.close(); collector = null ; } finally { closeQuietly(in); closeQuietly(collector); } Is it necessary to close in the try block, considering that close is guaranteed to happen again in the finally block? Also, why bother to set local variables in and collector to null? Attempting to encourage garbage collection to clean them up faster? Something else?
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12580040/MAPREDUCE-4737.patch
        against trunk revision .

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

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

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3542//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3542//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/12580040/MAPREDUCE-4737.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3542//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3542//console This message is automatically generated.
        Arun C Murthy made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Arun C Murthy made changes -
        Attachment MAPREDUCE-4737.patch [ 12580040 ]
        Hide
        Arun C Murthy added a comment -

        Fixed RAT warning.

        Show
        Arun C Murthy added a comment - Fixed RAT warning.
        Arun C Murthy made changes -
        Attachment MAPREDUCE-4737_branch1.patch [ 12580039 ]
        Arun C Murthy made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12579951/MAPREDUCE-4737.patch
        against trunk revision .

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

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

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 1 release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//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/12579951/MAPREDUCE-4737.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 1 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3541//console This message is automatically generated.
        Arun C Murthy made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Arun C Murthy made changes -
        Attachment MAPREDUCE-4737.patch [ 12579951 ]
        Hide
        Arun C Murthy added a comment -

        Patch for trunk.

        Show
        Arun C Murthy added a comment - Patch for trunk.
        Arun C Murthy made changes -
        Attachment MAPREDUCE-4737_branch1.patch [ 12579950 ]
        Hide
        Arun C Murthy added a comment -

        Complete patch for branch-1.

        Show
        Arun C Murthy added a comment - Complete patch for branch-1.
        Arun C Murthy made changes -
        Assignee Daniel Dai [ daijy ] Arun C Murthy [ acmurthy ]
        Hide
        Daniel Dai added a comment -

        Arun C Murthy, sure, feel free to take it over.

        Show
        Daniel Dai added a comment - Arun C Murthy , sure, feel free to take it over.
        Arun C Murthy made changes -
        Affects Version/s 1.1.2 [ 12323594 ]
        Affects Version/s 2.0.3-alpha [ 12323275 ]
        Arun C Murthy made changes -
        Attachment MAPREDUCE-4737.patch [ 12579769 ]
        Hide
        Arun C Murthy added a comment -

        Updated patch to fix exception handling for old api too, in addition to Daniel Dai's fix.

        A unit test is pending.

        Show
        Arun C Murthy added a comment - Updated patch to fix exception handling for old api too, in addition to Daniel Dai 's fix. A unit test is pending.
        Hide
        Arun C Murthy added a comment -

        Daniel Dai I started looking closely at this and see that we need a lot more work to get this right (lots of existing exception handling for o.a.h.mapred is broken too). Do you mind if I take this over, fix related issues and add tests etc.? Thanks!

        Show
        Arun C Murthy added a comment - Daniel Dai I started looking closely at this and see that we need a lot more work to get this right (lots of existing exception handling for o.a.h.mapred is broken too). Do you mind if I take this over, fix related issues and add tests etc.? Thanks!
        Arun C Murthy made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Target Version/s 1.2.0, 1-win, 2.0.5-beta [ 12321661, 12321744, 12324032 ]
        Hide
        Arun C Murthy added a comment -

        Agree with Tom White - we'll need to mark this as incompatible even though it's actually making behaviour consistent with o.a.h.mapred.


        Daniel Dai Can you please go ahead and add a test case for this? Thanks!

        Show
        Arun C Murthy added a comment - Agree with Tom White - we'll need to mark this as incompatible even though it's actually making behaviour consistent with o.a.h.mapred. Daniel Dai Can you please go ahead and add a test case for this? Thanks!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12549646/HADOOP-23-2.patch
        against trunk revision .

        +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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2946//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2946//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/12549646/HADOOP-23-2.patch against trunk revision . +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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2946//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2946//console This message is automatically generated.
        Suresh Srinivas made changes -
        Assignee Daniel Dai [ daijy ]
        Suresh Srinivas made changes -
        Project Hadoop Common [ 12310240 ] Hadoop Map/Reduce [ 12310941 ]
        Key HADOOP-8904 MAPREDUCE-4737
        Affects Version/s 1-win [ 12321744 ]
        Affects Version/s 1-win [ 12320361 ]
        Assignee Daniel Dai [ daijy ]
        Hide
        Tom White added a comment -

        This change makes the behaviour the same as the old API where the Mapper's close() method is called in a finally block - which is a good thing. Will it cause incompatibilities with existing code - i.e. any that assume cleanup() won't be called if the map throws an exception?

        We should at least mark this as an incompatible change with a note saying that you need to override the Mapper's (or Reducer's) run() method to restore the old behaviour.

        Show
        Tom White added a comment - This change makes the behaviour the same as the old API where the Mapper's close() method is called in a finally block - which is a good thing. Will it cause incompatibilities with existing code - i.e. any that assume cleanup() won't be called if the map throws an exception? We should at least mark this as an incompatible change with a note saying that you need to override the Mapper's (or Reducer's) run() method to restore the old behaviour.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12549646/HADOOP-23-2.patch
        against trunk revision .

        +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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1644//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1644//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/12549646/HADOOP-23-2.patch against trunk revision . +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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1644//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1644//console This message is automatically generated.
        Hide
        Daniel Dai added a comment -

        Todd Lipcon yes, it is for all branches, not just branch-1-win. The patch itself is for trunk, may need rebase to other branches
        Suresh Srinivas I updated the patch for the format issues

        Show
        Daniel Dai added a comment - Todd Lipcon yes, it is for all branches, not just branch-1-win. The patch itself is for trunk, may need rebase to other branches Suresh Srinivas I updated the patch for the format issues
        Daniel Dai made changes -
        Attachment HADOOP-23-2.patch [ 12549646 ]
        Suresh Srinivas made changes -
        Assignee Daniel Dai [ daijy ]
        Hide
        Suresh Srinivas added a comment -

        Daniel, can you please also change the indentation in this patch to two spaces.

        Show
        Suresh Srinivas added a comment - Daniel, can you please also change the indentation in this patch to two spaces.
        Hide
        Todd Lipcon added a comment -

        This seems like a bug fix which is relevant for branch-1 as well as newer branches, not just branch-1-win, right? Is trunk/branch-2 affected?

        Show
        Todd Lipcon added a comment - This seems like a bug fix which is relevant for branch-1 as well as newer branches, not just branch-1-win, right? Is trunk/branch-2 affected?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12548375/HADOOP-8904-1.patch
        against trunk revision .

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1588//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/12548375/HADOOP-8904-1.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1588//console This message is automatically generated.
        Daniel Dai made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Daniel Dai made changes -
        Field Original Value New Value
        Attachment HADOOP-8904-1.patch [ 12548375 ]
        Daniel Dai created issue -

          People

          • Assignee:
            Arun C Murthy
            Reporter:
            Daniel Dai
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development