You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2022/01/01 15:56:10 UTC

[hbase] 11/15: HBASE-26328 Clone snapshot doesn't load reference files into FILE SFT impl (#3749)

This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a commit to branch HBASE-26067-branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git

commit 7e55f835ca6e7662eeac6da7f996aa672281e037
Author: Wellington Ramos Chevreuil <wc...@apache.org>
AuthorDate: Fri Oct 22 16:56:15 2021 +0100

    HBASE-26328 Clone snapshot doesn't load reference files into FILE SFT impl (#3749)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../java/org/apache/hadoop/hbase/io/HFileLink.java | 63 ++++++--------
 .../master/procedure/CloneSnapshotProcedure.java   | 52 +++---------
 .../storefiletracker/StoreFileTrackerBase.java     |  1 +
 .../hbase/snapshot/RestoreSnapshotHelper.java      | 95 ++++++++++++++++++----
 .../TestCloneSnapshotProcedureFileBasedSFT.java    | 42 ++++++++++
 5 files changed, 157 insertions(+), 96 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java
index 74836ce..fbed724 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HFileLink.java
@@ -201,7 +201,6 @@ public class HFileLink extends FileLink {
     return isHFileLink(path.getName());
   }
 
-
   /**
    * @param fileName File name to check.
    * @return True if the path is a HFileLink.
@@ -322,10 +321,10 @@ public class HFileLink extends FileLink {
    * @param dstFamilyPath - Destination path (table/region/cf/)
    * @param hfileRegionInfo - Linked HFile Region Info
    * @param hfileName - Linked HFile name
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
+   * @return the file link name.
+   * @throws IOException on file or parent directory creation failure.
    */
