Hadoop Common
  1. Hadoop Common
  2. HADOOP-6906

FileContext copy() utility doesn't work with recursive copying of directories.

    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
    1. HADOOP-6906-regression.1.txt
      2 kB
      Vinod Kumar Vavilapalli
    2. HADOOP-6906-20100809.1.txt
      5 kB
      Vinod Kumar Vavilapalli

      Issue Links

        Activity

        Hide
        Vinod Kumar Vavilapalli added a comment -

        Attaching a regression test. Running TestFcLocalFsUtil after applying this patch verifies that a simple recursive copy of a directory from local file system to the same FS fails. I could verify the same failure with HDFS to HDFS copy also.

        Show
        Vinod Kumar Vavilapalli added a comment - Attaching a regression test. Running TestFcLocalFsUtil after applying this patch verifies that a simple recursive copy of a directory from local file system to the same FS fails. I could verify the same failure with HDFS to HDFS copy also.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Attaching a patch to address this issue. Please review this patch. It does the following:

        • Fixes the main bug which incorrectly constructs path names of contents of a directory while copying (FileContext.java +2012 with the patch)
        • Modifies FileContext.isSameFS() to avoid null errors when using file:/// URIs which don't have an authority part.
        • LocalFs extends ChecksumFs and has a RawLocalFs object. RawLocalFs is backed by RawLocalFileSystem which doesn't know about .crc files. Because of this, files written via LocalFs automatically write .crc files transparently, but listing of files in a LocalFs doesn't hide .crc files. The modified test fails because of this. I changed ChecksumFs to override listStatus() and swallow .crc files. Please correct me if I am wrong.
        Show
        Vinod Kumar Vavilapalli added a comment - Attaching a patch to address this issue. Please review this patch. It does the following: Fixes the main bug which incorrectly constructs path names of contents of a directory while copying (FileContext.java +2012 with the patch) Modifies FileContext.isSameFS() to avoid null errors when using file:/// URIs which don't have an authority part. LocalFs extends ChecksumFs and has a RawLocalFs object. RawLocalFs is backed by RawLocalFileSystem which doesn't know about .crc files. Because of this, files written via LocalFs automatically write .crc files transparently, but listing of files in a LocalFs doesn't hide .crc files. The modified test fails because of this. I changed ChecksumFs to override listStatus() and swallow .crc files. Please correct me if I am wrong.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        The problem with .crc files indeed is a valid one - HADOOP-6872.

        Show
        Vinod Kumar Vavilapalli added a comment - The problem with .crc files indeed is a valid one - HADOOP-6872 .
        Hide
        Bern Hassmo added a comment -

        Regarding HADOOP-6872 - just a quick question: Why not provide a listStatus(Path,Filter) method in FilterFs and do the filtering there? Sounds more natural to me, since it is already a filtering fs. Other subclasses of FilterFs might benefit from this as well. Minor detail, just curious why it is in ChecksumFS now.

        Show
        Bern Hassmo added a comment - Regarding HADOOP-6872 - just a quick question: Why not provide a listStatus(Path,Filter) method in FilterFs and do the filtering there? Sounds more natural to me, since it is already a filtering fs. Other subclasses of FilterFs might benefit from this as well. Minor detail, just curious why it is in ChecksumFS now.
        Hide
        Hairong Kuang added a comment -

        I think that AbstractFileSystem should have an API listStatus(Path, PathFilter).

        Show
        Hairong Kuang added a comment - I think that AbstractFileSystem should have an API listStatus(Path, PathFilter).
        Hide
        Mahadev konar added a comment -

        +1 for the fix. as per the listStatus(Path, PathFilter) api, it would be good to have it, but again this fix should neways be there.

        Show
        Mahadev konar added a comment - +1 for the fix. as per the listStatus(Path, PathFilter) api, it would be good to have it, but again this fix should neways be there.
        Hide
        Mahadev konar added a comment -

        I just committed this.

        thanks vinod!

        Show
        Mahadev konar added a comment - I just committed this. thanks vinod!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #362 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/362/)
        HADOOP-6906. FileContext copy() utility doesn't work with recursive copying of directories. (vinod k v via mahadev)

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #362 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/362/ ) HADOOP-6906 . FileContext copy() utility doesn't work with recursive copying of directories. (vinod k v via mahadev)
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk #429 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/429/)
        HADOOP-6906. FileContext copy() utility doesn't work with recursive copying of directories. (vinod k v via mahadev)

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk #429 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/429/ ) HADOOP-6906 . FileContext copy() utility doesn't work with recursive copying of directories. (vinod k v via mahadev)
        Hide
        Mahadev konar added a comment -

        sorry I forgot to mention I ran ant test and
        and test-patch. They both pass!

        Show
        Mahadev konar added a comment - sorry I forgot to mention I ran ant test and and test-patch. They both pass!

          People

          • Assignee:
            Vinod Kumar Vavilapalli
            Reporter:
            Vinod Kumar Vavilapalli
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development