Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-4318

TestRecoveryManager should not use raw and deprecated configuration parameters.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.1
    • Fix Version/s: 0.22.1
    • Component/s: test
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      TestRecoveryManager should not use deprecated config keys, and should use constants for the keys where possible.

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #105 (See https://builds.apache.org/job/Hadoop-Mapreduce-22-branch/105/)
          MAPREDUCE-4318. TestRecoveryManager should not use raw configuration keys. Contributed by Benoy Antony. (Revision 1347853)

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

          • /hadoop/common/branches/branch-0.22/mapreduce/CHANGES.txt
          • /hadoop/common/branches/branch-0.22/mapreduce/src/test/mapred/org/apache/hadoop/mapred/TestRecoveryManager.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #105 (See https://builds.apache.org/job/Hadoop-Mapreduce-22-branch/105/ ) MAPREDUCE-4318 . TestRecoveryManager should not use raw configuration keys. Contributed by Benoy Antony. (Revision 1347853) Result = FAILURE shv : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1347853 Files : /hadoop/common/branches/branch-0.22/mapreduce/CHANGES.txt /hadoop/common/branches/branch-0.22/mapreduce/src/test/mapred/org/apache/hadoop/mapred/TestRecoveryManager.java
          Hide
          shv Konstantin Shvachko added a comment -

          I just committed this. Thank you Benoy.

          Show
          shv Konstantin Shvachko added a comment - I just committed this. Thank you Benoy.
          Hide
          shv Konstantin Shvachko added a comment -

          +1 Looks good to me.

          Show
          shv Konstantin Shvachko added a comment - +1 Looks good to me.
          Hide
          benoyantony Benoy Antony added a comment -

          Th other option was to use the new scheme of specifying mapred-queues.xml containing he queue configuration. I used QueueManagerTestUtils class to achieve this. But there are other mapred-queues.xml in the classpath which gets picked up before test's mapred-queues.xml with different configuration.

          These files seem to be created when I build using eclipse and if I remove those mapred-queues.xml, then test passes. So this may be an eclipse created problem.

          The old scheme of defining queues does not use mapred-queues.xml and hence will work regardless multiple mapred-queues.xml issues.

          Since we are not testing Queue management here, I believe, keeping the following line makes the test more reliable.

          mr.getJobTrackerConf().set(DeprecatedQueueConfigurationParser.MAPRED_QUEUE_NAMES_KEY,
          "default");

          So I recommend to go with the attached patch. Please let me know if there are some other ideas.

          Show
          benoyantony Benoy Antony added a comment - Th other option was to use the new scheme of specifying mapred-queues.xml containing he queue configuration. I used QueueManagerTestUtils class to achieve this. But there are other mapred-queues.xml in the classpath which gets picked up before test's mapred-queues.xml with different configuration. These files seem to be created when I build using eclipse and if I remove those mapred-queues.xml, then test passes. So this may be an eclipse created problem. The old scheme of defining queues does not use mapred-queues.xml and hence will work regardless multiple mapred-queues.xml issues. Since we are not testing Queue management here, I believe, keeping the following line makes the test more reliable. mr.getJobTrackerConf().set(DeprecatedQueueConfigurationParser.MAPRED_QUEUE_NAMES_KEY, "default"); So I recommend to go with the attached patch. Please let me know if there are some other ideas.
          Hide
          benoyantony Benoy Antony added a comment -

          The other option will be to define the new way fo defining the queue configuration. This is followed in some other tests. I'll try that and attach a new patch

          Show
          benoyantony Benoy Antony added a comment - The other option will be to define the new way fo defining the queue configuration. This is followed in some other tests. I'll try that and attach a new patch
          Hide
          benoyantony Benoy Antony added a comment -

          Fixed the hardcoded keys.
          Still need to use deprecated config key for mapred.queue.names

          Show
          benoyantony Benoy Antony added a comment - Fixed the hardcoded keys. Still need to use deprecated config key for mapred.queue.names

            People

            • Assignee:
              benoyantony Benoy Antony
              Reporter:
              shv Konstantin Shvachko
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development