[ https://issues.apache.org/jira/browse/HDFS-5285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13828510#comment-13828510 ]
Vinay commented on HDFS-5285: ----------------------------- Overall patch looks good. Following are some of the minor nits 1. Better to add an error message to these checks wherever necessary ( May not be necessary at all places) {code} Preconditions.checkState(fileINode.isUnderConstruction());{code} {code} Preconditions.checkArgument(file.isUnderConstruction());{code} 2. Better update {{INodeFile.toINodeFile()}} to some suitable name like {{INodeFile.completeFile()}} 3. in {{FSNameSystem#getAdditionalBlock}} can add Precondition check for underconstruction ( 2nd time) {code} final INodeFile pendingFile = inodes[inodes.length - 1].asFile();{code} 4. There are many blank lines with trailing whitespaces. Its better to clean these. (May be can correct eclipse formatter) 5. Since {{MutableBlockCollection}} is removed we can remove in comments as well {code} @Override // MutableBlockCollection public void setBlock(int index, BlockInfo blk) { this.blocks[index] = blk; }{code} 6. In TestRenameWithSnapshots additional assertion of isUnderConstruction is required. {code} INode fooNode = fooRef.asFile(); assertTrue(fooNode instanceof INodeFileWithSnapshot);{code} Lets wait and see for jenkins report, how many tests are OK..? > Flatten INodeFile hierarchy: Add UnderContruction Feature > --------------------------------------------------------- > > Key: HDFS-5285 > URL: https://issues.apache.org/jira/browse/HDFS-5285 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: namenode > Reporter: Tsz Wo (Nicholas), SZE > Assignee: Jing Zhao > Attachments: HDFS-5285.001.patch, HDFS-5285.002.patch, > h5285_20131001.patch, h5285_20131002.patch, h5285_20131118.patch > > > For files, there are INodeFile, INodeFileUnderConstruction, > INodeFileWithSnapshot and INodeFileUnderConstructionWithSnapshot for > representing whether a file is under construction or whether it is in some > snapshot. The following are two major problems of the current approach: > - Java class does not support multiple inheritances so that > INodeFileUnderConstructionWithSnapshot cannot extend both > INodeFileUnderConstruction and INodeFileWithSnapshot. > - The number of classes is exponential to the number of features. Currently, > there are only two features, UnderConstruction and WithSnapshot. The number > of classes is 2^2 = 4. It is hard to add one more feature since the number > of classes will become 2^3 = 8. > As a first step, we implement an Under-Construction feature to replace > INodeFileUnderConstruction and INodeFileUnderConstructionWithSnapshot in this > jira. -- This message was sent by Atlassian JIRA (v6.1#6144)