Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-5081

Backport DistCpV2 and the related JIRAs to branch-1

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major 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_20130321.patch
        386 kB
        Tsz Wo Nicholas Sze
      2. m5981_20130321b.patch
        386 kB
        Tsz Wo Nicholas Sze
      3. m5981_20130323.patch
        391 kB
        Tsz Wo Nicholas Sze
      4. DistCp.java.diff
        4 kB
        Tsz Wo Nicholas Sze
      5. m5081_20130328.patch
        383 kB
        Tsz Wo Nicholas Sze
      6. m5081_20130328b.patch
        383 kB
        Tsz Wo Nicholas Sze

        Issue Links

          Activity

          Hide
          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
          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.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Sure, MAPREDUCE-5075 is a useful bug fix.

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

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

          Show
          Tsz Wo Nicholas Sze added a comment - m5981_20130321.patch: backports all JIRAs except MAPREDUCE-5014 .
          Hide
          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
          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
          Tsz Wo Nicholas Sze added a comment -

          Two of the findbugs warnings are related.

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

          Show
          Tsz Wo Nicholas Sze added a comment - Two of the findbugs warnings are related. m5981_20130321b.patch: backports distcp part of HADOOP-8341 .
          Hide
          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
          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
          Tsz Wo Nicholas Sze added a comment -

          All tests passed with the patch in my machine.

          Show
          Tsz Wo Nicholas Sze added a comment - All tests passed with the patch in my machine.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          MAPREDUCE-5014 was committed recently; revised description.

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

          m5981_20130323.patch: includes MAPREDUCE-5014.

          Show
          Tsz Wo Nicholas Sze added a comment - m5981_20130323.patch: includes MAPREDUCE-5014 .
          Hide
          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
          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
          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
          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
          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 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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Tsz Wo Nicholas Sze added a comment -

          Thanks Suresh for reviewing the patch.

          I have committed this.

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

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development