hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Aaron T. Myers (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-4170) Add snapshot information to INodesInPath
Date Tue, 13 Nov 2012 22:40:12 GMT

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

Aaron T. Myers commented on HDFS-4170:
--------------------------------------

The patch looks pretty good to me. Just a few questions:

# I'm a little confused by this comment:
{code}
+    /**
+     * For snapshot paths, it is the reference to the snapshot; or null if the
+     * snapshot does not exist. For non-snapshot paths, it is the reference to
+     * the latest snapshot found in the path; or null if no snapshot is found.
+     */
{code}
First, I'm not sure what exactly is meant by a "snapshot path" versus a "non-snapshot path."
Is a "snapshot path" a path under an already-created snapshot? If so, how could it ever be
the case that that snapshot might not exist? I think we could make this comment a little clearer.
# What's the purpose of the "compareTo" here:
{code}
+      if (snapshot == null || snapshot.compareTo(s) < 0) {
{code}
I think here we're exploiting the fact that Snapshot#compareTo just compares snapshot IDs,
which are monotonically increasing. I think it would be a little clearer if we added a Snapshot#isNewerThan(Snapshot)
method. At least, it would be good to add a comment here explaining why the "compareTo(s)
< 0" is important. Further confusing the situation is that the Snapshot class itself has
two compareTo methods: one which just compares the names of the snapshots, and another which
compares the numeric IDs. I think we could stand to rename/clear up those two methods.

Sorry for providing my feedback so late. These issues can certainly be addressed in another
JIRA.
                
> Add snapshot information to INodesInPath
> ----------------------------------------
>
>                 Key: HDFS-4170
>                 URL: https://issues.apache.org/jira/browse/HDFS-4170
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: name-node
>            Reporter: Tsz Wo (Nicholas), SZE
>            Assignee: Tsz Wo (Nicholas), SZE
>             Fix For: Snapshot (HDFS-2802)
>
>         Attachments: h4170_20121108.patch, h4170_20121109.patch
>
>
> For snapshot paths, the snapshot information is required for accessing the snapshot.
> For non-snapshot paths, the latest snapshot found in the path is required for maintaining
diffs for modification.

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