huaxiangsun commented on a change in pull request #1791:
URL: https://github.com/apache/hbase/pull/1791#discussion_r434769865
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
##########
@@ -383,25 +385,38 @@ public static SnapshotDescription
readSnapshotInfo(FileSystem fs, Path snapshotD
}
/**
- * Move the finished snapshot to its final, publicly visible directory -
this marks the snapshot
- * as 'complete'.
- * @param snapshot description of the snapshot being tabken
- * @param rootdir root directory of the hbase installation
- * @param workingDir directory where the in progress snapshot was built
- * @param fs {@link FileSystem} where the snapshot was built
- * @throws org.apache.hadoop.hbase.snapshot.SnapshotCreationException if the
- * snapshot could not be moved
+ * Commits the snapshot process by moving the working snapshot
+ * to the finalized filepath
+ *
+ * @param snapshotDir The file path of the completed snapshots
+ * @param workingDir The file path of the in progress snapshots
+ * @param fs The file system of the completed snapshots
+ * @param workingDirFs The file system of the in progress snapshots
+ * @param conf Configuration
+ *
+ * @throws SnapshotCreationException if the snapshot could not be moved
* @throws IOException the filesystem could not be reached
*/
- public static void completeSnapshot(SnapshotDescription snapshot, Path
rootdir, Path workingDir,
- FileSystem fs) throws SnapshotCreationException, IOException {
- Path finishedDir = getCompletedSnapshotDir(snapshot, rootdir);
- LOG.debug("Snapshot is done, just moving the snapshot from " + workingDir
+ " to "
- + finishedDir);
- if (!fs.rename(workingDir, finishedDir)) {
- throw new SnapshotCreationException(
- "Failed to move working directory(" + workingDir + ") to completed
directory("
- + finishedDir + ").", ProtobufUtil.createSnapshotDesc(snapshot));
+ public static void completeSnapshot(Path snapshotDir, Path workingDir,
FileSystem fs,
Review comment:
completeSnapshot() is being used by some other test classes besides
TakeSnapshotHandler. What it does is doing a few checks and do a rename/copy (
a static utility method). SnapshotDescriptionUtils is a class which aggregates
set of utility methods, though the name is kind of misleading, it already has a
set of utility methods beyond SnapshotDescription. Putting a static utility
method in TakeSnapshotHandler does not seem a good fit to me, what do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
