Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.24.0
    • Fix Version/s: 0.23.3, 2.0.2-alpha
    • Component/s: mrv2
    • Labels:
      None

      Description

      TestWritableJobConf is currently failing two tests on trunk:

      • testEmptyConfiguration
      • testNonEmptyConfiguration

      Appears to have been caused by HADOOP-8167.

      1. MAPREDUCE-4010.patch
        2 kB
        Alejandro Abdelnur
      2. MAPREDUCE-4010.patch
        0.9 kB
        Alejandro Abdelnur
      3. MAPREDUCE-4010.patch
        2 kB
        Alejandro Abdelnur

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #330 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/330/)
          svn merge -c 1301551,1300642,1332821,1303884 FIXES:MAPREDUCE-4010,HADOOP-8167,HADOOP-8172,HADOOP-8197 Patches to fix OOZIE-761 (Revision 1367102)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #330 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/330/ ) svn merge -c 1301551,1300642,1332821,1303884 FIXES: MAPREDUCE-4010 , HADOOP-8167 , HADOOP-8172 , HADOOP-8197 Patches to fix OOZIE-761 (Revision 1367102) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1367102 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestDeprecatedKeys.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Robert Joseph Evans added a comment -

          I pulled this into branch-0.23

          Show
          Robert Joseph Evans added a comment - I pulled this into branch-0.23
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1022 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1022/)
          MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1022 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1022/ ) MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Build #228 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/228/)
          svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #228 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/228/ ) svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #200 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/200/)
          svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #200 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/200/ ) svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #987 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/987/)
          MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #987 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/987/ ) MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1896 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1896/)
          MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551)

          Result = ABORTED
          bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1896 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1896/ ) MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551) Result = ABORTED bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Commit #700 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/700/)
          svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553)

          Result = ABORTED
          bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #700 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/700/ ) svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553) Result = ABORTED bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1888 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1888/)
          MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1888 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1888/ ) MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #1962 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1962/)
          MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1962 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1962/ ) MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301551) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301551 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-0.23-Commit #692 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/692/)
          svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #692 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/692/ ) svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Commit #683 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/683/)
          svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010. TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #683 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/683/ ) svn merge -c 1301551 from trunk to branch-0.23 FIXES MAPREDUCE-4010 . TestWritableJobConf fails on trunk (tucu via bobby) (Revision 1301553) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1301553 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          Hide
          Robert Joseph Evans added a comment -

          Thanks Alejandro,

          I just checked this into 0.23.3 and trunk. I also added a small comment in the test about why we are skipping the deprecated values.

          Show
          Robert Joseph Evans added a comment - Thanks Alejandro, I just checked this into 0.23.3 and trunk. I also added a small comment in the test about why we are skipping the deprecated values.
          Hide
          Alejandro Abdelnur added a comment -

          patch is good, test-patch does not like it because goes across common/mapred

          Show
          Alejandro Abdelnur added a comment - patch is good, test-patch does not like it because goes across common/mapred
          Hide
          Hadoop QA added a comment -

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

          +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: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2060//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/12518551/MAPREDUCE-4010.patch against trunk revision . +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: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2060//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          Adding Robert's fix to the testcase (missed that before as was not part of my change, seems the bug in the testcase was there all along).

          I don't see a better solution than exposing isDeprecated(). I think is a useful method to have, just odd that we are making it public because of a testcase

          Show
          Alejandro Abdelnur added a comment - Adding Robert's fix to the testcase (missed that before as was not part of my change, seems the bug in the testcase was there all along). I don't see a better solution than exposing isDeprecated(). I think is a useful method to have, just odd that we are making it public because of a testcase
          Hide
          Robert Joseph Evans added a comment -

          The assert equals bug is fixed by the following patch

          diff --git hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java hadoop-mapreduce-project/hadoop
          index c3996b2..e8dff93 100644
          --- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          +++ hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java
          @@ -66,7 +66,7 @@ public class TestWritableJobConf extends TestCase {
                 map1.put(entry.getKey(), entry.getValue());
               }
           
          -    Iterator<Map.Entry<String, String>> iterator2 = conf1.iterator();
          +    Iterator<Map.Entry<String, String>> iterator2 = conf2.iterator();
               Map<String, String> map2 = new HashMap<String,String>();
               while (iterator2.hasNext()) {
                 Map.Entry<String, String> entry = iterator2.next();
          

          Once the test is fixed it is not just the size comparison that fails but also the comparison of the config objects. The only way to fix these failures is to take into account the deprecated configs when comparing both the size and the values of the Configuration objects. Your first patch takes the deprecated values into account when comparing. The second patch does not.

          I don't like the first patch having to expose a new API, and if you can think of a good way to fix the test without needing that API, I would love it. But I do not see any way to do that, and keep the tests still loading values from the *-default.xml files.

          Show
          Robert Joseph Evans added a comment - The assert equals bug is fixed by the following patch diff --git hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java hadoop-mapreduce-project/hadoop index c3996b2..e8dff93 100644 --- hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java +++ hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestWritableJobConf.java @@ -66,7 +66,7 @@ public class TestWritableJobConf extends TestCase { map1.put(entry.getKey(), entry.getValue()); } - Iterator<Map.Entry<String, String>> iterator2 = conf1.iterator(); + Iterator<Map.Entry<String, String>> iterator2 = conf2.iterator(); Map<String, String> map2 = new HashMap<String,String>(); while (iterator2.hasNext()) { Map.Entry<String, String> entry = iterator2.next(); Once the test is fixed it is not just the size comparison that fails but also the comparison of the config objects. The only way to fix these failures is to take into account the deprecated configs when comparing both the size and the values of the Configuration objects. Your first patch takes the deprecated values into account when comparing. The second patch does not. I don't like the first patch having to expose a new API, and if you can think of a good way to fix the test without needing that API, I would love it. But I do not see any way to do that, and keep the tests still loading values from the *-default.xml files.
          Hide
          Alejandro Abdelnur added a comment -

          What do you mean by fixing the assertEquals() bug? How you suggest to fix it?

          Also, my first patch patch makes the Configuration.isDeprecated() method public, is this something we want to do for fixing a testcase? I'm more inclined to the second patch to fix the testcases.

          Thoughts?

          Show
          Alejandro Abdelnur added a comment - What do you mean by fixing the assertEquals() bug? How you suggest to fix it? Also, my first patch patch makes the Configuration.isDeprecated() method public, is this something we want to do for fixing a testcase? I'm more inclined to the second patch to fix the testcases. Thoughts?
          Hide
          Robert Joseph Evans added a comment -

          OK I have debugged the root cause of the failure.

          When we read back in a value. It sets both the original value and it also sets the deprecated value. This way the original 273 map entries can become much bigger 364 entries. Which looking at your original patch is probably what you already knew was happening, and why you were ignoring the deprecated values when doing the comparison. Which means that not only the size comparison should fail, but so should the map comparison. Looking at the test there is a 3 and a half year old copy and past error where we are comparing conf1 to conf1!!! iterator2 is being set to conf1.iterator not conf2.iterator.

          Alejandro,

          Thanks for putting up with me digging down into a root cause that you probably already understood. I am fine with your first patch, so long as you fix the assertEquals bug I mentioned above and explain in a comment why we are ignoring the deprecated values.

          Show
          Robert Joseph Evans added a comment - OK I have debugged the root cause of the failure. When we read back in a value. It sets both the original value and it also sets the deprecated value. This way the original 273 map entries can become much bigger 364 entries. Which looking at your original patch is probably what you already knew was happening, and why you were ignoring the deprecated values when doing the comparison. Which means that not only the size comparison should fail, but so should the map comparison. Looking at the test there is a 3 and a half year old copy and past error where we are comparing conf1 to conf1!!! iterator2 is being set to conf1.iterator not conf2.iterator. Alejandro, Thanks for putting up with me digging down into a root cause that you probably already understood. I am fine with your first patch, so long as you fix the assertEquals bug I mentioned above and explain in a comment why we are ignoring the deprecated values.
          Hide
          Alejandro Abdelnur added a comment -

          The root cause of the failure is that the deprecation logic (to replace old keys with new keys) has always been wrong as it was breaking iterator(), but his was never exposed as it was not being used until now. HADOOP-8167 attempts to fix this in the least intrusive way, still intrusive. We can can 'fix' size() to return size of the iterator, but that would require building and iterating the iterator on size().

          IMO this is an expensive thing to do, thus the change in this testcase which verifies the contents of the configuration are the expected ones.

          Show
          Alejandro Abdelnur added a comment - The root cause of the failure is that the deprecation logic (to replace old keys with new keys) has always been wrong as it was breaking iterator(), but his was never exposed as it was not being used until now. HADOOP-8167 attempts to fix this in the least intrusive way, still intrusive. We can can 'fix' size() to return size of the iterator, but that would require building and iterating the iterator on size(). IMO this is an expensive thing to do, thus the change in this testcase which verifies the contents of the configuration are the expected ones.
          Hide
          Robert Joseph Evans added a comment -

          I personally think that we do want Container.size() to return the same number of elements as iterator would. If it does not the serialization/deserialization code is broken. That is what this test is testing.

            public int size() {
              return getProps().size();
            }
          
            public void write(DataOutput out) throws IOException {
              Properties props = getProps();
              WritableUtils.writeVInt(out, props.size());
              for(Map.Entry<Object, Object> item: props.entrySet()) {
                org.apache.hadoop.io.Text.writeString(out, (String) item.getKey());
                org.apache.hadoop.io.Text.writeString(out, (String) item.getValue());
              }
            }
          
            @Override
            public void readFields(DataInput in) throws IOException {
              clear();
              int size = WritableUtils.readVInt(in);
              for(int i=0; i < size; ++i) {
                set(org.apache.hadoop.io.Text.readString(in), 
                    org.apache.hadoop.io.Text.readString(in));
              }
            }
          

          I am not saying that the code is wrong here. All I am saying is that we should not fix a test by commenting out assertions until we truly understand the root cause of the failure.

          Show
          Robert Joseph Evans added a comment - I personally think that we do want Container.size() to return the same number of elements as iterator would. If it does not the serialization/deserialization code is broken. That is what this test is testing. public int size() { return getProps().size(); } public void write(DataOutput out) throws IOException { Properties props = getProps(); WritableUtils.writeVInt(out, props.size()); for (Map.Entry< Object , Object > item: props.entrySet()) { org.apache.hadoop.io.Text.writeString(out, ( String ) item.getKey()); org.apache.hadoop.io.Text.writeString(out, ( String ) item.getValue()); } } @Override public void readFields(DataInput in) throws IOException { clear(); int size = WritableUtils.readVInt(in); for ( int i=0; i < size; ++i) { set(org.apache.hadoop.io.Text.readString(in), org.apache.hadoop.io.Text.readString(in)); } } I am not saying that the code is wrong here. All I am saying is that we should not fix a test by commenting out assertions until we truly understand the root cause of the failure.
          Hide
          Alejandro Abdelnur added a comment -

          patch removing only the size comparison.

          Show
          Alejandro Abdelnur added a comment - patch removing only the size comparison.
          Hide
          Alejandro Abdelnur added a comment -

          Are you suggesting Configuration.size() to return the number of elements iterator() would return? That will be an expensive operation, no?

          I'd instead remove the the size() assertion then from the patch, thoughts?

          Show
          Alejandro Abdelnur added a comment - Are you suggesting Configuration.size() to return the number of elements iterator() would return? That will be an expensive operation, no? I'd instead remove the the size() assertion then from the patch, thoughts?
          Hide
          Robert Joseph Evans added a comment -

          I am a bit confused why changing the test is the correct solution to this problem. This test has not changed in 3 and a half years. The test predates Oozie, so I am very nervous about trying to maintain backwards compatibility by modifying a test in what appears to be a non-backwards compatible way.

          The patch is doing more changes then are needed even if changing the test is what is wanted. The only assertion that is failing is the size assertion. The maps produced are identical, so it is perhaps Configuration.size that we want to look at. We do not need to pull out deprecated keys, because if HADOOP-8167 is working properly it should make sure that iterator returns the same thing.

          Show
          Robert Joseph Evans added a comment - I am a bit confused why changing the test is the correct solution to this problem. This test has not changed in 3 and a half years . The test predates Oozie, so I am very nervous about trying to maintain backwards compatibility by modifying a test in what appears to be a non-backwards compatible way. The patch is doing more changes then are needed even if changing the test is what is wanted. The only assertion that is failing is the size assertion. The maps produced are identical, so it is perhaps Configuration.size that we want to look at. We do not need to pull out deprecated keys, because if HADOOP-8167 is working properly it should make sure that iterator returns the same thing.
          Hide
          Alejandro Abdelnur added a comment -

          and again test-patch burps with files in different projects (common, mapred)

          Show
          Alejandro Abdelnur added a comment - and again test-patch burps with files in different projects (common, mapred)
          Hide
          Hadoop QA added a comment -

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

          +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: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2055//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/12518391/MAPREDUCE-4010.patch against trunk revision . +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: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2055//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          Had to make the Configuration.isDeprecated() method public. Using that method from the testcase to ignore deprecated values spitted by the iterator.

          Show
          Alejandro Abdelnur added a comment - Had to make the Configuration.isDeprecated() method public. Using that method from the testcase to ignore deprecated values spitted by the iterator.
          Hide
          Robert Joseph Evans added a comment -

          Added Alejandro because he did HADOOP-8167

          Show
          Robert Joseph Evans added a comment - Added Alejandro because he did HADOOP-8167
          Hide
          Robert Joseph Evans added a comment -

          This is very important because of how java does lazy loading of classes we can set values on the command line that are deprecated values and the deprecations are not processed until later. It means that all deprecated conf options set on the command line may be silently be ignored depending on where they fall in the hash map.

          Show
          Robert Joseph Evans added a comment - This is very important because of how java does lazy loading of classes we can set values on the command line that are deprecated values and the deprecations are not processed until later. It means that all deprecated conf options set on the command line may be silently be ignored depending on where they fall in the hash map.

            People

            • Assignee:
              Alejandro Abdelnur
              Reporter:
              Jason Lowe
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development