Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. h2237_20110810.patch
      9 kB
      Tsz Wo Nicholas Sze
    2. h2237_20110808b.patch
      7 kB
      Tsz Wo Nicholas Sze
    3. h2237_20110808.patch
      13 kB
      Tsz Wo Nicholas Sze

      Activity

      Hide
      Tsz Wo Nicholas Sze added a comment -

      h2237_20110808.patch:

      • change the classes and methods from public to package private or private.
      • add synchronized to clear().
      Show
      Tsz Wo Nicholas Sze added a comment - h2237_20110808.patch: change the classes and methods from public to package private or private. add synchronized to clear().
      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 doesn't appear to include any new or modified tests.
           [exec]                         Please justify why no new tests are needed for this patch.
           [exec]                         Also please list what manual steps were performed to verify this patch.
           [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 doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [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 -

      h2237_20110808b.patch: reverted some change including adding synchronized to clear(). Don't want to accidentally introduce deadlocks.

      Show
      Tsz Wo Nicholas Sze added a comment - h2237_20110808b.patch: reverted some change including adding synchronized to clear(). Don't want to accidentally introduce deadlocks.
      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 doesn't appear to include any new or modified tests.
           [exec]                         Please justify why no new tests are needed for this patch.
           [exec]                         Also please list what manual steps were performed to verify this patch.
           [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 doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [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 -

      Passed all tests except TestHDFSCLI, which was not related to this.

      Show
      Tsz Wo Nicholas Sze added a comment - Passed all tests except TestHDFSCLI, which was not related to this.
      Hide
      Uma Maheswara Rao G added a comment -

      Hi Nicholas,

      I have gone through the changes.
      Patch looks good to me.

      Looks patch could not be applied any more to trunk. You may need to regenerate.

      I have small nit:
      can we convert comments to javadoc comments in UnderReplicatedBlocks.java?.
      /* return an iterator of all the under replication blocks */
      public synchronized BlockIterator iterator()

      { return new BlockIterator(); }

      --Thanks

      Show
      Uma Maheswara Rao G added a comment - Hi Nicholas, I have gone through the changes. Patch looks good to me. Looks patch could not be applied any more to trunk. You may need to regenerate. I have small nit: can we convert comments to javadoc comments in UnderReplicatedBlocks.java?. /* return an iterator of all the under replication blocks */ public synchronized BlockIterator iterator() { return new BlockIterator(); } --Thanks
      Hide
      Tsz Wo Nicholas Sze added a comment -

      Hi Uma, thanks for the review.

      h2237_20110810.patch: changed comments to javadoc.

      (BTW, the patch still apply although there are some offsets.)

      szetszwo hdfs$patch -p0 -i ~/hadoop/2237/h2237_20110808b.patch 
      patching file src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      Hunk #3 succeeded at 2905 (offset -32 lines).
      Hunk #4 succeeded at 3949 (offset -51 lines).
      patching file src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java
      patching file src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hunk #2 succeeded at 2587 (offset 7 lines).
      
      Show
      Tsz Wo Nicholas Sze added a comment - Hi Uma, thanks for the review. h2237_20110810.patch: changed comments to javadoc. (BTW, the patch still apply although there are some offsets.) szetszwo hdfs$patch -p0 -i ~/hadoop/2237/h2237_20110808b.patch patching file src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Hunk #3 succeeded at 2905 (offset -32 lines). Hunk #4 succeeded at 3949 (offset -51 lines). patching file src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java patching file src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java Hunk #2 succeeded at 2587 (offset 7 lines).
      Hide
      Uma Maheswara Rao G added a comment -

      Patch looks good
      +1 from my side.

      Show
      Uma Maheswara Rao G added a comment - Patch looks good +1 from my side.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      The new patch only add javadoc changes. Checked that there is no javadoc warnings.

      I have committed this.

      Show
      Tsz Wo Nicholas Sze added a comment - The new patch only add javadoc changes. Checked that there is no javadoc warnings. I have committed this.
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Common-trunk-Commit #723 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/723/)
      HDFS-2237. Change UnderReplicatedBlocks from public to package private.

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

      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #723 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/723/ ) HDFS-2237 . Change UnderReplicatedBlocks from public to package private. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1156424 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk-Commit #824 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/824/)
      HDFS-2237. Change UnderReplicatedBlocks from public to package private.

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

      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #824 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/824/ ) HDFS-2237 . Change UnderReplicatedBlocks from public to package private. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1156424 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk #748 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/748/)
      HDFS-2237. Change UnderReplicatedBlocks from public to package private.

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

      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #748 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/748/ ) HDFS-2237 . Change UnderReplicatedBlocks from public to package private. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1156424 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/UnderReplicatedBlocks.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java

        People

        • Assignee:
          Tsz Wo Nicholas Sze
          Reporter:
          Tsz Wo Nicholas Sze
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development