Hadoop Common
  1. Hadoop Common
  2. HADOOP-7006

hadoop fs -getmerge does not work using codebase from trunk.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Running the codebase from trunk, the hadoop fs -getmerge command does not work. As implemented in prior versions (i.e. 0.20.2), I could run hadoop fs -getmerge pointed at a directory containing multiple files. It would merge all files into a single file on the local file system. Running the same command using the codebase from trunk, it looks like nothing happens.

      1. HADOOP-7006.patch
        6 kB
        Chris Nauroth
      2. HADOOP-7006.patch
        6 kB
        Doug Cutting

        Activity

        Konstantin Shvachko made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk #494 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/494/)
        HADOOP-7006. Fix 'fs -getmerge' command to not be a no-op. Contributed by Chris Nauroth.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk #494 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/494/ ) HADOOP-7006 . Fix 'fs -getmerge' command to not be a no-op. Contributed by Chris Nauroth.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #402 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/402/)
        HADOOP-7006. Fix 'fs -getmerge' command to not be a no-op. Contributed by Chris Nauroth.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #402 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/402/ ) HADOOP-7006 . Fix 'fs -getmerge' command to not be a no-op. Contributed by Chris Nauroth.
        Doug Cutting made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Doug Cutting added a comment -

        I just committed this. Thanks, Chris!

        Show
        Doug Cutting added a comment - I just committed this. Thanks, Chris!
        Hide
        Doug Cutting added a comment -

        Ran test-patch by hand.

        -1 overall.  
        
            +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 system tests framework.  The patch passed system tests framework compile.
        

        The javadoc test seems broken, as it fails even when I run test-patch with an empty patch file.

        Show
        Doug Cutting added a comment - Ran test-patch by hand. -1 overall. +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 system tests framework. The patch passed system tests framework compile. The javadoc test seems broken, as it fails even when I run test-patch with an empty patch file.
        Hide
        Aaron T. Myers added a comment -

        Doug already marked the ticket patch available.

        Not quite sure why Hudson hasn't picked it up yet - might just be taking some time to get around to it (the HDFS tests take quite a while to run.)

        Show
        Aaron T. Myers added a comment - Doug already marked the ticket patch available. Not quite sure why Hudson hasn't picked it up yet - might just be taking some time to get around to it (the HDFS tests take quite a while to run.)
        Hide
        Chris Nauroth added a comment -

        Thanks, Aaron and Doug.

        I'm following the directions from http://wiki.apache.org/hadoop/HowToContribute . I don't see the "Submit Patch" link to trigger Hudson though. Is there something else that I need to do?

        Show
        Chris Nauroth added a comment - Thanks, Aaron and Doug. I'm following the directions from http://wiki.apache.org/hadoop/HowToContribute . I don't see the "Submit Patch" link to trigger Hudson though. Is there something else that I need to do?
        Doug Cutting made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hadoop Flags [Reviewed]
        Assignee Chris Nauroth [ cnauroth ]
        Doug Cutting made changes -
        Attachment HADOOP-7006.patch [ 12457995 ]
        Hide
        Doug Cutting added a comment -

        Tests failed for me until I also changed the file merge to sort the files. This is probably a good idea anyway. Here's a new version of the patch with that change.

        Show
        Doug Cutting added a comment - Tests failed for me until I also changed the file merge to sort the files. This is probably a good idea anyway. Here's a new version of the patch with that change.
        Hide
        Aaron T. Myers added a comment -

        I've reviewed the patch, and it looks good to me.

        Show
        Aaron T. Myers added a comment - I've reviewed the patch, and it looks good to me.
        Chris Nauroth made changes -
        Field Original Value New Value
        Attachment HADOOP-7006.patch [ 12457950 ]
        Hide
        Chris Nauroth added a comment -

        I've attached the patch. Could I please have a code review? Thank you.

        Show
        Chris Nauroth added a comment - I've attached the patch. Could I please have a code review? Thank you.
        Hide
        Chris Nauroth added a comment -

        It appears that this was introduced with changeset 949658. Here is a partial diff of org.apache.hadoop.fs.FileUtil:

        @@ -258,7 +258,7 @@
                                           Configuration conf, String addString) throws IOException {
             dstFile = checkDest(srcDir.getName(), dstFS, dstFile, false);
         
        -    if (!srcFS.getFileStatus(srcDir).isDir())
        +    if (srcFS.getFileStatus(srcDir).isDirectory())
               return false;
        

        Notice that in addition to switching from isDir() to isDirectory(), this change also dropped the negation on the front of the condition. I'll attach a simple one-line patch to restore functionality. I've also added a unit test to cover the FileUtil.copyMerge API. I'd like to volunteer for HADOOP-6387 next, and I want this unit test in place as a regression test while I work on that.

        Show
        Chris Nauroth added a comment - It appears that this was introduced with changeset 949658. Here is a partial diff of org.apache.hadoop.fs.FileUtil: @@ -258,7 +258,7 @@ Configuration conf, String addString) throws IOException { dstFile = checkDest(srcDir.getName(), dstFS, dstFile, false); - if (!srcFS.getFileStatus(srcDir).isDir()) + if (srcFS.getFileStatus(srcDir).isDirectory()) return false; Notice that in addition to switching from isDir() to isDirectory(), this change also dropped the negation on the front of the condition. I'll attach a simple one-line patch to restore functionality. I've also added a unit test to cover the FileUtil.copyMerge API. I'd like to volunteer for HADOOP-6387 next, and I want this unit test in place as a regression test while I work on that.
        Chris Nauroth created issue -

          People

          • Assignee:
            Chris Nauroth
            Reporter:
            Chris Nauroth
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development