[ https://issues.apache.org/jira/browse/HDFS-5782?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14261712#comment-14261712 ]
Hadoop QA commented on HDFS-5782: --------------------------------- {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12689565/HDFS-5782.patch against trunk revision 6621c35. {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:green}+1 tests included{color}. The patch appears to include 3 new or modified test files. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. There were no new javadoc warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:red}-1 findbugs{color}. The patch appears to introduce 1 new Findbugs (version 2.0.3) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 core tests{color}. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9132//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9132//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9132//console This message is automatically generated. > BlockListAsLongs should take lists of Replicas rather than concrete classes > --------------------------------------------------------------------------- > > Key: HDFS-5782 > URL: https://issues.apache.org/jira/browse/HDFS-5782 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: datanode > Affects Versions: 3.0.0 > Reporter: David Powell > Assignee: David Powell > Priority: Minor > Attachments: HDFS-5782.patch, HDFS-5782.patch > > > From HDFS-5194: > {quote} > BlockListAsLongs's constructor takes a list of Blocks and a list of > ReplicaInfos. On the surface, the former is mildly irritating because it is > a concrete class, while the latter is a greater concern due to being a > File-based implementation of Replica. > On deeper inspection, BlockListAsLongs passes members of both to an internal > method that accepts just Blocks, which conditionally casts them *back* to > ReplicaInfos (this cast only happens to the latter, though this isn't > immediately obvious to the reader). > Conveniently, all methods called on these objects are found in the Replica > interface, and all functional (i.e. non-test) consumers of this interface > pass in Replica subclasses. If this constructor took Lists of Replicas > instead, it would be more generally useful and its implementation would be > cleaner as well. > {quote} > Fixing this indeed makes the business end of BlockListAsLongs cleaner while > requiring no changes to FsDatasetImpl. As suggested by the above > description, though, the HDFS tests use BlockListAsLongs differently from the > production code -- they pretty much universally provide a list of actual > Blocks. To handle this: > - In the case of SimulatedFSDataset, providing a list of Replicas is actually > less work. > - In the case of NNThroughputBenchmark, rewriting to use Replicas is fairly > invasive. Instead, the patch creates a second constructor in > BlockListOfLongs specifically for the use of NNThrougputBenchmark. It turns > the stomach a little, but is clearer and requires less code than the > alternatives (and isn't without precedent). -- This message was sent by Atlassian JIRA (v6.3.4#6332)