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

Backport DistCpV2 and the related JIRAs to branch-1

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2.0
    • Component/s: distcp
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Here is a list of DistCpV2 JIRAs:

      1. m5981_20130323.patch
        391 kB
        Tsz Wo Nicholas Sze
      2. m5981_20130321b.patch
        386 kB
        Tsz Wo Nicholas Sze
      3. m5981_20130321.patch
        386 kB
        Tsz Wo Nicholas Sze
      4. m5081_20130328b.patch
        383 kB
        Tsz Wo Nicholas Sze
      5. m5081_20130328.patch
        383 kB
        Tsz Wo Nicholas Sze
      6. DistCp.java.diff
        4 kB
        Tsz Wo Nicholas Sze

        Issue Links

          Activity

          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          By hdfs2, do you mean hdfs in branch-2? The DistCpV2 hers is for branch-1. You may try it in your setup.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - By hdfs2, do you mean hdfs in branch-2? The DistCpV2 hers is for branch-1. You may try it in your setup.
          Hide
          tychang Tianying Chang added a comment -

          Tsz Wo Nicholas Sze is this distcpV2 available if I am using hdfs2 with Mapreduce1? It seems it has a API break change on this API: org.apache.hadoop.mapreduce.JobSubmissionFiles.getStagingDir(Lorg/apache/hadoop/mapreduce/Cluster;Lorg/apache/hadoop/conf/Configuration BTW, we are using Hadoop 2.0.0-cdh4.2.0

          Show
          tychang Tianying Chang added a comment - Tsz Wo Nicholas Sze is this distcpV2 available if I am using hdfs2 with Mapreduce1? It seems it has a API break change on this API: org.apache.hadoop.mapreduce.JobSubmissionFiles.getStagingDir(Lorg/apache/hadoop/mapreduce/Cluster;Lorg/apache/hadoop/conf/Configuration BTW, we are using Hadoop 2.0.0-cdh4.2.0
          Hide
          mattf Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          mattf Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Thanks Suresh for reviewing the patch.

          I have committed this.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Thanks Suresh for reviewing the patch. I have committed this.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Oops, I accidentally added a new vaidya entry to site.xml.

               <distcp         label="DistCp"  href="distcp.html" />
          +    <distcp2        label="DistCp Version 2"  href="distcp2.html" />
          +    <vaidya         label="Vaidya"  href="vaidya.html"/>
               <vaidya         label="Vaidya"  href="vaidya.html"/>
          

          m5081_20130328b.patch: removes the new vaidya entry.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Oops, I accidentally added a new vaidya entry to site.xml. <distcp label= "DistCp" href= "distcp.html" /> + <distcp2 label= "DistCp Version 2" href= "distcp2.html" /> + <vaidya label= "Vaidya" href= "vaidya.html" /> <vaidya label= "Vaidya" href= "vaidya.html" /> m5081_20130328b.patch: removes the new vaidya entry.
          Hide
          sureshms Suresh Srinivas added a comment -

          Are you sure that you diff the correct file since there two DistCp.java files? Let me post the diff.

          Sorry, I must have compared the wrong files.

          In branch-1, other xml files like core-default.xml and hdfs-default.xml also do not have license header. So let's don't fix the new xml here and fix them with other xml files together later.

          Sounds good.

          +1 for the patch.

          Show
          sureshms Suresh Srinivas added a comment - Are you sure that you diff the correct file since there two DistCp.java files? Let me post the diff. Sorry, I must have compared the wrong files. In branch-1, other xml files like core-default.xml and hdfs-default.xml also do not have license header. So let's don't fix the new xml here and fix them with other xml files together later. Sounds good. +1 for the patch.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          m5081_20130328.patch:

          • keeps distcp (version 1) unchanged and adds a new distcp2 command;
          • also updates the doc.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - m5081_20130328.patch: keeps distcp (version 1) unchanged and adds a new distcp2 command; also updates the doc.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          > 1. sslConfig.xml, distcp-default.xml is missing Apache license header.

          Will do.

          In branch-1, other xml files like core-default.xml and hdfs-default.xml also do not have license header. So let's don't fix the new xml here and fix them with other xml files together later.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - > 1. sslConfig.xml, distcp-default.xml is missing Apache license header. Will do. In branch-1, other xml files like core-default.xml and hdfs-default.xml also do not have license header. So let's don't fix the new xml here and fix them with other xml files together later.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          DistCp.java.diff: diff between branch-1 tools/distcp2/DistCp.java (not tools/DistCp.java) and trunk tools/DistCp.java

          diff b-1/src/tools/org/apache/hadoop/tools/distcp2/DistCp.java t3/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCp.java > DistCp.java.diff

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - DistCp.java.diff: diff between branch-1 tools/distcp2/DistCp.java (not tools/DistCp.java) and trunk tools/DistCp.java diff b-1/src/tools/org/apache/hadoop/tools/distcp2/DistCp.java t3/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCp.java > DistCp.java.diff
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          > 1. sslConfig.xml, distcp-default.xml is missing Apache license header.

          Will do.

          > 2. There is a lot difference in DistCp.java in trunk and DistCp.java in this patch?

          Are you sure that you diff the correct file since there two DistCp.java files? Let me post the diff.

          > 3. ... Why not leave the old distcp as is and add a new command for distcp2?

          Sure, I can add a new command for distcp2.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - > 1. sslConfig.xml, distcp-default.xml is missing Apache license header. Will do. > 2. There is a lot difference in DistCp.java in trunk and DistCp.java in this patch? Are you sure that you diff the correct file since there two DistCp.java files? Let me post the diff. > 3. ... Why not leave the old distcp as is and add a new command for distcp2? Sure, I can add a new command for distcp2.
          Hide
          sanjay.radia Sanjay Radia added a comment -

          Why not leave the old distcp as is and add a new command for distcp2?

          Agree this is safer.
          Only challenge is whether we will carry distcp1 onto Hadoop 2 line for compatibility? We could take it forward and mark it deprecated and remove in a later release.

          Show
          sanjay.radia Sanjay Radia added a comment - Why not leave the old distcp as is and add a new command for distcp2? Agree this is safer. Only challenge is whether we will carry distcp1 onto Hadoop 2 line for compatibility? We could take it forward and mark it deprecated and remove in a later release.
          Hide
          sureshms Suresh Srinivas added a comment -

          This is a large patch. All the new code that is added in this patch, I compared with the trunk version of the files and found very little changes. These changes are mainly related method changes and going back to old ways of doing things.

          Comments:

          1. sslConfig.xml, distcp-default.xml is missing Apache license header.
          2. There is a lot difference in DistCp.java in trunk and DistCp.java in this patch?
          3. I have concerns about replacing existing distcp with new distcp2. Would this have some behavior that could make the tool behave in a way that could be deemed incompatible by existing users. Why not leave the old distcp as is and add a new command for distcp2?
          Show
          sureshms Suresh Srinivas added a comment - This is a large patch. All the new code that is added in this patch, I compared with the trunk version of the files and found very little changes. These changes are mainly related method changes and going back to old ways of doing things. Comments: sslConfig.xml, distcp-default.xml is missing Apache license header. There is a lot difference in DistCp.java in trunk and DistCp.java in this patch? I have concerns about replacing existing distcp with new distcp2. Would this have some behavior that could make the tool behave in a way that could be deemed incompatible by existing users. Why not leave the old distcp as is and add a new command for distcp2?
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          For m5981_20130323.patch:

               [exec] -1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 41 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 appears to introduce 17 new Findbugs (version 1.3.9) warnings.
          
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - For m5981_20130323.patch: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 41 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 appears to introduce 17 new Findbugs (version 1.3.9) warnings.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          m5981_20130323.patch: includes MAPREDUCE-5014.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - m5981_20130323.patch: includes MAPREDUCE-5014 .
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          MAPREDUCE-5014 was committed recently; revised description.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - MAPREDUCE-5014 was committed recently; revised description.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          All tests passed with the patch in my machine.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - All tests passed with the patch in my machine.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -
               [exec] -1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 41 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 appears to introduce 17 new Findbugs (version 1.3.9) warnings.
          

          The remaining findbugs warnings are not related to this.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 41 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 appears to introduce 17 new Findbugs (version 1.3.9) warnings. The remaining findbugs warnings are not related to this.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Two of the findbugs warnings are related.

          m5981_20130321b.patch: backports distcp part of HADOOP-8341.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Two of the findbugs warnings are related. m5981_20130321b.patch: backports distcp part of HADOOP-8341 .
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -
               [exec] -1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 41 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 appears to introduce 19 new Findbugs (version 1.3.9) warnings.
          
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 41 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 appears to introduce 19 new Findbugs (version 1.3.9) warnings.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          m5981_20130321.patch: backports all JIRAs except MAPREDUCE-5014.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - m5981_20130321.patch: backports all JIRAs except MAPREDUCE-5014 .
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Sure, MAPREDUCE-5075 is a useful bug fix.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Sure, MAPREDUCE-5075 is a useful bug fix.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi, Nicholas. You may also want to include MAPREDUCE-5075 in this list, which fixes a file handle leak in DistCp on trunk. I've uploaded a patch on that jira, which is waiting for review and commit.

          Show
          cnauroth Chris Nauroth added a comment - Hi, Nicholas. You may also want to include MAPREDUCE-5075 in this list, which fixes a file handle leak in DistCp on trunk. I've uploaded a patch on that jira, which is waiting for review and commit.

            People

            • Assignee:
              szetszwo Tsz Wo Nicholas Sze
              Reporter:
              szetszwo Tsz Wo Nicholas Sze
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development