[
https://issues.apache.org/jira/browse/HDFS-5715?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13863495#comment-13863495
]
Arpit Agarwal commented on HDFS-5715:
-------------------------------------
The patch looks great, a few minor comments:
# {{AbstractINodeDiffList#getDiff}} looks unused, should we remove it?
# {{FsPermissionChecker#checkTraverse, #checkSubAccess}} etc. - rename
{{snapshot}} --> {{snapshotId}} to be consistent with changes in all other
files?
Javadocs for a few functions are outdated and can be updated now:
# {{INode#getModificationTime(), #getUserName, #getGroupName,
#getFsPermission}} etc. You already fixed these in INodeDirectory.java.
# {{INode#getSnapshotINode}} (it was wrong before your patch).
One simplification we can do in a separate jira:
# {{FSDirectory#unprotectedRenameTo}} can use the overload of
{{INode#computeQuotaUsage}} without the {{snapshotId}} parameter, instead of
having to pass {{Snapshot.CURRENT_STATE_ID}} to make calling code more
readable. Or if we prefer to pass the snapshot ID explicitly we can remove the
second overload. This simplification can be done for few other places in
FSDirectory.java.
> Use Snapshot ID to indicate the corresponding Snapshot for a
> FileDiff/DirectoryDiff
> -----------------------------------------------------------------------------------
>
> Key: HDFS-5715
> URL: https://issues.apache.org/jira/browse/HDFS-5715
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Reporter: Jing Zhao
> Assignee: Jing Zhao
> Attachments: HDFS-5715.000.patch, HDFS-5715.001.patch
>
>
> Currently FileDiff and DirectoryDiff both contain a snapshot object reference
> to indicate its associated snapshot. Instead, we can simply record the
> corresponding snapshot id there. This can simplify some logic and allow us to
> use a byte array to represent the snapshot feature (HDFS-5714).
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
