HBASE-26280 Use store file tracker when snapshoting (apache#3685)

Signed-off-by: Wellington Chevreuil <[email protected]>
Reviewed-by: Josh Elser <[email protected]>

* Fix broken JUnit parameterization
* Also adds an one-liner fix to HBTU to be smarter when dropping tables.

Change-Id: I11bed0526715781d14476207e60ffa13cdbcdeba

hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java

@@ -613,9 +613,8 @@ private List<Path> mergeStoreFiles(MasterProcedureEnv env, HRegionFileSystem reg
     List<Path> mergedFiles = new ArrayList<>();
     for (ColumnFamilyDescriptor hcd : htd.getColumnFamilies()) {
       String family = hcd.getNameAsString();
-      Configuration trackerConfig =
-        StoreFileTrackerFactory.mergeConfigurations(env.getMasterConfiguration(), htd, hcd);
-      StoreFileTracker tracker = StoreFileTrackerFactory.create(trackerConfig, family, regionFs);
+      StoreFileTracker tracker =
+        StoreFileTrackerFactory.create(env.getMasterConfiguration(), htd, hcd, regionFs);
       final Collection<StoreFileInfo> storeFiles = tracker.load();
       if (storeFiles != null && storeFiles.size() > 0) {
         for (StoreFileInfo storeFileInfo : storeFiles) {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java

@@ -670,9 +670,8 @@ private Pair<List<Path>, List<Path>> splitStoreFiles(final MasterProcedureEnv en
         new HashMap<String, Collection<StoreFileInfo>>(htd.getColumnFamilyCount());
     for (ColumnFamilyDescriptor cfd : htd.getColumnFamilies()) {
       String family = cfd.getNameAsString();
-      Configuration trackerConfig = StoreFileTrackerFactory.
-        mergeConfigurations(env.getMasterConfiguration(), htd, htd.getColumnFamily(cfd.getName()));
-      StoreFileTracker tracker = StoreFileTrackerFactory.create(trackerConfig, family, regionFs);
+      StoreFileTracker tracker =
+        StoreFileTrackerFactory.create(env.getMasterConfiguration(), htd, cfd, regionFs);
       Collection<StoreFileInfo> sfis = tracker.load();
       if (sfis == null) {
         continue;

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java

@@ -598,7 +598,6 @@ void cleanupDaughterRegion(final RegionInfo regionInfo) throws IOException {
    * to the proper location in the filesystem.
    *
    * @param regionInfo daughter {@link org.apache.hadoop.hbase.client.RegionInfo}
-   * @throws IOException
    */
   public Path commitDaughterRegion(final RegionInfo regionInfo, List<Path> allRegionFiles,
       MasterProcedureEnv env) throws IOException {
@@ -625,12 +624,8 @@ private void insertRegionFilesIntoStoreTracker(List<Path> allFiles, MasterProced
     Map<String, List<StoreFileInfo>> fileInfoMap = new HashMap<>();
     for(Path file : allFiles) {
       String familyName = file.getParent().getName();
-      trackerMap.computeIfAbsent(familyName, t -> {
-        Configuration config = StoreFileTrackerFactory.mergeConfigurations(conf, tblDesc,
-          tblDesc.getColumnFamily(Bytes.toBytes(familyName)));
-        return StoreFileTrackerFactory.
-          create(config, familyName, regionFs);
-      });
+      trackerMap.computeIfAbsent(familyName, t -> StoreFileTrackerFactory.create(conf, tblDesc,
+        tblDesc.getColumnFamily(Bytes.toBytes(familyName)), regionFs));
       fileInfoMap.computeIfAbsent(familyName, l -> new ArrayList<>());
       List<StoreFileInfo> infos = fileInfoMap.get(familyName);
       infos.add(new StoreFileInfo(conf, fs, fs.getFileStatus(file)));
@@ -676,7 +671,6 @@ public void createSplitsDir(RegionInfo daughterA, RegionInfo daughterB) throws I
    *                    this method is invoked on the Master side, then the RegionSplitPolicy will
    *                    NOT have a reference to a Region.
    * @return Path to created reference.
-   * @throws IOException
    */
   public Path splitStoreFile(RegionInfo hri, String familyName, HStoreFile f, byte[] splitRow,
       boolean top, RegionSplitPolicy splitPolicy) throws IOException {

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java

@@ -23,12 +23,10 @@
 import org.apache.hadoop.hbase.CompoundConfiguration;
 import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
-import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
 import org.apache.hadoop.hbase.regionserver.StoreContext;
-import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.ReflectionUtils;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
@@ -113,18 +111,17 @@ public static StoreFileTracker create(Configuration conf, boolean isPrimaryRepli
    * Used at master side when splitting/merging regions, as we do not have a Store, thus no
    * StoreContext at master side.
    */
-  public static StoreFileTracker create(Configuration conf, String family,
-    HRegionFileSystem regionFs) {
-    ColumnFamilyDescriptorBuilder fDescBuilder =
-      ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes(family));
-    StoreContext ctx = StoreContext.getBuilder().withColumnFamilyDescriptor(fDescBuilder.build())
-      .withRegionFileSystem(regionFs)
-      .withFamilyStoreDirectoryPath(regionFs.getStoreDir(family))
+  public static StoreFileTracker create(Configuration conf, TableDescriptor td,
+    ColumnFamilyDescriptor cfd, HRegionFileSystem regionFs) {
+    StoreContext ctx =
+      StoreContext.getBuilder().withColumnFamilyDescriptor(cfd).withRegionFileSystem(regionFs)
+        .withFamilyStoreDirectoryPath(regionFs.getStoreDir(cfd.getNameAsString()))
+      .withFamilyStoreDirectoryPath(regionFs.getStoreDir(cfd.getNameAsString()))
       .build();
-    return StoreFileTrackerFactory.create(conf, true, ctx);
+    return StoreFileTrackerFactory.create(mergeConfigurations(conf, td, cfd), true, ctx);
   }
 
-  public static Configuration mergeConfigurations(Configuration global, TableDescriptor table,
+  private static Configuration mergeConfigurations(Configuration global, TableDescriptor table,
     ColumnFamilyDescriptor family) {
     return new CompoundConfiguration().add(global).addBytesMap(table.getValues())
       .addStringMap(family.getConfiguration()).addBytesMap(family.getValues());

hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifest.java

@@ -47,7 +47,8 @@
 import org.apache.hadoop.hbase.regionserver.HStore;
 import org.apache.hadoop.hbase.regionserver.HStoreFile;
 import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
-import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTracker;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.util.CommonFSUtils;
 import org.apache.hadoop.hbase.util.FSTableDescriptors;
 import org.apache.hadoop.hbase.util.Threads;
@@ -317,26 +318,19 @@ protected void addRegion(final Path tableDir, final RegionInfo regionInfo, Regio
       // in batches and may miss files being added/deleted. This could be more robust (iteratively
       // checking to see if we have all the files until we are sure), but the limit is currently
       // 1000 files/batch, far more than the number of store files under a single column family.
-      Collection<String> familyNames = regionFs.getFamilies();
-      if (familyNames != null) {
-        for (String familyName: familyNames) {
-          Object familyData = visitor.familyOpen(regionData, Bytes.toBytes(familyName));
-          monitor.rethrowException();
-
-          Collection<StoreFileInfo> storeFiles = regionFs.getStoreFiles(familyName);
-          if (storeFiles == null) {
-            if (LOG.isDebugEnabled()) {
-              LOG.debug("No files under family: " + familyName);
-            }
-            continue;
-          }
-
-          // 2.1. build the snapshot reference for the store
-          // iterate through all the store's files and create "references".
-          addReferenceFiles(visitor, regionData, familyData, storeFiles, false);
-
-          visitor.familyClose(regionData, familyData);
+      for (ColumnFamilyDescriptor cfd : htd.getColumnFamilies()) {
+        Object familyData = visitor.familyOpen(regionData, cfd.getName());
+        monitor.rethrowException();
+        StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, htd, cfd, regionFs);
+        List<StoreFileInfo> storeFiles = tracker.load();
+        if (storeFiles.isEmpty()) {
+          LOG.debug("No files under family: {}", cfd.getNameAsString());
+          continue;
         }
+        // 2.1. build the snapshot reference for the store
+        // iterate through all the store's files and create "references".
+        addReferenceFiles(visitor, regionData, familyData, storeFiles, false);
+        visitor.familyClose(regionData, familyData);
       }
       visitor.regionClose(regionData);
     } catch (IOException e) {

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java

@@ -1950,6 +1950,9 @@ public static void setReplicas(Admin admin, TableName table, int replicaCount)
    * @param tableName existing table
    */
   public void deleteTable(TableName tableName) throws IOException {
+    if (!getAdmin().tableExists(tableName)) {
+      return;
+    }
     try {
       getAdmin().disableTable(tableName);
     } catch (TableNotEnabledException e) {

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMobCloneSnapshotFromClientCloneLinksAfterDelete.java

@@ -30,6 +30,7 @@
 import org.apache.hadoop.hbase.master.cleaner.TimeToLiveHFileCleaner;
 import org.apache.hadoop.hbase.mob.MobConstants;
 import org.apache.hadoop.hbase.regionserver.FlushLifeCycleTracker;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.snapshot.MobSnapshotTestingUtils;
 import org.apache.hadoop.hbase.snapshot.SnapshotTestingUtils;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
@@ -91,7 +92,8 @@ public static void setUpBeforeClass() throws Exception {
   @Override
   protected void createTable() throws IOException, InterruptedException {
     MobSnapshotTestingUtils.createMobTable(TEST_UTIL, tableName,
-      SnapshotTestingUtils.getSplitKeys(), getNumReplicas(), DelayFlushCoprocessor.class.getName(),
+      SnapshotTestingUtils.getSplitKeys(), getNumReplicas(),
+      StoreFileTrackerFactory.Trackers.DEFAULT.name(), DelayFlushCoprocessor.class.getName(),
       FAMILY);
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMobSnapshotFromClient.java

@@ -20,28 +20,32 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.mob.MobConstants;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.snapshot.MobSnapshotTestingUtils;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.experimental.categories.Category;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 /**
  * Test create/using/deleting snapshots from the client
  * <p>
  * This is an end-to-end test for the snapshot utility
  */
 @Category({LargeTests.class, ClientTests.class})
+@RunWith(Parameterized.class)
 public class TestMobSnapshotFromClient extends TestSnapshotFromClient {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
       HBaseClassTestRule.forClass(TestMobSnapshotFromClient.class);
 
-  private static final Logger LOG = LoggerFactory.getLogger(TestMobSnapshotFromClient.class);
+  public TestMobSnapshotFromClient(StoreFileTrackerFactory.Trackers trackerImpl) {
+    super(trackerImpl);
+  }
 
   /**
    * Setup the config for the cluster
@@ -60,6 +64,7 @@ protected static void setupConf(Configuration conf) {
 
   @Override
   protected void createTable() throws Exception {
-    MobSnapshotTestingUtils.createMobTable(UTIL, TABLE_NAME, getNumReplicas(), TEST_FAM);
+    MobSnapshotTestingUtils.createMobTable(UTIL, TABLE_NAME, getNumReplicas(), trackerImpl.name(),
+      TEST_FAM);
   }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotFromClient.java

@@ -23,6 +23,7 @@
 import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.regex.Pattern;
 import org.apache.hadoop.conf.Configuration;
@@ -33,9 +34,12 @@
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNameTestRule;
 import org.apache.hadoop.hbase.TableNotFoundException;
 import org.apache.hadoop.hbase.master.snapshot.SnapshotManager;
 import org.apache.hadoop.hbase.regionserver.ConstantSizeRegionSplitPolicy;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTracker;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
 import org.apache.hadoop.hbase.snapshot.SnapshotDoesNotExistException;
 import org.apache.hadoop.hbase.snapshot.SnapshotManifestV1;
@@ -52,7 +56,10 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.TestName;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -66,6 +73,7 @@
  * This is an end-to-end test for the snapshot utility
  */
 @Category({LargeTests.class, ClientTests.class})
+@RunWith(Parameterized.class)
 public class TestSnapshotFromClient {
 
   @ClassRule
@@ -83,7 +91,19 @@ public class TestSnapshotFromClient {
   private static final Pattern MATCH_ALL = Pattern.compile(".*");
 
   @Rule
-  public TestName name = new TestName();
+  public TableNameTestRule name = new TableNameTestRule();
+
+  StoreFileTrackerFactory.Trackers trackerImpl;
+
+  @Parameters(name = "{index}: tracker={0}")
+  public static List<Object[]> params() {
+    return Arrays.asList(new Object[] { StoreFileTrackerFactory.Trackers.DEFAULT },
+      new Object[] { StoreFileTrackerFactory.Trackers.FILE });
+  }
+
+  public TestSnapshotFromClient(StoreFileTrackerFactory.Trackers tracker) {
+    this.trackerImpl = tracker;
+  }
 
   /**
    * Setup the config for the cluster
@@ -110,7 +130,6 @@ protected static void setupConf(Configuration conf) {
     conf.setBoolean(SnapshotManager.HBASE_SNAPSHOT_ENABLED, true);
     conf.set(HConstants.HBASE_REGION_SPLIT_POLICY_KEY,
         ConstantSizeRegionSplitPolicy.class.getName());
-
   }
 
   @Before
@@ -119,9 +138,10 @@ public void setup() throws Exception {
   }
 
   protected void createTable() throws Exception {
-    HTableDescriptor htd = new HTableDescriptor(TABLE_NAME);
-    htd.setRegionReplication(getNumReplicas());
-    UTIL.createTable(htd, new byte[][]{TEST_FAM}, null);
+    TableDescriptor htd =
+      TableDescriptorBuilder.newBuilder(TABLE_NAME).setRegionReplication(getNumReplicas())
+        .setValue(StoreFileTrackerFactory.TRACKER_IMPL, trackerImpl.name()).build();
+    UTIL.createTable(htd, new byte[][] { TEST_FAM }, null);
   }
 
   protected int getNumReplicas() {
@@ -326,7 +346,7 @@ public void testOfflineTableSnapshotWithEmptyRegions() throws Exception {
   @Test
   public void testListTableSnapshots() throws Exception {
     Admin admin = null;
-    final TableName tableName = TableName.valueOf(name.getMethodName());
+    final TableName tableName = name.getTableName();
     try {
       admin = UTIL.getAdmin();
 
@@ -411,7 +431,7 @@ public void testListTableSnapshotsWithRegex() throws Exception {
   @Test
   public void testDeleteTableSnapshots() throws Exception {
     Admin admin = null;
-    final TableName tableName = TableName.valueOf(name.getMethodName());
+    final TableName tableName = name.getTableName();
     try {
       admin = UTIL.getAdmin();
 

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSnapshotFromClientWithRegionReplicas.java

@@ -18,18 +18,26 @@
 package org.apache.hadoop.hbase.client;
 
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.junit.ClassRule;
 import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 @Category({LargeTests.class, ClientTests.class})
+@RunWith(Parameterized.class)
 public class TestSnapshotFromClientWithRegionReplicas extends TestSnapshotFromClient {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
       HBaseClassTestRule.forClass(TestSnapshotFromClientWithRegionReplicas.class);
 
+  public TestSnapshotFromClientWithRegionReplicas(StoreFileTrackerFactory.Trackers trackerImpl) {
+    super(trackerImpl);
+  }
+
   @Override
   protected int getNumReplicas() {
     return 3;

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java

@@ -61,6 +61,7 @@
 import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper;
 import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.io.FSDataInputStreamWrapper;
 import org.apache.hadoop.hbase.io.HFileLink;
 import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
@@ -1110,10 +1111,9 @@ private Path splitStoreFile(final HRegionFileSystem regionFs, final HRegionInfo
     when(mockEnv.getMasterConfiguration()).thenReturn(new Configuration());
     TableDescriptors mockTblDescs = mock(TableDescriptors.class);
     when(mockServices.getTableDescriptors()).thenReturn(mockTblDescs);
-    TableDescriptor mockTblDesc = mock(TableDescriptor.class);
+    TableDescriptor mockTblDesc = TableDescriptorBuilder.newBuilder(hri.getTable())
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of(family)).build();
     when(mockTblDescs.get(any())).thenReturn(mockTblDesc);
-    ColumnFamilyDescriptor mockCfDesc = mock(ColumnFamilyDescriptor.class);
-    when(mockTblDesc.getColumnFamily(any())).thenReturn(mockCfDesc);
     Path regionDir = regionFs.commitDaughterRegion(hri, splitFiles, mockEnv);
     return new Path(new Path(regionDir, family), path.getName());
   }