Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-680

Add new access method to a copy of a block's replica

    Details

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

      Description

      Sometimes tests (and perhaps other client applications) might need an access to the state of a block's replica.
      Currently it is accessible through FSDataset member of DataNode class. However, FSDataset is an internal matter of DataNode and shouldn't be available externally.

      This JIRA will provide new getter method which will be returning a copy of a block's replica instead of a reference to an actual replica.

      1. replicaClone.patch
        11 kB
        Konstantin Shvachko
      2. replicaClone.patch
        11 kB
        Konstantin Shvachko

        Issue Links

          Activity

          Hide
          Konstantin Shvachko added a comment -

          This patch

          • introduces a new method FSDataset.getReplicaClone(), which find a replica in data-node's memory, clones it and returns the clone.
          • This is done using copy constructors on different replica types. I considered using Object.clone() in the beginning, but rejected it because clone returns Object, and we will have to convert every time we use it. Doesn't look appealing to me.
          • I removed all use cases of FSDataset.getReplica() but one in BlockSender. This should be fixed by HDFS-698. I deprecated getReplica() for now.
          • I was not sure whether getReplicaClone() belongs to FSDataset or DataNode. If people think it should be in DataNode I can move it, let me know.
          Show
          Konstantin Shvachko added a comment - This patch introduces a new method FSDataset.getReplicaClone() , which find a replica in data-node's memory, clones it and returns the clone. This is done using copy constructors on different replica types. I considered using Object.clone() in the beginning, but rejected it because clone returns Object, and we will have to convert every time we use it. Doesn't look appealing to me. I removed all use cases of FSDataset.getReplica() but one in BlockSender . This should be fixed by HDFS-698 . I deprecated getReplica() for now. I was not sure whether getReplicaClone() belongs to FSDataset or DataNode . If people think it should be in DataNode I can move it, let me know.
          Hide
          Konstantin Boudnik added a comment -

          Other Konstantin is working on this

          Show
          Konstantin Boudnik added a comment - Other Konstantin is working on this
          Hide
          Konstantin Boudnik added a comment -

          As we have discussed off-line having a true Replica's clone might help to avoid copy-constructor implementations for all sub-classes. Although, cloning doesn't work very well in this case as in 'ugly'.

          Patch looks fine.

          Nits:

          • line 92:93 seems like a white-space mod
          • line 83:84 has changed the formatting

          +1 other than that!

          Show
          Konstantin Boudnik added a comment - As we have discussed off-line having a true Replica's clone might help to avoid copy-constructor implementations for all sub-classes. Although, cloning doesn't work very well in this case as in 'ugly'. Patch looks fine. Nits: line 92:93 seems like a white-space mod line 83:84 has changed the formatting +1 other than that!
          Hide
          Konstantin Shvachko added a comment -

          > line 83:84 has changed the formatting
          Yes, this fixes wrong alignment in this line.
          > line 92:93 seems like a white-space mod
          Removed spaces in an empty line, could not resist.

          Show
          Konstantin Shvachko added a comment - > line 83:84 has changed the formatting Yes, this fixes wrong alignment in this line. > line 92:93 seems like a white-space mod Removed spaces in an empty line, could not resist.
          Hide
          Konstantin Shvachko added a comment -
          • merged with trunk
          • renamed getReplicaClone() to fetchReplicaInfo()
          • now fetchReplicaInfo() is mostly used for checking whether it exists or not.
          • Except for TestInterDatanodeProtocol, in which fetching the replica can be avoided all together because it is needed for FSDataset.checkReplicaFiles(replica), and this one seems redundant as the same method is called in updateReplicaUnderRecovery() before and after replica update.
          Show
          Konstantin Shvachko added a comment - merged with trunk renamed getReplicaClone() to fetchReplicaInfo() now fetchReplicaInfo() is mostly used for checking whether it exists or not. Except for TestInterDatanodeProtocol, in which fetching the replica can be avoided all together because it is needed for FSDataset.checkReplicaFiles(replica), and this one seems redundant as the same method is called in updateReplicaUnderRecovery() before and after replica update.
          Hide
          Konstantin Boudnik added a comment -

          +1 looks good

          Show
          Konstantin Boudnik added a comment - +1 looks good
          Hide
          Konstantin Shvachko added a comment -

          test-core passed. test-patch has one javac warning. This is related to the use of now deprecated getReplica() in BlockSender. HDFS-698 is supposed to fix that.

               [exec] There appear to be 108 release audit warnings before the patch and 108 release audit warnings after applying the patch.
               [exec] -1 overall.  
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec]     +1 tests included.  The patch appears to include 9 new or modified tests.
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec]     -1 javac.  The applied patch generated 111 javac compiler warnings (more than the trunk's current 110 warnings).
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec] ======================================================================
               [exec] ======================================================================
               [exec]     Finished build.
               [exec] ======================================================================
               [exec] ======================================================================
          
          Show
          Konstantin Shvachko added a comment - test-core passed. test-patch has one javac warning. This is related to the use of now deprecated getReplica() in BlockSender. HDFS-698 is supposed to fix that. [exec] There appear to be 108 release audit warnings before the patch and 108 release audit warnings after applying the patch. [exec] -1 overall. [exec] +1 @author. The patch does not contain any @author tags. [exec] +1 tests included. The patch appears to include 9 new or modified tests. [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] -1 javac. The applied patch generated 111 javac compiler warnings (more than the trunk's current 110 warnings). [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] ====================================================================== [exec] ====================================================================== [exec] Finished build. [exec] ====================================================================== [exec] ======================================================================
          Hide
          Konstantin Shvachko added a comment -

          I just committed this.

          Show
          Konstantin Shvachko added a comment - I just committed this.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12422055/replicaClone.patch
          against trunk revision 824984.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 javac. The applied patch generated 19 javac compiler warnings (more than the trunk's current 18 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 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12422055/replicaClone.patch against trunk revision 824984. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 19 javac compiler warnings (more than the trunk's current 18 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 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/64/console This message is automatically generated.
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #47 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/47/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #47 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/47/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #79 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/79/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #79 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/79/ )
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/ )

            People

            • Assignee:
              Konstantin Shvachko
              Reporter:
              Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development