Hadoop Common
  1. Hadoop Common
  2. HADOOP-5671

DistCp.sameFile(..) should return true if src fs does not support checksum

    Details

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

      Description

      In DistCp.sameFile(..), if src FileSystem does not support file checksum, it should return true.

      1. 5671_20090414.patch
        1 kB
        Tsz Wo Nicholas Sze
      2. 5671_20090415.patch
        1 kB
        Tsz Wo Nicholas Sze

        Activity

        Hide
        Tsz Wo Nicholas Sze added a comment -

        5671_20090414.patch: If src fs does not support file checksum, it may throw a FileNotFoundException. In such case, we should return true.

        Show
        Tsz Wo Nicholas Sze added a comment - 5671_20090414.patch: If src fs does not support file checksum, it may throw a FileNotFoundException. In such case, we should return true.
        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 doesn't appear to include any new or modified tests.
             [exec]                         Please justify why no tests are needed for this patch.
             [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 does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        

        Tested manually, no new tests added.

        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 doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [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 does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. Tested manually, no new tests added.
        Hide
        Suresh Srinivas added a comment -

        Based on the documentation of the method FileSystem.getFileChecksum(), not supporting checksum is indicated by null return and not by throwing exception. So should we ensure the classes extending FileSystem honor this?

        Show
        Suresh Srinivas added a comment - Based on the documentation of the method FileSystem.getFileChecksum() , not supporting checksum is indicated by null return and not by throwing exception. So should we ensure the classes extending FileSystem honor this?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > In DistCp.sameFile(..), if src FileSystem does not support file checksum, it should return true.

        More details of the problem:
        Some previous versions HftpFileSystem do not support file checksum and throw FileNotFoundException since the request url is not found. In such case, DistCp.sameFile(..) should return true.

        5671_20090415.patch: rewritten the comments, no code changes.

        Show
        Tsz Wo Nicholas Sze added a comment - > In DistCp.sameFile(..), if src FileSystem does not support file checksum, it should return true. More details of the problem: Some previous versions HftpFileSystem do not support file checksum and throw FileNotFoundException since the request url is not found. In such case, DistCp.sameFile(..) should return true. 5671_20090415.patch: rewritten the comments, no code changes.
        Hide
        Chris Douglas added a comment -

        +1

        Show
        Chris Douglas added a comment - +1
        Hide
        Chris Douglas added a comment -

        I committed this. Thanks, Nicholas

        Show
        Chris Douglas added a comment - I committed this. Thanks, Nicholas
        Hide
        Koji Noguchi added a comment -

        Thanks Nicholas, Chris, Suresh!

        Since this is sort of a regression for 0.19, can we also commit it to 0.19.* and 0.20.1 ?
        (-update option won't work without this patch when pulling from 0.18 cluster or earlier versions)

        Show
        Koji Noguchi added a comment - Thanks Nicholas, Chris, Suresh! Since this is sort of a regression for 0.19, can we also commit it to 0.19.* and 0.20.1 ? (-update option won't work without this patch when pulling from 0.18 cluster or earlier versions)
        Hide
        dhruba borthakur added a comment -

        Really appreciate it if we can commit it to 0.19 and earlier releases.

        Show
        dhruba borthakur added a comment - Really appreciate it if we can commit it to 0.19 and earlier releases.
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #811 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/811/ )
        Hide
        Chris Douglas added a comment -

        Committed to 0.19.2 and 0.20.1 as well

        Show
        Chris Douglas added a comment - Committed to 0.19.2 and 0.20.1 as well
        Hide
        Hudson added a comment -

        Integrated in Hadoop-trunk #815 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/815/)
        Move from 0.21 to 0.19.2

        Show
        Hudson added a comment - Integrated in Hadoop-trunk #815 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/815/ ) Move from 0.21 to 0.19.2

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development