Hadoop Common
  1. Hadoop Common
  2. HADOOP-6439

Shuffle deadlocks on wrong number of maps

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.21.0, 0.22.0
    • Fix Version/s: 0.21.0
    • Component/s: conf
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The new shuffle assumes that the number of maps is correct. The new JobSubmitter sets the old value. Something misfires in the middle causing:

      09/12/01 00:00:15 WARN conf.Configuration: mapred.job.split.file is deprecated. Instead, use mapreduce.job.splitfile
      09/12/01 00:00:15 WARN conf.Configuration: mapred.map.tasks is deprecated. Instead, use mapreduce.job.maps

      But my reduces got stuck at 2 maps / 12 when there were only 2 maps in the job.

      1. HADOOP-6439-7.patch
        21 kB
        Hemanth Yamijala
      2. ASF.LICENSE.NOT.GRANTED--HADOOP-6439-6.patch
        21 kB
        V.V.Chaitanya Krishna
      3. HADOOP-6439-6.patch
        21 kB
        V.V.Chaitanya Krishna
      4. HADOOP-6439-5.patch
        21 kB
        V.V.Chaitanya Krishna
      5. HADOOP-6439-4.patch
        20 kB
        V.V.Chaitanya Krishna
      6. HADOOP-6439-3.patch
        20 kB
        V.V.Chaitanya Krishna
      7. HADOOP-6439-2.patch
        19 kB
        V.V.Chaitanya Krishna
      8. HADOOP-6439-1.patch
        17 kB
        V.V.Chaitanya Krishna
      9. mr-1252.patch
        0.8 kB
        Owen O'Malley

        Issue Links

          Activity

          Hide
          Hemanth Yamijala added a comment -

          I just committed this. Thanks, Chaitanya ! (Also marked the fix for version to 0.22, based on Tom's response).

          Show
          Hemanth Yamijala added a comment - I just committed this. Thanks, Chaitanya ! (Also marked the fix for version to 0.22, based on Tom's response).
          Hide
          Tom White added a comment -

          Hemanth/Amareshwari

          That's right - I'm going to cut a new 0.21 branch from trunk on April 30, so this patch only needs to be committed to trunk. Thanks!

          Tom

          Show
          Tom White added a comment - Hemanth/Amareshwari That's right - I'm going to cut a new 0.21 branch from trunk on April 30, so this patch only needs to be committed to trunk. Thanks! Tom
          Hide
          Hemanth Yamijala added a comment -

          @Hemanth, I don't think we need to update patches for branch 0.21, as per discussion in general mailing list, branch 0.21 will be cut from trunk itself.

          Ah. If that's so, then we are good to go. I will wait for Tom's / anyone else's confirmation on this.

          Show
          Hemanth Yamijala added a comment - @Hemanth, I don't think we need to update patches for branch 0.21, as per discussion in general mailing list, branch 0.21 will be cut from trunk itself. Ah. If that's so, then we are good to go. I will wait for Tom's / anyone else's confirmation on this.
          Hide
          Amareshwari Sriramadasu added a comment -

          However, I tried to see if the patch applies to branch 0.21 as well, since this is a blocker for that version. The patch failed to apply, unfortunately. Can you please upload a patch for that as well ?

          @Hemanth, I don't think we need to update patches for branch 0.21, as per discussion in general mailing list, branch 0.21 will be cut from trunk itself.
          @Tom, please correct me if I'm wrong.

          Show
          Amareshwari Sriramadasu added a comment - However, I tried to see if the patch applies to branch 0.21 as well, since this is a blocker for that version. The patch failed to apply, unfortunately. Can you please upload a patch for that as well ? @Hemanth, I don't think we need to update patches for branch 0.21, as per discussion in general mailing list, branch 0.21 will be cut from trunk itself. @Tom, please correct me if I'm wrong.
          Hide
          Hemanth Yamijala added a comment -

          Chaitanya, thanks for running the tests. The patch for trunk is ready for commit. However, I tried to see if the patch applies to branch 0.21 as well, since this is a blocker for that version. The patch failed to apply, unfortunately. Can you please upload a patch for that as well ?

          Show
          Hemanth Yamijala added a comment - Chaitanya, thanks for running the tests. The patch for trunk is ready for commit. However, I tried to see if the patch applies to branch 0.21 as well, since this is a blocker for that version. The patch failed to apply, unfortunately. Can you please upload a patch for that as well ?
          Hide
          V.V.Chaitanya Krishna added a comment -

          Ran tests of mapreduce and hdfs with the core jar built with this patch.
          All tests passed except for TestTrackerDistributedCacheManager in mapreduce and TestFiHFlush in hdfs.
          These tests failed even without this patch being applied.

          Show
          V.V.Chaitanya Krishna added a comment - Ran tests of mapreduce and hdfs with the core jar built with this patch. All tests passed except for TestTrackerDistributedCacheManager in mapreduce and TestFiHFlush in hdfs. These tests failed even without this patch being applied.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442271/HADOOP-6439-7.patch
          against trunk revision 934619.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/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/12442271/HADOOP-6439-7.patch against trunk revision 934619. +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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/465/console This message is automatically generated.
          Hide
          Hemanth Yamijala added a comment -

          Running through Hudson. Chaitanya, can you please (one last time hopefully, smile), test a core jar built with this patch with mapreduce and HDFS and then I can commit this ?

          Show
          Hemanth Yamijala added a comment - Running through Hudson. Chaitanya, can you please (one last time hopefully, smile ), test a core jar built with this patch with mapreduce and HDFS and then I can commit this ?
          Hide
          Hemanth Yamijala added a comment -

          Patch fixing minor formatting problems; no other changes.

          Show
          Hemanth Yamijala added a comment - Patch fixing minor formatting problems; no other changes.
          Hide
          Hemanth Yamijala added a comment -

          I looked at the patch as a sanity check and it seems fine. There were couple of formatting problems in the patch which I will fix and upload a new one soon. Canceling the current patch to take care of that.

          Show
          Hemanth Yamijala added a comment - I looked at the patch as a sanity check and it seems fine. There were couple of formatting problems in the patch which I will fix and upload a new one soon. Canceling the current patch to take care of that.
          Hide
          V.V.Chaitanya Krishna added a comment -

          The Hudson result doesn't seem to be posting here. The result is as below:

            [exec] BUILD SUCCESSFUL
               [exec] Total time: 7 seconds
               [exec] 
               [exec] 
               [exec] 
               [exec] 
               [exec] +1 overall.  Here are the results of testing the latest attachment 
               [exec]   http://issues.apache.org/jira/secure/attachment/12441685/HADOOP-6439-6.patch
               [exec]   against trunk revision 933810.
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec] 
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec] 
               [exec]     +1 core tests.  The patch passed core unit tests.
               [exec] 
               [exec]     +1 contrib tests.  The patch passed contrib unit tests.
               [exec] 
               [exec] Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/testReport/
               [exec] Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
               [exec] Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/artifact/trunk/build/test/checkstyle-errors.html
               [exec] Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/console
               [exec] 
          
          
          Show
          V.V.Chaitanya Krishna added a comment - The Hudson result doesn't seem to be posting here. The result is as below: [exec] BUILD SUCCESSFUL [exec] Total time: 7 seconds [exec] [exec] [exec] [exec] [exec] +1 overall. Here are the results of testing the latest attachment [exec] http://issues.apache.org/jira/secure/attachment/12441685/HADOOP-6439-6.patch [exec] against trunk revision 933810. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 core tests. The patch passed core unit tests. [exec] [exec] +1 contrib tests. The patch passed contrib unit tests. [exec] [exec] Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/testReport/ [exec] Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html [exec] Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/artifact/trunk/build/test/checkstyle-errors.html [exec] Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/462/console [exec]
          Hide
          V.V.Chaitanya Krishna added a comment -

          Re-submitting the same patch for Hudson.

          Show
          V.V.Chaitanya Krishna added a comment - Re-submitting the same patch for Hudson.
          Hide
          V.V.Chaitanya Krishna added a comment -

          The patch applies even after a long time. smile

          Running through hudson again.

          Show
          V.V.Chaitanya Krishna added a comment - The patch applies even after a long time. smile Running through hudson again.
          Hide
          Tom White added a comment -

          Is this ready to be committed?

          Show
          Tom White added a comment - Is this ready to be committed?
          Hide
          V.V.Chaitanya Krishna added a comment -

          Ran ant test in mapreduce successfully with the core jar containing the patch.
          All tests passed except TestTrackerDistributedCacheManager which passed later when run individually.

          Show
          V.V.Chaitanya Krishna added a comment - Ran ant test in mapreduce successfully with the core jar containing the patch. All tests passed except TestTrackerDistributedCacheManager which passed later when run individually.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12429634/HADOOP-6439-6.patch
          against trunk revision 896691.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/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/12429634/HADOOP-6439-6.patch against trunk revision 896691. +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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/267/console This message is automatically generated.
          Hide
          V.V.Chaitanya Krishna added a comment -

          Ran the hdfs project with the common jar with the patch.
          Some of them failed out (timeout issue) but succeeded when run individually.

          Show
          V.V.Chaitanya Krishna added a comment - Ran the hdfs project with the common jar with the patch. Some of them failed out (timeout issue) but succeeded when run individually.
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading new patch.

          Show
          V.V.Chaitanya Krishna added a comment - Uploading new patch.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/266/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/12429519/HADOOP-6439-5.patch against trunk revision 896691. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/266/console This message is automatically generated.
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading patch with above suggested testcase incorporated.

          Also, locally running the jar with this patch on mapreduce and hdfs projects.

          Show
          V.V.Chaitanya Krishna added a comment - Uploading patch with above suggested testcase incorporated. Also, locally running the jar with this patch on mapreduce and hdfs projects.
          Hide
          Hemanth Yamijala added a comment -

          This is looking good. I have a very minor comment. In the test case, I would recommend we write a deprecated key with multiple values - like the key X, and make sure that loadResource actually loads all the keys - both old and new. With this change, I think the patch is good to go.

          Can you please run tests with the new common jar file with both HDFS and MapReduce projects and upload results of those also here. This is in addition to Hudson validation of the common project's unit tests itself.

          Show
          Hemanth Yamijala added a comment - This is looking good. I have a very minor comment. In the test case, I would recommend we write a deprecated key with multiple values - like the key X, and make sure that loadResource actually loads all the keys - both old and new. With this change, I think the patch is good to go. Can you please run tests with the new common jar file with both HDFS and MapReduce projects and upload results of those also here. This is in addition to Hudson validation of the common project's unit tests itself.
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading patch with the above mentioned comments implemented.

          Show
          V.V.Chaitanya Krishna added a comment - Uploading patch with the above mentioned comments implemented.
          Hide
          Hemanth Yamijala added a comment -
          • In this patch, we are updating a resource in the updatingResource map, if a null value has been set for a key, but we are not adding the null value to the properties object storing the key-value mapping. So, suppose a non-null value is stored for this key from a previously loaded resource, then we would printing the value belonging to the older resource, but the name of the new resource. This is inconsistent. In general, I am very skeptical of changing any behavior in how configurations are loaded because it is so core to the entire framework. IOW, I would be happy if the old code structure in loadResource is retained when pulled to the new method loadProperty. Of course, unless there's a good reason for the change which I'd be happy to see.
          • Also, previously whenever we are reloading the properties object, we were reseting the 'accessed' flag for the deprecated key, which is removed in this patch.

          I haven't looked at the test cases very closely - though the changes seem mostly OK so far. Cutting down the changes in Configuration.java will actually allow us to not have to write more test scenarios. So, I would like to relook at the cases after the changes above are made.

          Show
          Hemanth Yamijala added a comment - In this patch, we are updating a resource in the updatingResource map, if a null value has been set for a key, but we are not adding the null value to the properties object storing the key-value mapping. So, suppose a non-null value is stored for this key from a previously loaded resource, then we would printing the value belonging to the older resource, but the name of the new resource. This is inconsistent. In general, I am very skeptical of changing any behavior in how configurations are loaded because it is so core to the entire framework. IOW, I would be happy if the old code structure in loadResource is retained when pulled to the new method loadProperty. Of course, unless there's a good reason for the change which I'd be happy to see. Also, previously whenever we are reloading the properties object, we were reseting the 'accessed' flag for the deprecated key, which is removed in this patch. I haven't looked at the test cases very closely - though the changes seem mostly OK so far. Cutting down the changes in Configuration.java will actually allow us to not have to write more test scenarios. So, I would like to relook at the cases after the changes above are made.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12428406/HADOOP-6439-3.patch
          against trunk revision 892113.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/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/12428406/HADOOP-6439-3.patch against trunk revision 892113. +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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/223/console This message is automatically generated.
          Hide
          Amareshwari Sriramadasu added a comment -

          Submitting for hudson

          Show
          Amareshwari Sriramadasu added a comment - Submitting for hudson
          Hide
          Amareshwari Sriramadasu added a comment -

          Changes look fine to me.

          Show
          Amareshwari Sriramadasu added a comment - Changes look fine to me.
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading new patch with the above comments implemented.
          I tested it on trunk for the scenario mentioned by Owen in the above comments. It worked successfully.
          I'm not able to test it on 0.21 due to some problem with tasktracker crashing ( logs show java.lang.NoSuchMethodError: org.apache.hadoop.ipc.RPC.waitForProxy ).

          Show
          V.V.Chaitanya Krishna added a comment - Uploading new patch with the above comments implemented. I tested it on trunk for the scenario mentioned by Owen in the above comments. It worked successfully. I'm not able to test it on 0.21 due to some problem with tasktracker crashing ( logs show java.lang.NoSuchMethodError: org.apache.hadoop.ipc.RPC.waitForProxy ).
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch looks good. Some minor comments on the test:
          1. Javadoc for the test needs modification.
          2. Can you add some more comments on for the tests?
          3. Can you extend testDeprecation for "set for M and get on N" and "set for Y and Z, get on X" ?

          Show
          Amareshwari Sriramadasu added a comment - Patch looks good. Some minor comments on the test: 1. Javadoc for the test needs modification. 2. Can you add some more comments on for the tests? 3. Can you extend testDeprecation for "set for M and get on N" and "set for Y and Z, get on X" ?
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading patch with the above comments implemented.

          Show
          V.V.Chaitanya Krishna added a comment - Uploading patch with the above comments implemented.
          Hide
          Amareshwari Sriramadasu added a comment -

          Some comments on the patch:
          1.

                    if (finalParameter) {
                      finalParameters.add(attr);
                    }
          

          The above code is outside the check for value!= null, but it is moved inside in the patch. Please make sure it is outside.

          2. Shall we divide the test into two tests to simplify? First test tests the order of loading, Second one tests final parameters and order of loading.
          3. Can you add asserts for the values after every loadResource?
          4. Can you add test to verify null values for the final parameters?

          Show
          Amareshwari Sriramadasu added a comment - Some comments on the patch: 1. if (finalParameter) { finalParameters.add(attr); } The above code is outside the check for value!= null, but it is moved inside in the patch. Please make sure it is outside. 2. Shall we divide the test into two tests to simplify? First test tests the order of loading, Second one tests final parameters and order of loading. 3. Can you add asserts for the values after every loadResource? 4. Can you add test to verify null values for the final parameters?
          Hide
          Ravi Gummadi added a comment -

          OK. As nothing is marked as final in *default.xml, everything should be fine.

          Show
          Ravi Gummadi added a comment - OK. As nothing is marked as final in *default.xml, everything should be fine.
          Hide
          Ravi Gummadi added a comment -

          For final parameters, does it allow admin to set a value in mapred-site.xml ? OR Is it making the value given in mapred-default.xml as final and not considering the value given by admin in mapred-site.xml ?

          Show
          Ravi Gummadi added a comment - For final parameters, does it allow admin to set a value in mapred-site.xml ? OR Is it making the value given in mapred-default.xml as final and not considering the value given by admin in mapred-site.xml ?
          Hide
          V.V.Chaitanya Krishna added a comment -

          Uploading patch with the following changes implemented:

          • The value of the key will be the one which is loaded/set most recently. (Note: As maintained previously, there will be only new keys stored in the Configuration object.)
          • If the key is set to final, then that key (or the corresponding new key in case of deprecation) will be marked as final and no further changes to its value will be done. This is different from the way final parameters were handled in HADOOP-6105, where the presence of old key itself mattered(irrespective of which value is set/loaded last) and precedence order had to be considered even for being final.
          Show
          V.V.Chaitanya Krishna added a comment - Uploading patch with the following changes implemented: The value of the key will be the one which is loaded/set most recently. (Note: As maintained previously, there will be only new keys stored in the Configuration object.) If the key is set to final, then that key (or the corresponding new key in case of deprecation) will be marked as final and no further changes to its value will be done. This is different from the way final parameters were handled in HADOOP-6105 , where the presence of old key itself mattered(irrespective of which value is set/loaded last) and precedence order had to be considered even for being final.
          Hide
          Ravi Gummadi added a comment -

          When mapred-site.xml has old key and job.xml has new key, the value given in mapred-site.xml is taking precedence because of HADOOP-6105.

          Show
          Ravi Gummadi added a comment - When mapred-site.xml has old key and job.xml has new key, the value given in mapred-site.xml is taking precedence because of HADOOP-6105 .
          Hide
          Ravi Gummadi added a comment -

          In HADOOP-6105, old key takes precedence irrespective of the order in which resources are loaded. I propose that we resolve based on the order of loading of resources irrespective of old/new key is seen.

          Thoughts ?

          Show
          Ravi Gummadi added a comment - In HADOOP-6105 , old key takes precedence irrespective of the order in which resources are loaded. I propose that we resolve based on the order of loading of resources irrespective of old/new key is seen. Thoughts ?
          Hide
          Owen O'Malley added a comment -

          To be clearer:

          1. The previous patch fixes the warnings, but not the problem. Thus, it is necessary but not sufficient.
          2. On a one node cluster with 0.21.0-dev from yesterday:
          a. Have mapred.map.tasks set to 12 in hadoop-site.xml.
          b. Submit a word count example on a two file input directory.
          3. The 2 maps run fine.
          4. The 2 reduces both lock up at 5.5% with a status of 2 of 12 maps fetched.

          Looking at the system directory the job.xml has mapreduce.job.maps = 2, but the task and the web ui show it as 12.

          If I change the config to use mapreduce.job.maps to set the default, everything works correctly.

          Show
          Owen O'Malley added a comment - To be clearer: 1. The previous patch fixes the warnings, but not the problem. Thus, it is necessary but not sufficient. 2. On a one node cluster with 0.21.0-dev from yesterday: a. Have mapred.map.tasks set to 12 in hadoop-site.xml. b. Submit a word count example on a two file input directory. 3. The 2 maps run fine. 4. The 2 reduces both lock up at 5.5% with a status of 2 of 12 maps fetched. Looking at the system directory the job.xml has mapreduce.job.maps = 2, but the task and the web ui show it as 12. If I change the config to use mapreduce.job.maps to set the default, everything works correctly.
          Hide
          Owen O'Malley added a comment -

          This patch is necessary to suppress the warnings, but doesn't fix the problem.

          The problem is that I was using a configuration with the old attribute name. The attribute name isn't being translated during the xml loading. I'm not sure how it is happening, but in some context, we get 2 and others 12. When I change the config to use the new attribute name, it all goes away.

          I suspect this is caused by a bug in HADOOP-6105.

          Show
          Owen O'Malley added a comment - This patch is necessary to suppress the warnings, but doesn't fix the problem. The problem is that I was using a configuration with the old attribute name. The attribute name isn't being translated during the xml loading. I'm not sure how it is happening, but in some context, we get 2 and others 12. When I change the config to use the new attribute name, it all goes away. I suspect this is caused by a bug in HADOOP-6105 .
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12426479/mr-1252.patch
          against trunk revision 885530.

          +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 patch. The patch command could not apply the patch.

          Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/154/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/12426479/mr-1252.patch against trunk revision 885530. +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 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/154/console This message is automatically generated.

            People

            • Assignee:
              V.V.Chaitanya Krishna
              Reporter:
              Owen O'Malley
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development