Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. h2228_20110805.patch
      91 kB
      Tsz Wo Nicholas Sze
    2. h2228_20110805b.patch
      91 kB
      Tsz Wo Nicholas Sze
    3. h2228_20110805c.patch
      93 kB
      Tsz Wo Nicholas Sze
    4. h2228_20110808.patch
      109 kB
      Tsz Wo Nicholas Sze

      Activity

      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk #743 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/743/)
      HDFS-2228. Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo)

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899
      Files :

      • /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #743 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/743/ ) HDFS-2228 . Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo) szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899 Files : /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk-Commit #816 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/816/)
      HDFS-2228. Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo)

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899
      Files :

      • /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #816 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/816/ ) HDFS-2228 . Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo) szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899 Files : /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Common-trunk-Commit #710 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/710/)
      HDFS-2228. Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo)

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899
      Files :

      • /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Show
      Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #710 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/710/ ) HDFS-2228 . Move block and datanode code from FSNamesystem to BlockManager and DatanodeManager. (szetszwo) szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1154899 Files : /hadoop/common/trunk/hdfs/src/test/unit/org/apache/hadoop/hdfs/server/namenode/TestNNLeaseRecovery.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureReporting.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/DFSTestUtil.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FsckServlet.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestFileCorruption.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/TestBlocksScheduledCounter.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfo.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NamenodeJspHelper.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestNamenodeCapacityReport.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormat.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlocksMap.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestOverReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDataNodeVolumeFailureToleration.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
      Hide
      Tsz Wo Nicholas Sze added a comment -

      I have committed this.

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

      Passed all tests except TestHDFSCLI. The failure is not related to this. (I upgraded the OS of my machine recently. Now, TestLargeBlock is passing but TestHDFSCLI is failing.)

      Show
      Tsz Wo Nicholas Sze added a comment - Passed all tests except TestHDFSCLI. The failure is not related to this. (I upgraded the OS of my machine recently. Now, TestLargeBlock is passing but TestHDFSCLI is failing.)
      Hide
      Uma Maheswara Rao G added a comment -

      Hi Nicholas,
      Thanks for incorporating the comments.
      Latest changes looks good to me.
      +1 from my side.

      5,6,7,8) You are correct that getAdditionalBlock(..), getAdditionalDatanode(..), abandonBlock(..), allocateBlock(..) and datanodeReport(..) are supposed to be blockManager/DatanodeManager's responsibility. However, as you mentioned, they require FSDirectory or other stuffs. So we need to change the API before we can move them. Let's move them in separated JIRAs.

      Agree, lets raise separate JIRAs.

      Can you check test results also with latest patch
      ( fault injection tests also )

      --Thanks

      Show
      Uma Maheswara Rao G added a comment - Hi Nicholas, Thanks for incorporating the comments. Latest changes looks good to me. +1 from my side. 5,6,7,8) You are correct that getAdditionalBlock(..), getAdditionalDatanode(..), abandonBlock(..), allocateBlock(..) and datanodeReport(..) are supposed to be blockManager/DatanodeManager's responsibility. However, as you mentioned, they require FSDirectory or other stuffs. So we need to change the API before we can move them. Let's move them in separated JIRAs. Agree, lets raise separate JIRAs. Can you check test results also with latest patch ( fault injection tests also ) --Thanks
      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 appears to include 42 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
      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 appears to include 42 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
      Tsz Wo Nicholas Sze added a comment -

      Hi Uma, thank you for the careful review.

      h2228_20110808.patch:

      1) Fixed the name. Good catch.

      2) Removed the comment

      3,4) moved blockRecieved(..) and processReport(..) to BlockManager. These are really good suggestions.

      5,6,7,8) You are correct that getAdditionalBlock(..), getAdditionalDatanode(..), abandonBlock(..), allocateBlock(..) and datanodeReport(..) are supposed to be blockManager/DatanodeManager's responsibility. However, as you mentioned, they require FSDirectory or other stuffs. So we need to change the API before we can move them. Let's move them in separated JIRAs.

      Show
      Tsz Wo Nicholas Sze added a comment - Hi Uma, thank you for the careful review. h2228_20110808.patch: 1) Fixed the name. Good catch. 2) Removed the comment 3,4) moved blockRecieved(..) and processReport(..) to BlockManager. These are really good suggestions. 5,6,7,8) You are correct that getAdditionalBlock(..), getAdditionalDatanode(..), abandonBlock(..), allocateBlock(..) and datanodeReport(..) are supposed to be blockManager/DatanodeManager's responsibility. However, as you mentioned, they require FSDirectory or other stuffs. So we need to change the API before we can move them. Let's move them in separated JIRAs.
      Hide
      Uma Maheswara Rao G added a comment -

      Hi Nicholas,
      I have gone through the changes.
      Please check my comments below.

      1)DatanodeManager.java :

      can we check method name and correct it?
      fetchDatanodess
      Not related to this patch changes, but as part of this refactoring can we correct this typo.

      2)FSNameSystem.java

        // we need to call DFSNodeStatus to filter out the dead data nodes
          blockManager.getDatanodeManager().fetchDatanodess(null, dead, true);
          for (DatanodeDescriptor node : dead) {
       

      can you please update the comment.

      3) can't we move blockrecieved method logic to BlockManager and just delegete the call from fsNameSystem?

      4) can't we move below validations also to blockManager.processReport?

      
         final DatanodeDescriptor node = blockManager.getDatanodeManager(
                ).getDatanode(nodeID);
            if (node == null || !node.isAlive) {
              throw new IOException("ProcessReport from dead or unregistered node: "
                                    + nodeID.getName());
            }
            // To minimize startup time, we discard any second (or later) block reports
            // that we receive while still in startup phase.
            if (isInStartupSafeMode() && node.numBlocks() > 0) {
              NameNode.stateChangeLog.info("BLOCK* NameSystem.processReport: "
                  + "discarded non-initial block report from " + nodeID.getName()
                  + " because namenode still in startup phase");
              return;
            }
         

      5) I think getAdditionalBlock logic also should move to blockManager. no?

      6) Similar to above, getAdditionalDatanode logic also to DatanodeManager.

      7) Same as above, abandonBlock also may be blockManager's responsibility. No?
      But here i have one cocenrn, if we just move code from FSNameSystem to BlockManager,
      to romove the block we may need to contact FSNameSystem back.
      Because FSNameSystem is holding the FSDirectory. This looks may be a kind cycle. So, can we check some better option?

      allocateBlock also same.

      8) can we move datanodeReport also to DatanodeManager?

      --Thanks

      Show
      Uma Maheswara Rao G added a comment - Hi Nicholas, I have gone through the changes. Please check my comments below. 1) DatanodeManager.java : can we check method name and correct it? fetchDatanodess Not related to this patch changes, but as part of this refactoring can we correct this typo. 2) FSNameSystem.java // we need to call DFSNodeStatus to filter out the dead data nodes blockManager.getDatanodeManager().fetchDatanodess( null , dead, true ); for (DatanodeDescriptor node : dead) { can you please update the comment. 3) can't we move blockrecieved method logic to BlockManager and just delegete the call from fsNameSystem? 4) can't we move below validations also to blockManager.processReport? final DatanodeDescriptor node = blockManager.getDatanodeManager( ).getDatanode(nodeID); if (node == null || !node.isAlive) { throw new IOException( "ProcessReport from dead or unregistered node: " + nodeID.getName()); } // To minimize startup time, we discard any second (or later) block reports // that we receive while still in startup phase. if (isInStartupSafeMode() && node.numBlocks() > 0) { NameNode.stateChangeLog.info( "BLOCK* NameSystem.processReport: " + "discarded non-initial block report from " + nodeID.getName() + " because namenode still in startup phase" ); return ; } 5) I think getAdditionalBlock logic also should move to blockManager. no? 6) Similar to above, getAdditionalDatanode logic also to DatanodeManager. 7) Same as above, abandonBlock also may be blockManager's responsibility. No? But here i have one cocenrn, if we just move code from FSNameSystem to BlockManager, to romove the block we may need to contact FSNameSystem back. Because FSNameSystem is holding the FSDirectory. This looks may be a kind cycle. So, can we check some better option? allocateBlock also same. 8) can we move datanodeReport also to DatanodeManager? --Thanks
      Hide
      Tsz Wo Nicholas Sze added a comment -

      The patch passed all unit tests except TestLargeBlock in my machine. TestLargeBlock also failed without the patch. The failure is not related to the patch.

      Show
      Tsz Wo Nicholas Sze added a comment - The patch passed all unit tests except TestLargeBlock in my machine. TestLargeBlock also failed without the patch. The failure is not related to the patch.
      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 appears to include 42 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
      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 appears to include 42 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
      Tsz Wo Nicholas Sze added a comment -

      h2228_20110805c.patch: fixed bugs.

      Show
      Tsz Wo Nicholas Sze added a comment - h2228_20110805c.patch: fixed bugs.
      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 appears to include 42 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.
           [exec] 
      
      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 appears to include 42 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. [exec]
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h2228_20110805b.patch: fixed some javac warnings.

      Show
      Tsz Wo Nicholas Sze added a comment - h2228_20110805b.patch: fixed some javac warnings.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h2228_20110805.patch: 1st patch.

      Show
      Tsz Wo Nicholas Sze added a comment - h2228_20110805.patch: 1st patch.

        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