[
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
