hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Suresh Srinivas (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-4317) Change INode to support HDFS-4103
Date Sun, 16 Dec 2012 20:30:13 GMT

    [ 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

Mime
View raw message