[
https://issues.apache.org/jira/browse/HDFS-6609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14049090#comment-14049090
]
Haohui Mai commented on HDFS-6609:
----------------------------------
The patch looks good to me. Some comments:
1. in {{INodeDirectory}}:
{code}
- @VisibleForTesting
- protected static void dumpTreeRecursively(PrintWriter out,
+ public static void dumpTreeRecursively(PrintWriter out,
{code}
Is the deletion of {{VisibleForTesting}} accidental?
{code}
- protected static class SnapshotAndINode {
+ public static class SnapshotAndINode {
{code}
2. Is it possible to make {{SnapshotAndINode}} a package local class?
3. Now there're both {{DirectoryWithSnapshotFeature}} and
{{DirectorySnapshottableFeature}}, it might be worthwhile to add some comments
to document it.
4. {{DirectorySnapshottableFeature}} should have the
{{\@InterfaceAudience.Private}} annotation.
> Use DirectorySnapshottableFeature to represent a snapshottable directory
> ------------------------------------------------------------------------
>
> Key: HDFS-6609
> URL: https://issues.apache.org/jira/browse/HDFS-6609
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Reporter: Jing Zhao
> Assignee: Jing Zhao
> Attachments: HDFS-6609.000.patch
>
>
> Currently we use INodeDirectorySnapshottable to represent a snapshottable
> directory. When we set a directory to snapshottable, we also need to replace
> the corresponding INode. It will be better to use a
> DirectorySnapshottableFeature to simplify the process and get rid of the
> inode replacement.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
