Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-581

Introduce an iterator over blocks in the block report array.

    Details

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

      Description

      A block iterator will hide the internal implementation of the block report as an array of three longs per block.

      1. BlockReportIterator.patch
        5 kB
        Konstantin Shvachko
      2. BlockReportIterator.patch
        5 kB
        Konstantin Shvachko

        Issue Links

          Activity

          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have accidentally posted my HDFS-570 comment here Sorry.

          Show
          Tsz Wo Nicholas Sze added a comment - I have accidentally posted my HDFS-570 comment here Sorry.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          ClientDatanodeProtocol
          2. JavaDoc for getReplicaVisibleLength() is confusing. Could you please also make it 3 lines rather than 1.

          Why 3 lines?

          LocatedBlock
          3. Does not need any of the changes.

          Could you explain more?

          LocatedBlocks
          4. Here you do multiple field and method renames, combined with reformatting. I am lost.

          How can I help you?

          FSDatasetInterface
          5. Why do you need to abstract getReplicaInfo()? It does not seem that SimulatedFSDataset actually need it anywhere, at least not yet.

          FSDatasetInterface is an interface. By definition, all methods in an interface must be abstract.

          BlockManager
          6. You factored out a part of the code into a new method. I cannot see what the new changes are.

          The new method is involved in FSNamesystem. Could you take a look again?

          INodeFile
          8. It is not necessary to remove public from method declaration and remove unused method.

          I remove the method because I see the following comment in the code.

          // SHV !!! this is not used anywhere - remove
          

          The comment was introduced by you in HDFS-517. Could you explain what does it mean?

          Show
          Tsz Wo Nicholas Sze added a comment - ClientDatanodeProtocol 2. JavaDoc for getReplicaVisibleLength() is confusing. Could you please also make it 3 lines rather than 1. Why 3 lines? LocatedBlock 3. Does not need any of the changes. Could you explain more? LocatedBlocks 4. Here you do multiple field and method renames, combined with reformatting. I am lost. How can I help you? FSDatasetInterface 5. Why do you need to abstract getReplicaInfo()? It does not seem that SimulatedFSDataset actually need it anywhere, at least not yet. FSDatasetInterface is an interface . By definition, all methods in an interface must be abstract. BlockManager 6. You factored out a part of the code into a new method. I cannot see what the new changes are. The new method is involved in FSNamesystem. Could you take a look again? INodeFile 8. It is not necessary to remove public from method declaration and remove unused method. I remove the method because I see the following comment in the code. // SHV !!! this is not used anywhere - remove The comment was introduced by you in HDFS-517 . Could you explain what does it mean?
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #70 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/70/)
          . Introduce an iterator over blocks in the block report array. Contributed by Konstantin Shvachko.

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #70 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/70/ ) . Introduce an iterator over blocks in the block report array. Contributed by Konstantin Shvachko.
          Hide
          Hudson added a comment -

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

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

          Integrated in Hadoop-Hdfs-trunk-Commit #9 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/9/)
          . Introduce an iterator over blocks in the block report array. Contributed by Konstantin Shvachko.

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #9 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/9/ ) . Introduce an iterator over blocks in the block report array. Contributed by Konstantin Shvachko.
          Konstantin Shvachko made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Resolution Fixed [ 1 ]
          Hide
          Konstantin Shvachko added a comment -

          I just committed this.

          Show
          Konstantin Shvachko added a comment - I just committed this.
          Hide
          Sanjay Radia added a comment -

          +1

          Show
          Sanjay Radia added a comment - +1
          Konstantin Shvachko made changes -
          Attachment BlockReportIterator.patch [ 12418198 ]
          Hide
          Konstantin Shvachko added a comment -

          A minor improvement. The patch introduces the default constructor BlockListAsLongs() and calls it in reportDiff().
          New tests are not required as this does not introduce any new features.

          Show
          Konstantin Shvachko added a comment - A minor improvement. The patch introduces the default constructor BlockListAsLongs() and calls it in reportDiff() . New tests are not required as this does not introduce any new features.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          +1 javac. The applied patch does not increase the total number of javac compiler 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 passed 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/3/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/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/12418179/BlockReportIterator.patch against trunk revision 809439. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler 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 passed 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/3/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/3/console This message is automatically generated.
          Konstantin Shvachko made changes -
          Link This issue blocks HDFS-576 [ HDFS-576 ]
          Konstantin Shvachko made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Konstantin Shvachko made changes -
          Field Original Value New Value
          Attachment BlockReportIterator.patch [ 12418179 ]
          Hide
          Konstantin Shvachko added a comment -

          This patch introduces BlockReportIterator in BlockListAs Longs and deprecates previously publick getters of block id, length, and generation stamp of blocks by the index. The iterator simplifies code in processing the report on the name-node.

          Show
          Konstantin Shvachko added a comment - This patch introduces BlockReportIterator in BlockListAs Longs and deprecates previously publick getters of block id, length, and generation stamp of blocks by the index. The iterator simplifies code in processing the report on the name-node.
          Konstantin Shvachko created issue -

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development