hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Haohui Mai (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-6609) Use DirectorySnapshottableFeature to represent a snapshottable directory
Date Tue, 01 Jul 2014 17:28:24 GMT

    [ 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}}:
-  @VisibleForTesting
-  protected static void dumpTreeRecursively(PrintWriter out,
+  public static void dumpTreeRecursively(PrintWriter out,

Is the deletion of {{VisibleForTesting}} accidental?

-  protected static class SnapshotAndINode {
+  public static class SnapshotAndINode {

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

View raw message