Hadoop Common
  1. Hadoop Common
  2. HADOOP-6875

[Herriot] Cleanup of temp. configurations is needed upon restart of a cluster

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      herriot

      Description

      1. New configuration directory is not cleaning up after resetting to default configuration directory in a pushconfig functionality. Because of this reason, it's giving permission denied problem for a folder, if other user tried running the tests in the same cluster with pushconfig functionality. I could see this issue while running the tests on a cluster with security enabled and different user.

      I have added the functionality for above issue and attaching the patch

      2. Throwing IOException and it says token is expired while running the tests. I could see this issue in a secure cluster.

      This issue has been resolved by setting the following attribute in the configuration.

      mapreduce.job.complete.cancel.delegation.tokens=false

      adding/updating this attribute in the push configuration functionality while creating the new configuration.

      1. MAPREDUCE-1913.patch
        1 kB
        Vinay Kumar Thota
      2. HADOOP-6875.patch
        2 kB
        Vinay Kumar Thota
      3. HADOOP-6875.patch
        0.8 kB
        Vinay Kumar Thota
      4. 6875-ydist-security.patch
        1 kB
        Vinay Kumar Thota
      5. 6875-ydist-security.patch
        1 kB
        Vinay Kumar Thota
      6. 1913-ydist-security.patch
        2 kB
        Vinay Kumar Thota

        Issue Links

          Activity

          Hide
          Vinay Kumar Thota added a comment -

          Initial patch for yahoo dist security branch.

          Show
          Vinay Kumar Thota added a comment - Initial patch for yahoo dist security branch.
          Hide
          Vinay Kumar Thota added a comment -

          Patch for trunk.

          Show
          Vinay Kumar Thota added a comment - Patch for trunk.
          Hide
          Iyappan Srinivasan added a comment -

          + if (props.get(MAPRED_DELEGATION_TOKEN) == null)

          { + initConf.setBoolean(MAPRED_DELEGATION_TOKEN, false); + }
          • I think it should be, if the delegation token exists, then it should be set to false.
            So, should not it be if (props.get(MAPRED_DELEGATION_TOKEN) != null) {
          Show
          Iyappan Srinivasan added a comment - + if (props.get(MAPRED_DELEGATION_TOKEN) == null) { + initConf.setBoolean(MAPRED_DELEGATION_TOKEN, false); + } I think it should be, if the delegation token exists, then it should be set to false. So, should not it be if (props.get(MAPRED_DELEGATION_TOKEN) != null) {
          Hide
          Konstantin Boudnik added a comment -
          • For trunk, the config key is already defined as in
            src/java/org/apache/hadoop/mapreduce/MRJobConfig.java:  public static final String JOB_CANCEL_DELEGATION_TOKEN = "mapreduce.job.complete.cancel.delegation.tokens";
            
          • Also, please link if this JIRA to its blockers if any.
          Show
          Konstantin Boudnik added a comment - For trunk, the config key is already defined as in src/java/org/apache/hadoop/mapreduce/MRJobConfig.java: public static final String JOB_CANCEL_DELEGATION_TOKEN = "mapreduce.job.complete.cancel.delegation.tokens"; Also, please link if this JIRA to its blockers if any.
          Hide
          Vinay Kumar Thota added a comment -

          * I think it should be, if the delegation token exists, then it should be set to false. So, should not it be if (props.get(MAPRED_DELEGATION_TOKEN) != null) {

          I think you thought that I am checking the DELEGATION_TOKEN with configuration.However I am not checking with the config. I am checking in the hashtable whether user has set the property are not in the test. If not, I am setting the default value false.

          Show
          Vinay Kumar Thota added a comment - * I think it should be, if the delegation token exists, then it should be set to false. So, should not it be if (props.get(MAPRED_DELEGATION_TOKEN) != null) { I think you thought that I am checking the DELEGATION_TOKEN with configuration.However I am not checking with the config. I am checking in the hashtable whether user has set the property are not in the test. If not, I am setting the default value false.
          Hide
          Iyappan Srinivasan added a comment -

          Ok.

          Show
          Iyappan Srinivasan added a comment - Ok.
          Hide
          Vinay Kumar Thota added a comment -

          the config key is already defined as in mapreduce project(src/java/org/apache/hadoop/mapreduce/MRJobConfig.java). But the patch belongs to common project and it doesn't have defined config key.

          Show
          Vinay Kumar Thota added a comment - the config key is already defined as in mapreduce project(src/java/org/apache/hadoop/mapreduce/MRJobConfig.java). But the patch belongs to common project and it doesn't have defined config key.
          Hide
          Vinay Kumar Thota added a comment -

          the config key is already defined as in mapreduce project(src/java/org/apache/hadoop/mapreduce/MRJobConfig.java)

          The patch belongs to common project and it doesn't have defined config key.

          Show
          Vinay Kumar Thota added a comment - the config key is already defined as in mapreduce project(src/java/org/apache/hadoop/mapreduce/MRJobConfig.java) The patch belongs to common project and it doesn't have defined config key.
          Hide
          Konstantin Boudnik added a comment -

          The patch belongs to common project and it doesn't have defined config key.

          This is an indication why this patch shoudn't go to the Common project then. If a base project has a knowledge about an upstream module - this is plain wrong!
          I missed that during the first review - I think the name of the patch file confused me.

          So, it is big no-no. -1

          Show
          Konstantin Boudnik added a comment - The patch belongs to common project and it doesn't have defined config key. This is an indication why this patch shoudn't go to the Common project then. If a base project has a knowledge about an upstream module - this is plain wrong! I missed that during the first review - I think the name of the patch file confused me. So, it is big no-no. -1
          Hide
          Konstantin Boudnik added a comment -

          Basically, if you need to have this kind of properties for MR - it should go into concrete implementation of a cluster. I believe this is MRCLuster in this case.

          Show
          Konstantin Boudnik added a comment - Basically, if you need to have this kind of properties for MR - it should go into concrete implementation of a cluster. I believe this is MRCLuster in this case.
          Hide
          Vinay Kumar Thota added a comment -

          I totally agreed with you and It's valid. So I have created separate ticket(MAPREDUCE-1962) for that path and remaining thing keeping as it is.

          Show
          Vinay Kumar Thota added a comment - I totally agreed with you and It's valid. So I have created separate ticket( MAPREDUCE-1962 ) for that path and remaining thing keeping as it is.
          Hide
          Vinay Kumar Thota added a comment -

          Excluded the mapreduce related content in the patch.

          Show
          Vinay Kumar Thota added a comment - Excluded the mapreduce related content in the patch.
          Hide
          Balaji Rajagopalan added a comment -

          +1

          Show
          Balaji Rajagopalan added a comment - +1
          Hide
          Konstantin Boudnik added a comment -

          +1 patch looks good.

          Show
          Konstantin Boudnik added a comment - +1 patch looks good.
          Hide
          Hadoop QA added a comment -

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

          +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 appears to have generated 1 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/629/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/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/12450293/HADOOP-6875.patch against trunk revision 966919. +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 appears to have generated 1 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/629/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/629/console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          Going to commit it because all javadoc warnings seem to be coming from security related classes. Current patch is unlikely to introduce any of those.

          Show
          Konstantin Boudnik added a comment - Going to commit it because all javadoc warnings seem to be coming from security related classes. Current patch is unlikely to introduce any of those.
          Hide
          Konstantin Boudnik added a comment -

          I have committed this to the trunk and 0.21 branch. Thanks Vinay.

          Show
          Konstantin Boudnik added a comment - I have committed this to the trunk and 0.21 branch. Thanks Vinay.

            People

            • Assignee:
              Vinay Kumar Thota
              Reporter:
              Vinay Kumar Thota
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development