Hadoop Common
  1. Hadoop Common
  2. HADOOP-4176

Implement getFileChecksum(Path) in HftpFileSystem

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.19.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Implemented getFileChecksum(Path) in HftpFileSystemfor distcp support.

      Description

      In order to use FileChecksum in DistCp, we should implement getFileChecksum(Path) in HftpFileSystem

      1. 4176_20080917.patch
        14 kB
        Tsz Wo Nicholas Sze
      2. 4176_20080918.patch
        20 kB
        Tsz Wo Nicholas Sze
      3. 4176_20080918b.patch
        20 kB
        Tsz Wo Nicholas Sze
      4. 4176_20080918c.patch
        21 kB
        Tsz Wo Nicholas Sze
      5. 4176_20080919.patch
        20 kB
        Tsz Wo Nicholas Sze

        Issue Links

          Activity

          Hide
          Hudson added a comment -
          Show
          Hudson added a comment - Integrated in Hadoop-trunk #611 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/611/ )
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I just committed this.

          Show
          Tsz Wo Nicholas Sze added a comment - I just committed this.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I tested the patch locally. It only failed on TestLimitTasksPerJobTaskScheduler, which is not related to the patch. See HADOOP-4213.

          Show
          Tsz Wo Nicholas Sze added a comment - I tested the patch locally. It only failed on TestLimitTasksPerJobTaskScheduler, which is not related to the patch. See HADOOP-4213 .
          Hide
          Tsz Wo Nicholas Sze added a comment -

          4176_20080919.patch: I should set the conf value in the test but not change MiniDFSCluster. Otherwise, other tests will fail.

          Show
          Tsz Wo Nicholas Sze added a comment - 4176_20080919.patch: I should set the conf value in the test but not change MiniDFSCluster. Otherwise, other tests will fail.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          4176_20080918c.patch: fixed a bug in the test.

          Show
          Tsz Wo Nicholas Sze added a comment - 4176_20080918c.patch: fixed a bug in the test.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Submitting...

          Show
          Tsz Wo Nicholas Sze added a comment - Submitting...
          Hide
          Chris Douglas added a comment -

          Looks good; +1, assuming tests all pass.

          Show
          Chris Douglas added a comment - Looks good; +1, assuming tests all pass.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          4176_20080918b.patch: The Servlet classes should be static.

          Forgot to upload the latest patch. The previous test-patch results was run on this patch

          Show
          Tsz Wo Nicholas Sze added a comment - 4176_20080918b.patch: The Servlet classes should be static. Forgot to upload the latest patch. The previous test-patch results was run on this patch
          Hide
          Tsz Wo Nicholas Sze added a comment -
               [exec] +1 overall.  
          
               [exec]     +1 @author.  The patch does not contain any @author tags.
          
               [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
          
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
          
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
          
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
          

          Testing it locally.

          Show
          Tsz Wo Nicholas Sze added a comment - [exec] +1 overall. [exec] +1 @author. The patch does not contain any @author tags. [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. Testing it locally.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > Would you expect the load on the namenode to be significant for large numbers of checksum requests? Might it make sense to redirect this request to a datanode that can compute and return the checksum?

          Yes, we should do URL redirect.

          4176_20080918.patch: Incorporated all Chris's comments.

          Show
          Tsz Wo Nicholas Sze added a comment - > Would you expect the load on the namenode to be significant for large numbers of checksum requests? Might it make sense to redirect this request to a datanode that can compute and return the checksum? Yes, we should do URL redirect. 4176_20080918.patch: Incorporated all Chris's comments.
          Hide
          Chris Douglas added a comment -

          This looks good. Only a couple minor suggestions in DFSClient:

          • Since callGetBlockLocations is private, there's no compatibility issue and its signature can be changed and all the calls updated instead of redirecting again.
          • In the javadoc for getFileChecksum, @see is more standard than "Similar to..."

          Would you expect the load on the namenode to be significant for large numbers of checksum requests? Might it make sense to redirect this request to a datanode that can compute and return the checksum?

          Show
          Chris Douglas added a comment - This looks good. Only a couple minor suggestions in DFSClient: Since callGetBlockLocations is private, there's no compatibility issue and its signature can be changed and all the calls updated instead of redirecting again. In the javadoc for getFileChecksum, @see is more standard than "Similar to..." Would you expect the load on the namenode to be significant for large numbers of checksum requests? Might it make sense to redirect this request to a datanode that can compute and return the checksum?
          Hide
          Tsz Wo Nicholas Sze added a comment -

          4176_20080917.patch: first complete patch for reviewing.

          Show
          Tsz Wo Nicholas Sze added a comment - 4176_20080917.patch: first complete patch for reviewing.

            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