From 2052e80e5da50ec0598cc7c873de0357a4f894a5 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Tue, 14 Sep 2021 16:28:21 +0800 Subject: [PATCH] HBASE-26248 Should find a suitable way to let users specify the store file tracker implementation (#3665) Signed-off-by: Wellington Chevreuil --- .../MigrationStoreFileTracker.java | 13 ++- .../storefiletracker/StoreFileTracker.java | 7 +- .../StoreFileTrackerBase.java | 12 ++- .../StoreFileTrackerFactory.java | 97 +++++++++++++++++-- .../apache/hadoop/hbase/client/TestAdmin.java | 6 +- .../hadoop/hbase/client/TestAdmin3.java | 6 +- .../hbase/client/TestAsyncTableAdminApi.java | 6 +- .../hbase/client/TestAsyncTableAdminApi3.java | 6 +- .../MasterProcedureTestingUtility.java | 6 +- .../procedure/TestCreateTableProcedure.java | 6 +- .../TestMergesSplitsAddToTracker.java | 4 +- .../TestMigrationStoreFileTracker.java | 27 +++--- ...stRegionWithFileBasedStoreFileTracker.java | 3 +- .../TestStoreFileTrackerFactory.java | 58 +++++++++++ 14 files changed, 202 insertions(+), 55 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerFactory.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java index 483a240baded..3eeef9000576 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.procedure2.util.StringUtils; import org.apache.hadoop.hbase.regionserver.StoreContext; import org.apache.hadoop.hbase.regionserver.StoreFileInfo; import org.apache.yetus.audience.InterfaceAudience; @@ -44,8 +45,8 @@ class MigrationStoreFileTracker extends StoreFileTrackerBase { public MigrationStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) { super(conf, isPrimaryReplica, ctx); - this.src = StoreFileTrackerFactory.create(conf, SRC_IMPL, isPrimaryReplica, ctx); - this.dst = StoreFileTrackerFactory.create(conf, DST_IMPL, isPrimaryReplica, ctx); + this.src = StoreFileTrackerFactory.createForMigration(conf, SRC_IMPL, isPrimaryReplica, ctx); + this.dst = StoreFileTrackerFactory.createForMigration(conf, DST_IMPL, isPrimaryReplica, ctx); Preconditions.checkArgument(!src.getClass().equals(dst.getClass()), "src and dst is the same: %s", src.getClass()); } @@ -90,7 +91,11 @@ void set(List files) { @Override public void persistConfiguration(TableDescriptorBuilder builder) { super.persistConfiguration(builder); - builder.setValue(SRC_IMPL, src.getClass().getName()); - builder.setValue(DST_IMPL, dst.getClass().getName()); + if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) { + builder.setValue(SRC_IMPL, src.getTrackerName()); + } + if (StringUtils.isEmpty(builder.getValue(DST_IMPL))) { + builder.setValue(DST_IMPL, dst.getTrackerName()); + } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java index 81fa1a9be5b2..59fe7ef52f96 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java @@ -75,7 +75,12 @@ void replace(Collection compactedFiles, Collection StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException; /** - * Saves StoreFileTracker implementations specific configs into the table descriptors. + * Saves StoreFileTracker implementations specific configurations into the table descriptors. + *

+ * This is used to avoid accidentally data loss when changing the cluster level store file tracker + * implementation, and also possible misconfiguration between master and region servers. + *

+ * See HBASE-26246 for more details. * @param builder The table descriptor builder for the given table. */ void persistConfiguration(TableDescriptorBuilder builder); 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 83ebbc78ab35..a786add49b21 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 @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hbase.regionserver.storefiletracker; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import java.io.IOException; import java.util.Collection; @@ -84,13 +84,15 @@ public final void replace(Collection compactedFiles, @Override public void persistConfiguration(TableDescriptorBuilder builder) { - if (StringUtils.isEmpty(builder.getValue(TRACK_IMPL))) { - String trackerImpl = StoreFileTrackerFactory. - getStoreFileTrackerImpl(conf).getName(); - builder.setValue(TRACK_IMPL, trackerImpl).build(); + if (StringUtils.isEmpty(builder.getValue(TRACKER_IMPL))) { + builder.setValue(TRACKER_IMPL, getTrackerName()); } } + protected final String getTrackerName() { + return StoreFileTrackerFactory.getStoreFileTrackerName(getClass()); + } + private HFileContext createFileContext(Compression.Algorithm compression, boolean includeMVCCReadpoint, boolean includesTag, Encryption.Context encryptionContext) { if (compression == null) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java index b9ec713cf235..9be19ec15ed8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java @@ -15,6 +15,9 @@ */ package org.apache.hadoop.hbase.regionserver.storefiletracker; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; @@ -33,22 +36,81 @@ /** * Factory method for creating store file tracker. + *

+ * The current implementations are: + *

    + *
  • default: DefaultStoreFileTracker, see {@link DefaultStoreFileTracker}.
    • + *
  • file:FileBasedStoreFileTracker, see {@link FileBasedStoreFileTracker}.
    • + *
  • migration:MigrationStoreFileTracker, see {@link MigrationStoreFileTracker}.
    • + *
+ * @see DefaultStoreFileTracker + * @see FileBasedStoreFileTracker + * @see MigrationStoreFileTracker */ -@InterfaceAudience.Private public final class StoreFileTrackerFactory { - public static final String TRACK_IMPL = "hbase.store.file-tracker.impl"; +@InterfaceAudience.Private +public final class StoreFileTrackerFactory { + private static final Logger LOG = LoggerFactory.getLogger(StoreFileTrackerFactory.class); - public static Class getStoreFileTrackerImpl(Configuration conf) { - return conf.getClass(TRACK_IMPL, DefaultStoreFileTracker.class, StoreFileTracker.class); + public static final String TRACKER_IMPL = "hbase.store.file-tracker.impl"; + + /** + * Maps between configuration names for trackers and implementation classes. + */ + public enum Trackers { + DEFAULT(DefaultStoreFileTracker.class), FILE(FileBasedStoreFileTracker.class), + MIGRATION(MigrationStoreFileTracker.class); + + final Class clazz; + + Trackers(Class clazz) { + this.clazz = clazz; + } + } + + private static final Map, Trackers> CLASS_TO_ENUM = reverse(); + + private static Map, Trackers> reverse() { + Map, Trackers> map = new HashMap<>(); + for (Trackers tracker : Trackers.values()) { + map.put(tracker.clazz, tracker); + } + return Collections.unmodifiableMap(map); + } + + private StoreFileTrackerFactory() { + } + + public static String getStoreFileTrackerName(Configuration conf) { + return conf.get(TRACKER_IMPL, Trackers.DEFAULT.name()); + } + + static String getStoreFileTrackerName(Class clazz) { + Trackers name = CLASS_TO_ENUM.get(clazz); + return name != null ? name.name() : clazz.getName(); + } + + private static Class getTrackerClass(Configuration conf) { + try { + Trackers tracker = Trackers.valueOf(getStoreFileTrackerName(conf).toUpperCase()); + return tracker.clazz; + } catch (IllegalArgumentException e) { + // Fall back to them specifying a class name + return conf.getClass(TRACKER_IMPL, Trackers.DEFAULT.clazz, StoreFileTracker.class); + } } public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) { - Class tracker = getStoreFileTrackerImpl(conf); + Class tracker = getTrackerClass(conf); LOG.info("instantiating StoreFileTracker impl {}", tracker.getName()); return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx); } + /** + * 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, boolean isPrimaryReplica, String family, HRegionFileSystem regionFs) { ColumnFamilyDescriptorBuilder fDescBuilder = @@ -63,15 +125,30 @@ public static Configuration mergeConfigurations(Configuration global, TableDescr return StoreUtils.createStoreConfiguration(global, table, family); } - static StoreFileTrackerBase create(Configuration conf, String configName, + /** + * Create store file tracker to be used as source or destination for + * {@link MigrationStoreFileTracker}. + */ + static StoreFileTrackerBase createForMigration(Configuration conf, String configName, boolean isPrimaryReplica, StoreContext ctx) { - String className = + String trackerName = Preconditions.checkNotNull(conf.get(configName), "config %s is not set", configName); Class tracker; try { - tracker = Class.forName(className).asSubclass(StoreFileTrackerBase.class); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); + tracker = + Trackers.valueOf(trackerName.toUpperCase()).clazz.asSubclass(StoreFileTrackerBase.class); + } catch (IllegalArgumentException e) { + // Fall back to them specifying a class name + try { + tracker = Class.forName(trackerName).asSubclass(StoreFileTrackerBase.class); + } catch (ClassNotFoundException cnfe) { + throw new RuntimeException(cnfe); + } + } + // prevent nest of MigrationStoreFileTracker, it will cause infinite recursion. + if (MigrationStoreFileTracker.class.isAssignableFrom(tracker)) { + throw new IllegalArgumentException("Should not specify " + configName + " as " + + Trackers.MIGRATION + " because it can not be nested"); } LOG.info("instantiating StoreFileTracker impl {} as {}", tracker.getName(), configName); return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java index 10ab4961a5b4..ba843fe43460 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.client; import static org.apache.hadoop.hbase.HBaseTestingUtil.countRows; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -426,8 +426,8 @@ private void testCloneTableSchema(final TableName tableName, final TableName new assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled()); assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive()); // HBASE-26246 introduced persist of store file tracker into table descriptor - tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL, - StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()). + tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL, + StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())). build(); TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin3.java index 7d40fd12bda5..c2de0fbd3555 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin3.java @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hbase.client; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -239,8 +239,8 @@ public void testGetTableDescriptor() throws IOException { Table table = TEST_UTIL.getConnection().getTable(htd.getTableName()); TableDescriptor confirmedHtd = table.getDescriptor(); //HBASE-26246 introduced persist of store file tracker into table descriptor - htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACK_IMPL, - StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()). + htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACKER_IMPL, + StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())). build(); assertEquals(0, TableDescriptor.COMPARATOR.compare(htd, confirmedHtd)); MetaTableAccessor.fullScanMetaAndPrint(TEST_UTIL.getConnection()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi.java index a1832f49956a..147630b53a38 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.client; import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -375,8 +375,8 @@ private void testCloneTableSchema(final TableName tableName, assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled()); assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive()); //HBASE-26246 introduced persist of store file tracker into table descriptor - tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL, - StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()). + tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL, + StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())). build(); TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi3.java index cb89bf564720..67377b07c180 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableAdminApi3.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.client; import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -150,8 +150,8 @@ public void testGetTableDescriptor() throws Exception { admin.createTable(desc).join(); TableDescriptor confirmedHtd = admin.getDescriptor(tableName).get(); //HBASE-26246 introduced persist of store file tracker into table descriptor - desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACK_IMPL, - StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()). + desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACKER_IMPL, + StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())). build(); assertEquals(0, TableDescriptor.COMPARATOR.compare(desc, confirmedHtd)); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java index 2ceec5f241c6..726e117ef1ac 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.master.procedure; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -231,8 +231,8 @@ public static void validateTableCreation(final HMaster master, final TableName t // checks store file tracker impl has been properly set in htd String storeFileTrackerImpl = - StoreFileTrackerFactory.getStoreFileTrackerImpl(master.getConfiguration()).getName(); - assertEquals(storeFileTrackerImpl, htd.getValue(TRACK_IMPL)); + StoreFileTrackerFactory.getStoreFileTrackerName(master.getConfiguration()); + assertEquals(storeFileTrackerImpl, htd.getValue(TRACKER_IMPL)); } public static void validateTableDeletion( diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java index 0bc77f0ef8d9..f432c8060d3d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java @@ -17,7 +17,7 @@ */ package org.apache.hadoop.hbase.master.procedure; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -96,13 +96,13 @@ public void testCreateWithTrackImpl() throws Exception { ProcedureExecutor procExec = getMasterProcedureExecutor(); TableDescriptor htd = MasterProcedureTestingUtility.createHTD(tableName, F1); String trackerName = TestStoreFileTracker.class.getName(); - htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACK_IMPL, trackerName).build(); + htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACKER_IMPL, trackerName).build(); RegionInfo[] regions = ModifyRegionUtils.createRegionInfos(htd, null); long procId = ProcedureTestingUtility.submitAndWait(procExec, new CreateTableProcedure(procExec.getEnvironment(), htd, regions)); ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId)); htd = getMaster().getTableDescriptors().get(tableName); - assertEquals(trackerName, htd.getValue(TRACK_IMPL)); + assertEquals(trackerName, htd.getValue(TRACKER_IMPL)); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java index c6205cb18493..68fc444493c4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.regionserver; import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory. - TRACK_IMPL; + TRACKER_IMPL; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -74,7 +74,7 @@ public class TestMergesSplitsAddToTracker { @BeforeClass public static void setupClass() throws Exception { - TEST_UTIL.getConfiguration().set(TRACK_IMPL, TestStoreFileTracker.class.getName()); + TEST_UTIL.getConfiguration().set(TRACKER_IMPL, TestStoreFileTracker.class.getName()); TEST_UTIL.startMiniCluster(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestMigrationStoreFileTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestMigrationStoreFileTracker.java index e9ebb4c792e4..119f8fb553f8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestMigrationStoreFileTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestMigrationStoreFileTracker.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; @@ -86,10 +85,10 @@ public class TestMigrationStoreFileTracker { public TestName name = new TestName(); @Parameter(0) - public Class srcImplClass; + public StoreFileTrackerFactory.Trackers srcImpl; @Parameter(1) - public Class dstImplClass; + public StoreFileTrackerFactory.Trackers dstImpl; private HRegion region; @@ -99,11 +98,13 @@ public class TestMigrationStoreFileTracker { @Parameters(name = "{index}: src={0}, dst={1}") public static List params() { - List> impls = - Arrays.asList(DefaultStoreFileTracker.class, FileBasedStoreFileTracker.class); List params = new ArrayList<>(); - for (Class src : impls) { - for (Class dst : impls) { + for (StoreFileTrackerFactory.Trackers src : StoreFileTrackerFactory.Trackers.values()) { + for (StoreFileTrackerFactory.Trackers dst : StoreFileTrackerFactory.Trackers.values()) { + if (src == StoreFileTrackerFactory.Trackers.MIGRATION + || dst == StoreFileTrackerFactory.Trackers.MIGRATION) { + continue; + } if (src.equals(dst)) { continue; } @@ -122,8 +123,8 @@ public static void setUpBeforeClass() { @Before public void setUp() throws IOException { Configuration conf = UTIL.getConfiguration(); - conf.setClass(MigrationStoreFileTracker.SRC_IMPL, srcImplClass, StoreFileTrackerBase.class); - conf.setClass(MigrationStoreFileTracker.DST_IMPL, dstImplClass, StoreFileTrackerBase.class); + conf.set(MigrationStoreFileTracker.SRC_IMPL, srcImpl.name().toLowerCase()); + conf.set(MigrationStoreFileTracker.DST_IMPL, dstImpl.name().toLowerCase()); rootDir = UTIL.getDataTestDir(name.getMethodName().replaceAll("[=:\\[ ]", "_")); wal = HBaseTestingUtil.createWal(conf, rootDir, RI); } @@ -145,7 +146,7 @@ private List getStoreFiles() { private HRegion createRegion(Class trackerImplClass) throws IOException { Configuration conf = new Configuration(UTIL.getConfiguration()); - conf.setClass(StoreFileTrackerFactory.TRACK_IMPL, trackerImplClass, StoreFileTracker.class); + conf.setClass(StoreFileTrackerFactory.TRACKER_IMPL, trackerImplClass, StoreFileTracker.class); return HRegion.createHRegion(RI, rootDir, conf, TD, wal, true); } @@ -155,7 +156,7 @@ private void reopenRegion(Class trackerImplClass List before = getStoreFiles(); region.close(); Configuration conf = new Configuration(UTIL.getConfiguration()); - conf.setClass(StoreFileTrackerFactory.TRACK_IMPL, trackerImplClass, StoreFileTracker.class); + conf.setClass(StoreFileTrackerFactory.TRACKER_IMPL, trackerImplClass, StoreFileTracker.class); region = HRegion.openHRegion(rootDir, RI, TD, wal, conf); List after = getStoreFiles(); assertEquals(before.size(), after.size()); @@ -180,14 +181,14 @@ private void verifyData(int start, int end) throws IOException { @Test public void testMigration() throws IOException { - region = createRegion(srcImplClass); + region = createRegion(srcImpl.clazz.asSubclass(StoreFileTrackerBase.class)); putData(0, 100); verifyData(0, 100); reopenRegion(MigrationStoreFileTracker.class); verifyData(0, 100); region.compact(true); putData(100, 200); - reopenRegion(dstImplClass); + reopenRegion(dstImpl.clazz.asSubclass(StoreFileTrackerBase.class)); verifyData(0, 200); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestRegionWithFileBasedStoreFileTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestRegionWithFileBasedStoreFileTracker.java index 3bc60d13fd94..77cd3c448fa0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestRegionWithFileBasedStoreFileTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestRegionWithFileBasedStoreFileTracker.java @@ -71,8 +71,7 @@ public class TestRegionWithFileBasedStoreFileTracker { @Before public void setUp() throws IOException { Configuration conf = new Configuration(UTIL.getConfiguration()); - conf.setClass(StoreFileTrackerFactory.TRACK_IMPL, FileBasedStoreFileTracker.class, - StoreFileTracker.class); + conf.set(StoreFileTrackerFactory.TRACKER_IMPL, StoreFileTrackerFactory.Trackers.FILE.name()); region = HBaseTestingUtil.createRegionAndWAL(RI, UTIL.getDataTestDir(name.getMethodName()), conf, TD); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerFactory.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerFactory.java new file mode 100644 index 000000000000..41f2afdfa421 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerFactory.java @@ -0,0 +1,58 @@ +/** + * 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.regionserver.storefiletracker; + +import static org.junit.Assert.assertThrows; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.regionserver.StoreContext; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ RegionServerTests.class, SmallTests.class }) +public class TestStoreFileTrackerFactory { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestStoreFileTrackerFactory.class); + + @Test + public void testCreateForMigration() { + Configuration conf = HBaseConfiguration.create(); + String configName = "config"; + + // no config + assertThrows(NullPointerException.class, () -> StoreFileTrackerFactory.createForMigration(conf, + configName, false, StoreContext.getBuilder().build())); + + // class not found + conf.set(configName, "config"); + assertThrows(RuntimeException.class, () -> StoreFileTrackerFactory.createForMigration(conf, + configName, false, StoreContext.getBuilder().build())); + + // nested MigrationStoreFileTracker + conf.setClass(configName, MigrationStoreFileTracker.class, StoreFileTrackerBase.class); + assertThrows(IllegalArgumentException.class, () -> StoreFileTrackerFactory + .createForMigration(conf, configName, false, StoreContext.getBuilder().build())); + } +}