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

[MAPREDUCE] The new MapReduce API should make available task's progress to the task

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: task
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Map and Reduce task can access the attempt's overall progress via TaskAttemptContext.
    • Tags:
      mapreduce progress

      Description

      There is no way to get the task's current progress in the new MapReduce API. It would be nice to make it available so that the task (map/reduce) can use it.

      1. MAPREDUCE-2492-v1.3.patch
        30 kB
        Amar Kamat
      2. MAPREDUCE-2492-v1.4.patch
        31 kB
        Amar Kamat
      3. MAPREDUCE-2492-v1.5.patch
        31 kB
        Amar Kamat
      4. MAPREDUCE-2492-v1.6.patch
        31 kB
        Amar Kamat
      5. MAPREDUCE-2492-v1.7.patch
        29 kB
        Amar Kamat

        Issue Links

          Activity

          Hide
          Amar Kamat added a comment -

          Attaching a patch that
          1. Provides the progress to the task (map/reduce) via (Map/Reduce)Context and Reporter.
          2. Testcases for testing the same.
          3. Incorporates MAPREDUCE-2511 and MAPREDUCE-2519

          tets-patch and ant-tests passed on my local box.

          Show
          Amar Kamat added a comment - Attaching a patch that 1. Provides the progress to the task (map/reduce) via (Map/Reduce)Context and Reporter. 2. Testcases for testing the same. 3. Incorporates MAPREDUCE-2511 and MAPREDUCE-2519 tets-patch and ant-tests passed on my local box.
          Hide
          Amar Kamat added a comment -

          While testing, I discovered MAPREDUCE-2523. Will incorporate the fix in this patch as I am modifying the same file.

          Show
          Amar Kamat added a comment - While testing, I discovered MAPREDUCE-2523 . Will incorporate the fix in this patch as I am modifying the same file.
          Hide
          Chris Douglas added a comment -

          Adding progress to TaskAttemptContext makes sense. Minor points on the tests:

          • Please use JUnit4 annotations instead of extending TestCase for new tests
          • Using @BeforeClass instead of an instance initializer block would be easier to read and would fail appropriately
          • Deleting "test.build.data" (defaulting to "/tmp") recursively when the test runs could have bad side-effects. Please use a subdir of rootTestDir instead
          • Cleaning up after the test would also be a good idea (@AfterClass). Right now, many of the tests will not delete output when the test fails.
          Show
          Chris Douglas added a comment - Adding progress to TaskAttemptContext makes sense. Minor points on the tests: Please use JUnit4 annotations instead of extending TestCase for new tests Using @BeforeClass instead of an instance initializer block would be easier to read and would fail appropriately Deleting "test.build.data" (defaulting to "/tmp") recursively when the test runs could have bad side-effects. Please use a subdir of rootTestDir instead Cleaning up after the test would also be a good idea ( @AfterClass ). Right now, many of the tests will not delete output when the test fails.
          Hide
          Amar Kamat added a comment -

          Attaching a new patch incorporating the comments by Chris. test-patch and the modified test-cases passed on my local box. Changes are
          1. Using JUnit4 for the testcases and moved the code inside the instance initializer block into the @BeforeClass and @AfterClass methods.
          2. Each test now has a parent directory under the test-root folder. This directory is recreated on startup and deleted in cleanup.
          3. Incorporated MAPREDUCE-2523.

          Show
          Amar Kamat added a comment - Attaching a new patch incorporating the comments by Chris. test-patch and the modified test-cases passed on my local box. Changes are 1. Using JUnit4 for the testcases and moved the code inside the instance initializer block into the @BeforeClass and @AfterClass methods. 2. Each test now has a parent directory under the test-root folder. This directory is recreated on startup and deleted in cleanup. 3. Incorporated MAPREDUCE-2523 .
          Hide
          Amar Kamat added a comment -

          Running through Hudson.

          Show
          Amar Kamat added a comment - Running through Hudson.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12479987/MAPREDUCE-2492-v1.4.patch
          against trunk revision 1125599.

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

          +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests:
          org.apache.hadoop.mapred.TestReporter

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//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/12479987/MAPREDUCE-2492-v1.4.patch against trunk revision 1125599. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests: org.apache.hadoop.mapred.TestReporter -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/288//console This message is automatically generated.
          Hide
          Amar Kamat added a comment -

          I downloaded the latest patch and ran the failed test. The testcase which failed on Hudson passed on my local box. Attached here (http://pastebin.com/qaKZ57TE) is the run log. See line no 13, 37, 53 for progress values in map/reduce task cleanup.

          Show
          Amar Kamat added a comment - I downloaded the latest patch and ran the failed test. The testcase which failed on Hudson passed on my local box. Attached here ( http://pastebin.com/qaKZ57TE ) is the run log. See line no 13, 37, 53 for progress values in map/reduce task cleanup.
          Hide
          Amar Kamat added a comment -

          Attaching a new patch with custom (low) default Filesystem block size. The only difference between the Hudson run and the run on my local box is the number of splits (hence maps). Maybe Hudson sets the default block size to a low value (default being 32mb). I hope that Hudson respects the custom value for default block size set by the test case.

          The diff between the two patches is as follows

          318c318
          < @@ -0,0 +1,208 @@
          ---
          > @@ -0,0 +1,217 @@
          359a360,361
          > +  // using a default block size of 1k
          > +  private static final long DEFAULT_BLOCK_SIZE = 1024;
          365c367,371
          < +    fs = FileSystem.getLocal(new Configuration());
          ---
          > +    Configuration conf = new Configuration();
          > +    // set the default block size
          > +    conf.setLong("fs.local.block.size", DEFAULT_BLOCK_SIZE);
          > +    
          > +    fs = FileSystem.getLocal(conf);
          367a374,376
          > +    
          > +    assertEquals("Invalid block size", 
          > +                 DEFAULT_BLOCK_SIZE, fs.getDefaultBlockSize());
          
          Show
          Amar Kamat added a comment - Attaching a new patch with custom (low) default Filesystem block size. The only difference between the Hudson run and the run on my local box is the number of splits (hence maps). Maybe Hudson sets the default block size to a low value (default being 32mb). I hope that Hudson respects the custom value for default block size set by the test case. The diff between the two patches is as follows 318c318 < @@ -0,0 +1,208 @@ --- > @@ -0,0 +1,217 @@ 359a360,361 > + // using a default block size of 1k > + private static final long DEFAULT_BLOCK_SIZE = 1024; 365c367,371 < + fs = FileSystem.getLocal( new Configuration()); --- > + Configuration conf = new Configuration(); > + // set the default block size > + conf.setLong( "fs.local.block.size" , DEFAULT_BLOCK_SIZE); > + > + fs = FileSystem.getLocal(conf); 367a374,376 > + > + assertEquals( "Invalid block size" , > + DEFAULT_BLOCK_SIZE, fs.getDefaultBlockSize());
          Hide
          Amar Kamat added a comment -

          Resubmitting with custom value (1k) for default filesystem block size.

          Show
          Amar Kamat added a comment - Resubmitting with custom value (1k) for default filesystem block size.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12480023/MAPREDUCE-2492-v1.5.patch
          against trunk revision 1125599.

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

          +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests:
          org.apache.hadoop.mapred.TestReporter

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//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/12480023/MAPREDUCE-2492-v1.5.patch against trunk revision 1125599. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests: org.apache.hadoop.mapred.TestReporter +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/290//console This message is automatically generated.
          Hide
          Amar Kamat added a comment -

          Canceling the patch as Hudson is still not respecting the configuration values set in the testcase.

          Show
          Amar Kamat added a comment - Canceling the patch as Hudson is still not respecting the configuration values set in the testcase.
          Hide
          Amar Kamat added a comment -

          Attaching a new patch using NLineInputFormat for fooling Hudson. Will open another JIRA to investigate and fix the Hudson issue.

          test-patch passed. TestReporter passed on my local box.

          Show
          Amar Kamat added a comment - Attaching a new patch using NLineInputFormat for fooling Hudson. Will open another JIRA to investigate and fix the Hudson issue. test-patch passed. TestReporter passed on my local box.
          Hide
          Amar Kamat added a comment -

          Updated the testcase such that reduce() is called multiple times. test-patch and the modified testcases pass on my local box.

          Show
          Amar Kamat added a comment - Updated the testcase such that reduce() is called multiple times. test-patch and the modified testcases pass on my local box.
          Hide
          Amar Kamat added a comment -

          Running the latest patch (with NLineInputFormat) through Hudson.

          Show
          Amar Kamat added a comment - Running the latest patch (with NLineInputFormat) through Hudson.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12480095/MAPREDUCE-2492-v1.6.patch
          against trunk revision 1125599.

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

          +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests:
          org.apache.hadoop.mapred.TestReporter

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//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/12480095/MAPREDUCE-2492-v1.6.patch against trunk revision 1125599. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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 (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 failed these core unit tests: org.apache.hadoop.mapred.TestReporter +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/292//console This message is automatically generated.
          Hide
          Amar Kamat added a comment -

          Found some issue with stale left over files in the shared directory. Modified the patch to use a unique folder for each test case.

          Show
          Amar Kamat added a comment - Found some issue with stale left over files in the shared directory. Modified the patch to use a unique folder for each test case.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12480106/MAPREDUCE-2492-v1.7.patch
          against trunk revision 1125599.

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

          +1 tests included. The patch appears to include 15 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 (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 core unit tests.

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//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/12480106/MAPREDUCE-2492-v1.7.patch against trunk revision 1125599. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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 (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 core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/293//console This message is automatically generated.
          Hide
          Amar Kamat added a comment -

          I just committed this.

          Show
          Amar Kamat added a comment - I just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #696 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/696/)
          MAPREDUCE-2492. The new MapReduce API should make available task's progress to the task. (amarrk)

          amarrk : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1126591
          Files :

          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/UtilsForTests.java
          • /hadoop/mapreduce/trunk/CHANGES.txt
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskAttemptContextImpl.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Task.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/TestTaskContext.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/MapReduceTestUtil.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/WrappedMapper.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Reporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/TaskAttemptContext.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/MultithreadedMapper.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/reduce/WrappedReducer.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainReduceContextImpl.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/output/MultipleOutputs.java
          • /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mock/MockReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainMapContextImpl.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestMapProgress.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/join/Parser.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/MapTask.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/StatusReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/ReduceTask.java
          • /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mapreduce/mock/MockReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/task/TaskAttemptContextImpl.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #696 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/696/ ) MAPREDUCE-2492 . The new MapReduce API should make available task's progress to the task. (amarrk) amarrk : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1126591 Files : /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/UtilsForTests.java /hadoop/mapreduce/trunk/CHANGES.txt /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskAttemptContextImpl.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Task.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/TestTaskContext.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/MapReduceTestUtil.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/WrappedMapper.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Reporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/TaskAttemptContext.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/MultithreadedMapper.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/reduce/WrappedReducer.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainReduceContextImpl.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/output/MultipleOutputs.java /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mock/MockReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainMapContextImpl.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestMapProgress.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/join/Parser.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/MapTask.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/StatusReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mapreduce/mock/MockReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/task/TaskAttemptContextImpl.java
          Hide
          Tom White added a comment -

          This is an incompatible change, since it adds a method to the public @Stable mapred.Reporter interface. Is it possible to rework this to only change the new API, as the title suggests? This would then be a compatible change since the classes that have been changed in the new API are private or @Evolving.

          Also, it looks like this wasn't reviewed before being committed.

          Show
          Tom White added a comment - This is an incompatible change, since it adds a method to the public @Stable mapred.Reporter interface. Is it possible to rework this to only change the new API, as the title suggests? This would then be a compatible change since the classes that have been changed in the new API are private or @Evolving. Also, it looks like this wasn't reviewed before being committed.
          Hide
          Amar Kamat added a comment -

          Tom,
          The patch (v1.3) was reviewed by Chris and only few minor testcase changes were suggested (see http://tinyurl.com/mr-2492-review-comments). I incorporated those changes in the latest patch (v1.7).

          W.r.t to the title, I was under the impression that the old mapred API provides some indicator to the task's overall progress. Later during the code review I figured out that the older mapred API provides no such mechanism.

          I think it makes sense to have Reporter provide the task's progress to the map/reduce task attempts. I would prefer marking this change as incompatible.

          Thoughts?

          Show
          Amar Kamat added a comment - Tom, The patch (v1.3) was reviewed by Chris and only few minor testcase changes were suggested (see http://tinyurl.com/mr-2492-review-comments ). I incorporated those changes in the latest patch (v1.7). W.r.t to the title, I was under the impression that the old mapred API provides some indicator to the task's overall progress. Later during the code review I figured out that the older mapred API provides no such mechanism. I think it makes sense to have Reporter provide the task's progress to the map/reduce task attempts. I would prefer marking this change as incompatible. Thoughts?
          Hide
          Tom White added a comment -

          > I think it makes sense to have Reporter provide the task's progress to the map/reduce task attempts. I would prefer marking this change as incompatible.

          It's certainly an improvement, but does it warrant an incompatible change to an interface marked as stable? There may be an argument that it does, but I would have expected to see some discussion about this before it was committed. Why not only change the new API and tell users that they need to use that one if they want to use this feature?

          Show
          Tom White added a comment - > I think it makes sense to have Reporter provide the task's progress to the map/reduce task attempts. I would prefer marking this change as incompatible. It's certainly an improvement, but does it warrant an incompatible change to an interface marked as stable? There may be an argument that it does, but I would have expected to see some discussion about this before it was committed. Why not only change the new API and tell users that they need to use that one if they want to use this feature?
          Hide
          Chris Douglas added a comment -

          This is partially my fault. I only had time for a quick pass over the patch, and had missed the mapred API change among the updates to mapreduce.lib classes.

          I agree with Tom. It's a useful feature, but changing only the new API is probably the better course. Any breakage is unlikely- it's adding, not removing a method from a framework type almost never implemented by users- but I'd lean away from any modifications to the old APIs unless they affect correctness.

          Show
          Chris Douglas added a comment - This is partially my fault. I only had time for a quick pass over the patch, and had missed the mapred API change among the updates to mapreduce.lib classes. I agree with Tom. It's a useful feature, but changing only the new API is probably the better course. Any breakage is unlikely- it's adding, not removing a method from a framework type almost never implemented by users- but I'd lean away from any modifications to the old APIs unless they affect correctness.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #689 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk/689/)
          MAPREDUCE-2492. The new MapReduce API should make available task's progress to the task. (amarrk)

          amarrk : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1126591
          Files :

          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/UtilsForTests.java
          • /hadoop/mapreduce/trunk/CHANGES.txt
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskAttemptContextImpl.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Task.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/TestTaskContext.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/MapReduceTestUtil.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/WrappedMapper.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Reporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/TaskAttemptContext.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/MultithreadedMapper.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/reduce/WrappedReducer.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainReduceContextImpl.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/output/MultipleOutputs.java
          • /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mock/MockReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainMapContextImpl.java
          • /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestMapProgress.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/join/Parser.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/MapTask.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/StatusReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/ReduceTask.java
          • /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mapreduce/mock/MockReporter.java
          • /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/task/TaskAttemptContextImpl.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #689 (See https://builds.apache.org/hudson/job/Hadoop-Mapreduce-trunk/689/ ) MAPREDUCE-2492 . The new MapReduce API should make available task's progress to the task. (amarrk) amarrk : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1126591 Files : /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/UtilsForTests.java /hadoop/mapreduce/trunk/CHANGES.txt /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskAttemptContextImpl.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Task.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/TestTaskContext.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapreduce/MapReduceTestUtil.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/WrappedMapper.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/Reporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/TaskAttemptContext.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/map/MultithreadedMapper.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/reduce/WrappedReducer.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainReduceContextImpl.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/output/MultipleOutputs.java /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mock/MockReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/chain/ChainMapContextImpl.java /hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestMapProgress.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/lib/join/Parser.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/MapTask.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/StatusReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/ReduceTask.java /hadoop/mapreduce/trunk/src/contrib/mrunit/src/java/org/apache/hadoop/mrunit/mapreduce/mock/MockReporter.java /hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/task/TaskAttemptContextImpl.java

            People

            • Assignee:
              Amar Kamat
              Reporter:
              Amar Kamat
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development