[
https://issues.apache.org/jira/browse/HDFS-4196?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498540#comment-13498540
]
Aaron T. Myers commented on HDFS-4196:
--------------------------------------
Patch looks pretty good to me. A few little comments:
# It would be good to add a description of when an exception might be thrown in
the case of renameSnapshot or createSnapshot. Just adding "{{@throws
IOException}}" isn't very helpful.
# In INodeDirectorySnapshottable#renameSnapshot the order of the params is
oldName, newName, path, but everywhere else the order of the parameters is
path, oldName, newName.
# I recommend you use HdfsConstants.DOT_SNAPSHOT_DIR instead of hard-coding
".snapshot".
# Is it really necessary to synchronize in here? Shouldn't that be taken care
of by the fact that we synchronize externally on the FSNS and FSDir locks? For
that matter, it's a little odd that we're locking the FSDir lock in FSNS
methods. Perhaps we should move the FSDir locking into SnapshotManager? That
would be more inline with the rest of the locking structure in the NN.
# Seems like we should be throwing a SnapshotException in the case of rename
failure, not generic IOException.
# Nit: TestSnapshotRename#testSnapshotRename could use a little more white
space between lines for my taste. I find it a little dense / hard to read.
> Support renaming of snapshots
> -----------------------------
>
> Key: HDFS-4196
> URL: https://issues.apache.org/jira/browse/HDFS-4196
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: data-node, name-node
> Reporter: Jing Zhao
> Assignee: Jing Zhao
> Attachments: HDFS-4196.001.patch, HDFS-4196.002.patch
>
>
> Add the functionality of renaming an existing snapshot with a given new name.
--
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
