hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jing Zhao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-4446) Support file snapshot with diff lists
Date Thu, 07 Feb 2013 22:59:13 GMT

    [ https://issues.apache.org/jira/browse/HDFS-4446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13574026#comment-13574026
] 

Jing Zhao commented on HDFS-4446:
---------------------------------

The patch looks good and with the file diff we can simplify the logic a lot. Some of my thoughts:
1. In AbstractINodeDiffList.java, the methods append/insert can be renamed to addLast/addFirst
just like Deque?

2. Seems saveChild2Snapshot and createSnapshotCopy are only useful for INodeSymlink#recordModification
now. I think we can make INode#recordModification abstract, and move the
original INode#recordModification implementation to INodeSymlink.

3. INodeDirectorySnapshottable#replaceSelf is called when resetting a snapshottable dir. When
this dir has no snapshots but its directory diff list contains its ancestors' directory diff,
the current INodeDirectoryWithSnapshot(INodeDirectory that) may lose all the previous diffs.

4. In INodeFileWithSnapshot#recordModification, is the check "if (this == getParent().getChild(getLocalNameBytes(),
latest))" used to handle the corner case that user may delete and then create with the same
file name? Maybe we can add a comment to explain.

5. FSDirectory#unprotectedReplaceINodeFile and FSDirectory#unprotectedDelete may need more
javadoc to explain the logic.

6. In INodeFileWithSnapshot#destroySubtreeAndCollectBlocks, if snapshot is not null, is it
possible that Util.collectBlocksAndClear may run twice? Also the snapshot deletion has problems
when handling several special cases: 1) we need to go through the whole directory diff list
when processing recursively to handle the case that a file/directory containing the to-be-deleted
snapshot diff was deleted in a later snapshot. 2) when the diff list contains diff for snapshot
s0, s2, and s3, currently we simply combine s2 to s0 when deleting snapshot s2. This will
cause bug when a user later wants to visit snapshot s1. We can create a separate jira to fix
this.

Besides, the comment "// TODO: add a new testcase for this" can be removed from AbstractINodeDiffList#deleteSnapshotDiff
(I forgot to remove it in HDFS-4414...).
                
> Support file snapshot with diff lists
> -------------------------------------
>
>                 Key: HDFS-4446
>                 URL: https://issues.apache.org/jira/browse/HDFS-4446
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>            Reporter: Tsz Wo (Nicholas), SZE
>            Assignee: Tsz Wo (Nicholas), SZE
>         Attachments: h4446_20130131.patch, h4446_20130203.patch, h4446_20130204b.patch,
h4446_20130204.patch, h4446_20130205.patch
>
>
> Similar to INodeDirectoryWithSnapshot, it is better to implement INodeFileWithSnapshot
so that the INodeFileSnapshot class can be eliminated.

--
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