Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1482

Add listCorruptFileBlocks to DistributedFileSystem (and ClientProtocol)

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      This patch bumps the version number of ClientProtocol to support listCorruptFileBlocks in HDFS.

      Description

      As discussed in HDFS-1111, it would be beneficial for tools such as the RAID block fixer and RAID FSCK to have access to listCorruptFileBlocks via the DistributedFileSystem (rather than having to parse Servlet output, which could present a performance problem).

      1. HDFS-1482.2.patch
        21 kB
        Patrick Kling
      2. HDFS-1482.3.patch
        20 kB
        Patrick Kling
      3. HDFS-1482.5.patch
        19 kB
        Patrick Kling
      4. HDFS-1482.6.patch
        19 kB
        Patrick Kling
      5. HDFS-1482.patch
        20 kB
        Patrick Kling

        Issue Links

          Activity

          Hide
          dhruba borthakur added a comment -

          My vote would be to add this to ClientProtocol and DistributedFileSystem. I would not add this to FileSystem because this API is very DFS-centric. There is precedence of doing that, e.g. DistributedFileSystem.concat().

          Show
          dhruba borthakur added a comment - My vote would be to add this to ClientProtocol and DistributedFileSystem. I would not add this to FileSystem because this API is very DFS-centric. There is precedence of doing that, e.g. DistributedFileSystem.concat().
          Hide
          Patrick Kling 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 10 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 does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system tests framework. The patch passed system tests framework compile.

          Show
          Patrick Kling 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 10 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 does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system tests framework. The patch passed system tests framework compile.
          Hide
          Suresh Srinivas added a comment -

          FileSystem already has methods related to blocks - FileSystem#getFileBlockLocations(). Given that we should add this to FileSystem.

          Are you going to add this change to FileContext as well?

          Show
          Suresh Srinivas added a comment - FileSystem already has methods related to blocks - FileSystem#getFileBlockLocations(). Given that we should add this to FileSystem. Are you going to add this change to FileContext as well?
          Hide
          Patrick Kling added a comment -

          Updated the patch to incorporate Dhruba's feedback on the review board.

          ClientProtocol.listCorruptFileBlocks now returns a list of corrupt file names and a single cookie string used for making iterative calls.

          [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 10 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 does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system tests framework. The patch passed system tests framework compile.

          Show
          Patrick Kling added a comment - Updated the patch to incorporate Dhruba's feedback on the review board. ClientProtocol.listCorruptFileBlocks now returns a list of corrupt file names and a single cookie string used for making iterative calls. [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 10 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 does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system tests framework. The patch passed system tests framework compile.
          Hide
          Patrick Kling added a comment -

          The review board entry for this JIRA can be found at https://reviews.apache.org/r/18/

          Couldn't we add listCorruptFileBlocks as a DFS-only method for now and add it to FileSystem once other file systems support this operation (since it would require a separate JIRA in HADOOP anyway)?

          Show
          Patrick Kling added a comment - The review board entry for this JIRA can be found at https://reviews.apache.org/r/18/ Couldn't we add listCorruptFileBlocks as a DFS-only method for now and add it to FileSystem once other file systems support this operation (since it would require a separate JIRA in HADOOP anyway)?
          Hide
          Patrick Kling added a comment -

          Added listCorruptFileBlocks to FileSystem.

          Show
          Patrick Kling added a comment - Added listCorruptFileBlocks to FileSystem.
          Hide
          Eli Collins added a comment -

          Per Suresh this API needs to be added to FileContext as well.

          Show
          Eli Collins added a comment - Per Suresh this API needs to be added to FileContext as well.
          Hide
          Patrick Kling added a comment -

          Added listCorruptFileBlocks to FileContext.

          Show
          Patrick Kling added a comment - Added listCorruptFileBlocks to FileContext.
          Hide
          Hairong Kuang added a comment -

          This looks good. One minor comment is that lastCookie needs to be updated only once in NameNode.

          Show
          Hairong Kuang added a comment - This looks good. One minor comment is that lastCookie needs to be updated only once in NameNode.
          Hide
          Patrick Kling added a comment -

          Unfortunately, there is no efficient way of getting the last element of a Collection. So updating lastCookie while doing the iteration is our best bet I think.

          Show
          Patrick Kling added a comment - Unfortunately, there is no efficient way of getting the last element of a Collection. So updating lastCookie while doing the iteration is our best bet I think.
          Hide
          Patrick Kling added a comment -

          Fixed javadoc warnings.

          ant test-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 10 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 does not introduce any new Findbugs (version 1.3.9) warnings.
               [exec]
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec]
               [exec]     +1 system test framework.  The patch passed system test framework compile.
          
          Show
          Patrick Kling added a comment - Fixed javadoc warnings. ant test-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 10 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 does not introduce any new Findbugs (version 1.3.9) warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system test framework. The patch passed system test framework compile.
          Hide
          Hairong Kuang added a comment -

          +1. This looks good to me.

          Show
          Hairong Kuang added a comment - +1. This looks good to me.
          Hide
          Hairong Kuang added a comment -

          I've committed this. Thanks, Patrick!

          Show
          Hairong Kuang added a comment - I've committed this. Thanks, Patrick!

            People

            • Assignee:
              Patrick Kling
              Reporter:
              Patrick Kling
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development