-  public static boolean create(final Configuration conf, final FileSystem fs,
+  public static String create(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final RegionInfo hfileRegionInfo,
       final String hfileName) throws IOException {
     return create(conf, fs, dstFamilyPath, hfileRegionInfo, hfileName, true);
@@ -343,10 +342,10 @@ public class HFileLink extends FileLink {
    * @param hfileRegionInfo - Linked HFile Region Info
    * @param hfileName - Linked HFile name
    * @param createBackRef - Whether back reference should be created. Defaults to true.
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
+   * @return the file link name.
+   * @throws IOException on file or parent directory creation failure.
    */
-  public static boolean create(final Configuration conf, final FileSystem fs,
+  public static String create(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final RegionInfo hfileRegionInfo,
       final String hfileName, final boolean createBackRef) throws IOException {
     TableName linkedTable = hfileRegionInfo.getTable();
@@ -366,17 +365,18 @@ public class HFileLink extends FileLink {
    * @param linkedTable - Linked Table Name
    * @param linkedRegion - Linked Region Name
    * @param hfileName - Linked HFile name
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
+   * @return the file link name.
+   * @throws IOException on file or parent directory creation failure.
    */
-  public static boolean create(final Configuration conf, final FileSystem fs,
+  public static String create(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final TableName linkedTable, final String linkedRegion,
       final String hfileName) throws IOException {
     return create(conf, fs, dstFamilyPath, linkedTable, linkedRegion, hfileName, true);
   }
 
   /**
-   * Create a new HFileLink
+   * Create a new HFileLink. In the event of link creation failure, this method throws an
+   * IOException, so that the calling upper laying can decide on how to proceed with this.
    *
    * <p>It also adds a back-reference to the hfile back-reference directory
    * to simplify the reference-count and the cleaning process.
@@ -388,10 +388,10 @@ public class HFileLink extends FileLink {
    * @param linkedRegion - Linked Region Name
    * @param hfileName - Linked HFile name
    * @param createBackRef - Whether back reference should be created. Defaults to true.
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
+   * @return the file link name.
+   * @throws IOException on file or parent directory creation failure.
    */
-  public static boolean create(final Configuration conf, final FileSystem fs,
+  public static String create(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final TableName linkedTable, final String linkedRegion,
       final String hfileName, final boolean createBackRef) throws IOException {
     String familyName = dstFamilyPath.getName();
@@ -417,10 +417,10 @@ public class HFileLink extends FileLink {
    * @param linkedRegion - Linked Region Name
    * @param hfileName - Linked HFile name
    * @param createBackRef - Whether back reference should be created. Defaults to true.
-   * @return true if the file is created, otherwise the file exists.
+   * @return the file link name.
    * @throws IOException on file or parent directory creation failure
    */
-  public static boolean create(final Configuration conf, final FileSystem fs,
+  public static String create(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final String familyName, final String dstTableName,
       final String dstRegionName, final TableName linkedTable, final String linkedRegion,
       final String hfileName, final boolean createBackRef) throws IOException {
@@ -444,7 +444,9 @@ public class HFileLink extends FileLink {
     }
     try {
       // Create the link
-      return fs.createNewFile(new Path(dstFamilyPath, name));
+      if (fs.createNewFile(new Path(dstFamilyPath, name))) {
+        return name;
+      }
     } catch (IOException e) {
       LOG.error("couldn't create the link=" + name + " for " + dstFamilyPath, e);
       // Revert the reference if the link creation failed
@@ -453,25 +455,8 @@ public class HFileLink extends FileLink {
       }
       throw e;
     }
-  }
-
-  /**
-   * Create a new HFileLink starting from a hfileLink name
-   *
-   * <p>It also adds a back-reference to the hfile back-reference directory
-   * to simplify the reference-count and the cleaning process.
-   *
-   * @param conf {@link Configuration} to read for the archive directory name
-   * @param fs {@link FileSystem} on which to write the HFileLink
-   * @param dstFamilyPath - Destination path (table/region/cf/)
-   * @param hfileLinkName - HFileLink name (it contains hfile-region-table)
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
-   */
-  public static boolean createFromHFileLink(final Configuration conf, final FileSystem fs,
-      final Path dstFamilyPath, final String hfileLinkName)
-          throws IOException {
-    return createFromHFileLink(conf, fs, dstFamilyPath, hfileLinkName, true);
+    throw new IOException("File link=" + name + " already exists under " +
+      dstFamilyPath + " folder.");
   }
 
   /**
@@ -485,10 +470,10 @@ public class HFileLink extends FileLink {
    * @param dstFamilyPath - Destination path (table/region/cf/)
    * @param hfileLinkName - HFileLink name (it contains hfile-region-table)
    * @param createBackRef - Whether back reference should be created. Defaults to true.
-   * @return true if the file is created, otherwise the file exists.
-   * @throws IOException on file or parent directory creation failure
+   * @return the file link name.
+   * @throws IOException on file or parent directory creation failure.
    */
-  public static boolean createFromHFileLink(final Configuration conf, final FileSystem fs,
+  public static String createFromHFileLink(final Configuration conf, final FileSystem fs,
       final Path dstFamilyPath, final String hfileLinkName, final boolean createBackRef)
           throws IOException {
     Matcher m = LINK_NAME_PATTERN.matcher(hfileLinkName);
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
index 8157af9..7157fbf 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
@@ -41,7 +41,6 @@ import org.apache.hadoop.hbase.master.MetricsSnapshot;
 import org.apache.hadoop.hbase.master.RegionState;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
 import org.apache.hadoop.hbase.master.procedure.CreateTableProcedure.CreateHdfsRegions;
-import org.apache.hadoop.hbase.mob.MobUtils;
 import org.apache.hadoop.hbase.monitoring.MonitoredTask;
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
@@ -459,56 +458,25 @@ public class CloneSnapshotProcedure
     List<RegionInfo> newRegions,
     final CreateHdfsRegions hdfsRegionHandler) throws IOException {
     final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem();
-    final Path tempdir = mfs.getTempDir();
 
     // 1. Create Table Descriptor
     // using a copy of descriptor, table will be created enabling first
-    final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableDescriptor.getTableName());
-    if (CommonFSUtils.isExists(mfs.getFileSystem(), tempTableDir)) {
+    final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(),
+      tableDescriptor.getTableName());
+    if (CommonFSUtils.isExists(mfs.getFileSystem(), tableDir)) {
       // if the region dirs exist, will cause exception and unlimited retry (see HBASE-24546)
-      LOG.warn("temp table dir already exists on disk: {}, will be deleted.", tempTableDir);
-      CommonFSUtils.deleteDirectory(mfs.getFileSystem(), tempTableDir);
+      LOG.warn("temp table dir already exists on disk: {}, will be deleted.", tableDir);
+      CommonFSUtils.deleteDirectory(mfs.getFileSystem(), tableDir);
     }
-    ((FSTableDescriptors) (env.getMasterServices().getTableDescriptors()))
-      .createTableDescriptorForTableDirectory(tempTableDir,
-        TableDescriptorBuilder.newBuilder(tableDescriptor).build(), false);
+    ((FSTableDescriptors)(env.getMasterServices().getTableDescriptors()))
+      .createTableDescriptorForTableDirectory(tableDir,
+              TableDescriptorBuilder.newBuilder(tableDescriptor).build(), false);
 
     // 2. Create Regions
     newRegions = hdfsRegionHandler.createHdfsRegions(
-      env, tempdir, tableDescriptor.getTableName(), newRegions);
-
-    // 3. Move Table temp directory to the hbase root location
-    CreateTableProcedure.moveTempDirectoryToHBaseRoot(env, tableDescriptor, tempTableDir);
-    // Move Table temp mob directory to the hbase root location
-    Path tempMobTableDir = MobUtils.getMobTableDir(tempdir, tableDescriptor.getTableName());
-    if (mfs.getFileSystem().exists(tempMobTableDir)) {
-      moveTempMobDirectoryToHBaseRoot(mfs, tableDescriptor, tempMobTableDir);
-    }
-    return newRegions;
-  }
+      env, mfs.getRootDir(), tableDescriptor.getTableName(), newRegions);
 
-  /**
-   * Move table temp mob directory to the hbase root location
-   * @param mfs The master file system
-   * @param tableDescriptor The table to operate on
-   * @param tempMobTableDir The temp mob directory of table
-   * @throws IOException If failed to move temp mob dir to hbase root dir
-   */
-  private void moveTempMobDirectoryToHBaseRoot(final MasterFileSystem mfs,
-      final TableDescriptor tableDescriptor, final Path tempMobTableDir) throws IOException {
-    FileSystem fs = mfs.getFileSystem();
-    final Path tableMobDir =
-        MobUtils.getMobTableDir(mfs.getRootDir(), tableDescriptor.getTableName());
-    if (!fs.delete(tableMobDir, true) && fs.exists(tableMobDir)) {
-      throw new IOException("Couldn't delete mob table " + tableMobDir);
-    }
-    if (!fs.exists(tableMobDir.getParent())) {
-      fs.mkdirs(tableMobDir.getParent());
-    }
-    if (!fs.rename(tempMobTableDir, tableMobDir)) {
-      throw new IOException("Unable to move mob table from temp=" + tempMobTableDir
-          + " to hbase root=" + tableMobDir);
-    }
+    return newRegions;
   }
 
   /**
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
index d3c5859..b6de32b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
@@ -21,6 +21,7 @@ import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTra
 
 import java.io.IOException;
 import java.util.Collection;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java
index bd5388c..7c75e46 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java
@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.hbase.snapshot;
 
+import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -53,7 +55,10 @@ import org.apache.hadoop.hbase.monitoring.MonitoredTask;
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
+import org.apache.hadoop.hbase.regionserver.StoreContext;
 import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTracker;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.security.access.AccessControlClient;
 import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil;
@@ -500,18 +505,28 @@ public class RestoreSnapshotHelper {
     String tableName = tableDesc.getTableName().getNameAsString();
     final String snapshotName = snapshotDesc.getName();
 
+    Path regionPath = new Path(tableDir, regionInfo.getEncodedName());
+    HRegionFileSystem regionFS = (fs.exists(regionPath)) ?
+      HRegionFileSystem.openRegionFromFileSystem(conf, fs, tableDir, regionInfo, false) :
+      HRegionFileSystem.createRegionOnFileSystem(conf, fs, tableDir, regionInfo);
+
     // Restore families present in the table
     for (Path familyDir: FSUtils.getFamilyDirs(fs, regionDir)) {
       byte[] family = Bytes.toBytes(familyDir.getName());
+
       Set<String> familyFiles = getTableRegionFamilyFiles(familyDir);
       List<SnapshotRegionManifest.StoreFile> snapshotFamilyFiles =
           snapshotFiles.remove(familyDir.getName());
+      List<StoreFileInfo> filesToTrack = new ArrayList<>();
       if (snapshotFamilyFiles != null) {
         List<SnapshotRegionManifest.StoreFile> hfilesToAdd = new ArrayList<>();
         for (SnapshotRegionManifest.StoreFile storeFile: snapshotFamilyFiles) {
           if (familyFiles.contains(storeFile.getName())) {
             // HFile already present
             familyFiles.remove(storeFile.getName());
+            //no need to restore already present files, but we need to add those to tracker
+            filesToTrack.add(new StoreFileInfo(conf, fs,
+              new Path(familyDir, storeFile.getName()), true));
           } else {
             // HFile missing
             hfilesToAdd.add(storeFile);
@@ -521,9 +536,11 @@ public class RestoreSnapshotHelper {
         // Remove hfiles not present in the snapshot
         for (String hfileName: familyFiles) {
           Path hfile = new Path(familyDir, hfileName);
-          LOG.trace("Removing HFile=" + hfileName + " not present in snapshot=" + snapshotName+
-            " from region=" + regionInfo.getEncodedName() + " table=" + tableName);
-          HFileArchiver.archiveStoreFile(conf, fs, regionInfo, tableDir, family, hfile);
+          if (!fs.getFileStatus(hfile).isDirectory()) {
+            LOG.trace("Removing HFile=" + hfileName + " not present in snapshot=" +
+              snapshotName + " from region=" + regionInfo.getEncodedName() + " table=" + tableName);
+            HFileArchiver.archiveStoreFile(conf, fs, regionInfo, tableDir, family, hfile);
+          }
         }
 
         // Restore Missing files
@@ -531,7 +548,10 @@ public class RestoreSnapshotHelper {
           LOG.debug("Restoring missing HFileLink " + storeFile.getName() +
                   " of snapshot=" + snapshotName+
                   " to region=" + regionInfo.getEncodedName() + " table=" + tableName);
-          restoreStoreFile(familyDir, regionInfo, storeFile, createBackRefs);
+          String fileName = restoreStoreFile(familyDir, regionInfo, storeFile, createBackRefs);
+          //mark the reference file to be added to tracker
+          filesToTrack.add(new StoreFileInfo(conf, fs,
+            new Path(familyDir, fileName), true));
         }
       } else {
         // Family doesn't exists in the snapshot
@@ -540,12 +560,24 @@ public class RestoreSnapshotHelper {
         HFileArchiver.archiveFamilyByFamilyDir(fs, conf, regionInfo, familyDir, family);
         fs.delete(familyDir, true);
       }
+
+      StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true,
+          StoreContext.getBuilder().withFamilyStoreDirectoryPath(familyDir).
+            withRegionFileSystem(regionFS).build());
+
+      //simply reset list of tracked files with the matching files
+      //and the extra one present in the snapshot
+      tracker.set(filesToTrack);
     }
 
     // Add families not present in the table
     for (Map.Entry<String, List<SnapshotRegionManifest.StoreFile>> familyEntry:
                                                                       snapshotFiles.entrySet()) {
       Path familyDir = new Path(regionDir, familyEntry.getKey());
+      StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true,
+          StoreContext.getBuilder().withFamilyStoreDirectoryPath(familyDir).
+            withRegionFileSystem(regionFS).build());
+      List<StoreFileInfo> files = new ArrayList<>();
       if (!fs.mkdirs(familyDir)) {
         throw new IOException("Unable to create familyDir=" + familyDir);
       }
@@ -553,8 +585,10 @@ public class RestoreSnapshotHelper {
       for (SnapshotRegionManifest.StoreFile storeFile: familyEntry.getValue()) {
         LOG.trace("Adding HFileLink (Not present in the table) " + storeFile.getName()
                 + " of snapshot " + snapshotName + " to table=" + tableName);
-        restoreStoreFile(familyDir, regionInfo, storeFile, createBackRefs);
+        String fileName = restoreStoreFile(familyDir, regionInfo, storeFile, createBackRefs);
+        files.add(new StoreFileInfo(conf, fs, new Path(familyDir, fileName), true));
       }
+      tracker.set(files);
     }
   }
 
@@ -627,7 +661,8 @@ public class RestoreSnapshotHelper {
       final RegionInfo region) throws IOException {
     // clone region info (change embedded tableName with the new one)
     Path clonedRegionPath = MobUtils.getMobRegionPath(rootDir, tableDesc.getTableName());
-    cloneRegion(clonedRegionPath, region, regionManifests.get(region.getEncodedName()));
+    cloneRegion(MobUtils.getMobRegionInfo(tableDesc.getTableName()),
+      clonedRegionPath, region, regionManifests.get(region.getEncodedName()));
   }
 
   /**
@@ -641,18 +676,44 @@ public class RestoreSnapshotHelper {
    * @param regionDir {@link Path} cloned dir
    * @param snapshotRegionInfo
    */
-  private void cloneRegion(final Path regionDir, final RegionInfo snapshotRegionInfo,
-      final SnapshotRegionManifest manifest) throws IOException {
+  private void cloneRegion(final RegionInfo newRegionInfo, final Path regionDir,
+      final RegionInfo snapshotRegionInfo, final SnapshotRegionManifest manifest)
+        throws IOException {
     final String tableName = tableDesc.getTableName().getNameAsString();
     final String snapshotName = snapshotDesc.getName();
     for (SnapshotRegionManifest.FamilyFiles familyFiles: manifest.getFamilyFilesList()) {
       Path familyDir = new Path(regionDir, familyFiles.getFamilyName().toStringUtf8());
+      List<StoreFileInfo> clonedFiles = new ArrayList<>();
       for (SnapshotRegionManifest.StoreFile storeFile: familyFiles.getStoreFilesList()) {
         LOG.info("Adding HFileLink " + storeFile.getName() +" from cloned region "
                 + "in snapshot " + snapshotName + " to table=" + tableName);
-        restoreStoreFile(familyDir, snapshotRegionInfo, storeFile, createBackRefs);
+        if (MobUtils.isMobRegionInfo(newRegionInfo)) {
+          String mobFileName = HFileLink.createHFileLinkName(snapshotRegionInfo,
+            storeFile.getName());
+          Path mobPath = new Path(familyDir, mobFileName);
+          if (fs.exists(mobPath)) {
+            fs.delete(mobPath, true);
+          }
+          restoreStoreFile(familyDir, snapshotRegionInfo, storeFile, createBackRefs);
+        } else {
+          String file = restoreStoreFile(familyDir, snapshotRegionInfo, storeFile, createBackRefs);
+          clonedFiles.add(new StoreFileInfo(conf, fs, new Path(familyDir, file), true));
+        }
+      }
+      //we don't need to track files under mobdir
+      if (!MobUtils.isMobRegionInfo(newRegionInfo)) {
+        Path regionPath = new Path(tableDir, newRegionInfo.getEncodedName());
+        HRegionFileSystem regionFS = (fs.exists(regionPath)) ?
+          HRegionFileSystem.openRegionFromFileSystem(conf, fs, tableDir, newRegionInfo, false) :
+          HRegionFileSystem.createRegionOnFileSystem(conf, fs, tableDir, newRegionInfo);
+
+        StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true,
+          StoreContext.getBuilder().withFamilyStoreDirectoryPath(familyDir).
+            withRegionFileSystem(regionFS).build());
+        tracker.set(clonedFiles);
       }
     }
+
   }
 
   /**
@@ -668,7 +729,9 @@ public class RestoreSnapshotHelper {
    */
   private void cloneRegion(final HRegion region, final RegionInfo snapshotRegionInfo,
       final SnapshotRegionManifest manifest) throws IOException {
-    cloneRegion(new Path(tableDir, region.getRegionInfo().getEncodedName()), snapshotRegionInfo,
+    cloneRegion(region.getRegionInfo(),
+      new Path(tableDir, region.getRegionInfo().getEncodedName()),
+      snapshotRegionInfo,
       manifest);
   }
 
@@ -685,16 +748,16 @@ public class RestoreSnapshotHelper {
    * @param createBackRef - Whether back reference should be created. Defaults to true.
    * @param storeFile store file name (can be a Reference, HFileLink or simple HFile)
    */
-  private void restoreStoreFile(final Path familyDir, final RegionInfo regionInfo,
+  private String restoreStoreFile(final Path familyDir, final RegionInfo regionInfo,
       final SnapshotRegionManifest.StoreFile storeFile, final boolean createBackRef)
           throws IOException {
     String hfileName = storeFile.getName();
     if (HFileLink.isHFileLink(hfileName)) {
-      HFileLink.createFromHFileLink(conf, fs, familyDir, hfileName, createBackRef);
+      return HFileLink.createFromHFileLink(conf, fs, familyDir, hfileName, createBackRef);
     } else if (StoreFileInfo.isReference(hfileName)) {
-      restoreReferenceFile(familyDir, regionInfo, storeFile);
+      return restoreReferenceFile(familyDir, regionInfo, storeFile);
     } else {
-      HFileLink.create(conf, fs, familyDir, regionInfo, hfileName, createBackRef);
+      return HFileLink.create(conf, fs, familyDir, regionInfo, hfileName, createBackRef);
     }
   }
 
@@ -716,7 +779,7 @@ public class RestoreSnapshotHelper {
    * @param regionInfo destination region info for the table
    * @param storeFile reference file name
    */
-  private void restoreReferenceFile(final Path familyDir, final RegionInfo regionInfo,
+  private String restoreReferenceFile(final Path familyDir, final RegionInfo regionInfo,
       final SnapshotRegionManifest.StoreFile storeFile) throws IOException {
     String hfileName = storeFile.getName();
 
@@ -760,6 +823,7 @@ public class RestoreSnapshotHelper {
       IOUtils.copyBytes(in, out, conf);
     }
 
+
     // Add the daughter region to the map
     String regionName = Bytes.toString(regionsMap.get(regionInfo.getEncodedNameAsBytes()));
     if (regionName == null) {
@@ -777,6 +841,7 @@ public class RestoreSnapshotHelper {
         daughters.setSecond(regionName);
       }
     }
+    return outPath.getName();
   }
 
   /**
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedureFileBasedSFT.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedureFileBasedSFT.java
new file mode 100644
index 0000000..f3ae128
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedureFileBasedSFT.java
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.procedure;
+
+import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
+import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.Trackers.FILE;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.experimental.categories.Category;
+
+@Category({ MasterTests.class, MediumTests.class})
+public class TestCloneSnapshotProcedureFileBasedSFT extends TestCloneSnapshotProcedure {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestCloneSnapshotProcedureFileBasedSFT.class);
+
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    UTIL.getConfiguration().set(TRACKER_IMPL, FILE.name());
+    UTIL.getConfiguration().setInt(MasterProcedureConstants.MASTER_PROCEDURE_THREADS, 1);
+    UTIL.startMiniCluster(1);
+  }
+}