huaxiangsun commented on a change in pull request #769:
URL: https://github.com/apache/hbase/pull/769#discussion_r430663274



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
##########
@@ -251,6 +261,31 @@ private void refreshCache() throws IOException {
     this.snapshots.putAll(newSnapshots);
   }
 
+  @VisibleForTesting
+  List<String> getSnapshotsInProgress() throws IOException {
+    List<String> snapshotInProgress = Lists.newArrayList();
+    // only add those files to the cache, but not to the known snapshots
+    Path snapshotTmpDir = new Path(snapshotDir, 
SnapshotDescriptionUtils.SNAPSHOT_TMP_DIR_NAME);
+    FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir);
+    if (running != null) {
+      for (FileStatus run : running) {
+        try {
+          
snapshotInProgress.addAll(fileInspector.filesUnderSnapshot(run.getPath()));
+        } catch (CorruptedSnapshotException e) {
+          // See HBASE-16464
+          if (e.getCause() instanceof FileNotFoundException) {
+            // If the snapshot is corrupt, we will delete it
+            fs.delete(run.getPath(), true);
+            LOG.warn("delete the " + run.getPath() + " due to exception:", 
e.getCause());

Review comment:
       Yeah, if it reads into the middle of copying manifest files, it is ok to 
remove this snapshot as copying HFiles has not started yet. So there is no 
impact for the logic in snapshotCleaner. 

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
##########
@@ -251,6 +261,31 @@ private void refreshCache() throws IOException {
     this.snapshots.putAll(newSnapshots);
   }
 
+  @VisibleForTesting
+  List<String> getSnapshotsInProgress() throws IOException {
+    List<String> snapshotInProgress = Lists.newArrayList();
+    // only add those files to the cache, but not to the known snapshots
+    Path snapshotTmpDir = new Path(snapshotDir, 
SnapshotDescriptionUtils.SNAPSHOT_TMP_DIR_NAME);
+    FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir);
+    if (running != null) {
+      for (FileStatus run : running) {
+        try {
+          
snapshotInProgress.addAll(fileInspector.filesUnderSnapshot(run.getPath()));
+        } catch (CorruptedSnapshotException e) {
+          // See HBASE-16464
+          if (e.getCause() instanceof FileNotFoundException) {
+            // If the snapshot is corrupt, we will delete it
+            fs.delete(run.getPath(), true);
+            LOG.warn("delete the " + run.getPath() + " due to exception:", 
e.getCause());

Review comment:
       The logic of getUnreferencedFiles() is that for an HFile which is not in 
cache, it will refreshCache to get the latest snapshot hfiles. If one hfile 
from this exortSnapshot job is in the list, this means that manifest files have 
been copied over, so refreshCache() will get the latest snapshot file list. 

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
##########
@@ -251,6 +261,31 @@ private void refreshCache() throws IOException {
     this.snapshots.putAll(newSnapshots);
   }
 
+  @VisibleForTesting
+  List<String> getSnapshotsInProgress() throws IOException {
+    List<String> snapshotInProgress = Lists.newArrayList();
+    // only add those files to the cache, but not to the known snapshots
+    Path snapshotTmpDir = new Path(snapshotDir, 
SnapshotDescriptionUtils.SNAPSHOT_TMP_DIR_NAME);
+    FileStatus[] running = FSUtils.listStatus(fs, snapshotTmpDir);
+    if (running != null) {
+      for (FileStatus run : running) {
+        try {
+          
snapshotInProgress.addAll(fileInspector.filesUnderSnapshot(run.getPath()));
+        } catch (CorruptedSnapshotException e) {
+          // See HBASE-16464
+          if (e.getCause() instanceof FileNotFoundException) {
+            // If the snapshot is corrupt, we will delete it
+            fs.delete(run.getPath(), true);
+            LOG.warn("delete the " + run.getPath() + " due to exception:", 
e.getCause());

Review comment:
       @busbey @z-york Unless you see something missing, I think this one is 
good to go, thanks.




----------------------------------------------------------------
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:
[email protected]


Reply via email to