[
https://issues.apache.org/jira/browse/HBASE-8352?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13633294#comment-13633294
]
Jonathan Hsieh commented on HBASE-8352:
---------------------------------------
nit: Prefer keeping the no argument version -- this extra generally never used
arg makes this harder to read.
{code}
- List<SnapshotDescription> snapshots =
snapshotManager.getCompletedSnapshots();
+ List<SnapshotDescription> snapshots =
snapshotManager.getCompletedSnapshots(null);
{code}
nit: prefer if you create a private more generic version with the path argument
but keep the older no argument version public. the current way exposes
possibilities that don't need to be.
So instead of this:
{code}
/**
* Gets the list of all completed snapshots.
+ * @param snapshotDir snapshot directory
* @return list of SnapshotDescriptions
* @throws IOException File system exception
*/
- public List<SnapshotDescription> getCompletedSnapshots() throws IOException {
+ public List<SnapshotDescription> getCompletedSnapshots(Path snapshotDir)
throws IOException {
List<SnapshotDescription> snapshotDescs = new
ArrayList<SnapshotDescription>();
// first create the snapshot root path and check to see if it exists
- Path snapshotDir = SnapshotDescriptionUtils.getSnapshotsDir(rootDir);
FileSystem fs = master.getMasterFileSystem().getFileSystem();
+ if (snapshotDir == null) snapshotDir =
SnapshotDescriptionUtils.getSnapshotsDir(rootDir);
// if there are no snapshots, return an empty list
if (!fs.exists(snapshotDir)) {
@@ -877,6 +878,15 @@
{code}
have something more like this:
{code}
public List<SnapshotDescription> getCompletedSnapshots() throws IOException {
getCompletedSnapshots(SnapshotDescriptionUtils.getSnapshotsDir(rootDir));
}
private List<SnapshotDescription> getCompletedSnapshots(Path snapshotDir)
throws IOException {
...
}
{code}
Otherwise, lgtm.
> Rename '.snapshot' directory
> ----------------------------
>
> Key: HBASE-8352
> URL: https://issues.apache.org/jira/browse/HBASE-8352
> Project: HBase
> Issue Type: Bug
> Reporter: Ted Yu
> Assignee: Ted Yu
> Priority: Blocker
> Fix For: 0.98.0, 0.94.7, 0.95.1
>
> Attachments: 8352-0.94-v1.txt, 8352-0.94-v2.txt, 8352-trunk.txt,
> 8352-trunk-v2.txt, 8352-trunk-v3.txt, 8352-trunk-v4.txt
>
>
> Testing HBase Snapshot on top of Hadoop's Snapshot branch
> (http://svn.apache.org/viewvc/hadoop/common/branches/HDFS-2802/), we found
> that both features used '.snapshot' directory to store metadata.
> HDFS (built from HDFS-2802 branch) doesn't allow paths with .snapshot as a
> component
> From discussion on [email protected], (see
> http://search-hadoop.com/m/kY6C3cXMs51), consensus was to rename '.snapshot'
> directory in HBase so that both features can co-exist smoothly.
--
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
