[HDFS-2237] Change UnderReplicatedBlocks from public to package private - ASF JIRA

Details

Type: Sub-task
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: None
Fix Version/s: 0.23.0
Component/s: namenode
Labels:
None

Hadoop Flags:

Reviewed

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending

Attachments

h2237_20110808.patch

08/Aug/11 10:41

13 kB

Tsz Wo Nicholas Sze
h2237_20110808b.patch

08/Aug/11 14:11

7 kB

Tsz Wo Nicholas Sze
h2237_20110810.patch

10/Aug/11 15:39

9 kB

Tsz Wo Nicholas Sze

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 10:41

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 - 08/Aug/11 10:41 h2237_20110808.patch: change the classes and methods from public to package private or private. add synchronized to clear().

Hide

Permalink

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 10:50

     [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 - 08/Aug/11 10:50 [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

Permalink

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 14:11

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 - 08/Aug/11 14:11 h2237_20110808b.patch: reverted some change including adding synchronized to clear(). Don't want to accidentally introduce deadlocks.

Hide

Permalink

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 14:24

     [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 - 08/Aug/11 14:24 [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

Permalink

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 16:29

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

Show

Tsz Wo Nicholas Sze added a comment - 08/Aug/11 16:29 Passed all tests except TestHDFSCLI, which was not related to this.

Hide

Permalink

Uma Maheswara Rao G added a comment - 10/Aug/11 10:06

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 - 10/Aug/11 10:06 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

Permalink

Tsz Wo Nicholas Sze added a comment - 10/Aug/11 15:39

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 - 10/Aug/11 15:39 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

Permalink

Uma Maheswara Rao G added a comment - 10/Aug/11 16:17

Patch looks good
+1 from my side.

Show

Uma Maheswara Rao G added a comment - 10/Aug/11 16:17 Patch looks good +1 from my side.

Hide

Permalink

Tsz Wo Nicholas Sze added a comment - 11/Aug/11 01:06

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 - 11/Aug/11 01:06 The new patch only add javadoc changes. Checked that there is no javadoc warnings. I have committed this.

Hide

Permalink

Hudson added a comment - 11/Aug/11 01:24

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 - 11/Aug/11 01:24 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

Permalink

Hudson added a comment - 11/Aug/11 01:27

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 - 11/Aug/11 01:27 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

Permalink

Hudson added a comment - 11/Aug/11 11:30

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 - 11/Aug/11 11:30 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:

08/Aug/11 10:38

Updated:

15/Nov/11 00:52

Resolved:

11/Aug/11 01:07

Development

Agile

View on Board