[
https://issues.apache.org/jira/browse/HDFS-4317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13533505#comment-13533505
]
Suresh Srinivas commented on HDFS-4317:
---------------------------------------
Thanks for splitting the patch. Minor comments:
# INode#createSnapshotCopy - "The original inode usually is this inode." What
is "this" inode? I am also not clear why "original inode is the new inode"
relevant to the javadoc?
# INodeDirectory#replaceINodeFile4Snapshot - javadoc talks about replacing
child, but does not mention snapshot. However method name mentions snapshot.
Btw why cannot we just call it replaceChild()?
# INodesInPath#path is not currently not used. I assume it will be used in
later patches?
While reviewing the code, mapping the different cases
INodeDirectoryWithSnapshot in code makes it easy to understand later. I made
following changes while reviewing the code:
{code}
+ int create(final INode inode) {
+ final int c = search(created, inode); // Case 1.1
+ Pair<Integer, Integer> delete(final INode inode) {
...
- // remove a newly created inode
+ // Case 1.1.2 - remove a newly created inode
...
- // not in c-list, it must be in previous
+ // Case 2.2 not in c-list, it must be in previous
...
- // inode is already in c-list,
- created.set(c, newinode);
+ // Case 1.1.3 - inode is already in c-list,
...
if (d < 0) {
- // neither in c-list nor d-list
+ // Case 2.3 - neither in c-list nor d-list
insertCreated(newinode, c);
insertDeleted(oldinode, d);
{code}
I have attached a patch of INodeDirectoryWithSnapshot to this jira, in case you
find it easy to pick it up that way.
+1 with those changes for the patch.
> Change INode to support HDFS-4103
> ---------------------------------
>
> Key: HDFS-4317
> URL: https://issues.apache.org/jira/browse/HDFS-4317
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Tsz Wo (Nicholas), SZE
> Attachments: 4317.INodeDirectoryWithSnapshot.patch,
> h4317_20121216.patch
>
>
> This is a subtask separated from HDFS-4103:
> - Change Diff to support undo.
> - Add createSnapshotCopy() for inode
> - Reorganize the INode and its subclasses constructors and some other methods.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
