Skip to content

Commit 989028b

Browse files
Apache9Duo Zhang
Apache9
authored and
Duo Zhang
committed
HBASE-26248 Should find a suitable way to let users specify the store file tracker implementation (#3665)
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
1 parent 0ce891a commit 989028b

14 files changed

+202
-55
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import org.apache.hadoop.conf.Configuration;
2424
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
25+
import org.apache.hadoop.hbase.procedure2.util.StringUtils;
2526
import org.apache.hadoop.hbase.regionserver.StoreContext;
2627
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
2728
import org.apache.yetus.audience.InterfaceAudience;
@@ -44,8 +45,8 @@ class MigrationStoreFileTracker extends StoreFileTrackerBase {
4445

4546
public MigrationStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) {
4647
super(conf, isPrimaryReplica, ctx);
47-
this.src = StoreFileTrackerFactory.create(conf, SRC_IMPL, isPrimaryReplica, ctx);
48-
this.dst = StoreFileTrackerFactory.create(conf, DST_IMPL, isPrimaryReplica, ctx);
48+
this.src = StoreFileTrackerFactory.createForMigration(conf, SRC_IMPL, isPrimaryReplica, ctx);
49+
this.dst = StoreFileTrackerFactory.createForMigration(conf, DST_IMPL, isPrimaryReplica, ctx);
4950
Preconditions.checkArgument(!src.getClass().equals(dst.getClass()),
5051
"src and dst is the same: %s", src.getClass());
5152
}
@@ -90,7 +91,11 @@ void set(List<StoreFileInfo> files) {
9091
@Override
9192
public void persistConfiguration(TableDescriptorBuilder builder) {
9293
super.persistConfiguration(builder);
93-
builder.setValue(SRC_IMPL, src.getClass().getName());
94-
builder.setValue(DST_IMPL, dst.getClass().getName());
94+
if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
95+
builder.setValue(SRC_IMPL, src.getTrackerName());
96+
}
97+
if (StringUtils.isEmpty(builder.getValue(DST_IMPL))) {
98+
builder.setValue(DST_IMPL, dst.getTrackerName());
99+
}
95100
}
96101
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
7575
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
7676

7777
/**
78-
* Saves StoreFileTracker implementations specific configs into the table descriptors.
78+
* Saves StoreFileTracker implementations specific configurations into the table descriptors.
79+
* <p/>
80+
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
81+
* implementation, and also possible misconfiguration between master and region servers.
82+
* <p/>
83+
* See HBASE-26246 for more details.
7984
* @param builder The table descriptor builder for the given table.
8085
*/
8186
void persistConfiguration(TableDescriptorBuilder builder);

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.regionserver.storefiletracker;
1919

20-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
20+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2121

2222
import java.io.IOException;
2323
import java.util.Collection;
@@ -84,13 +84,15 @@ public final void replace(Collection<StoreFileInfo> compactedFiles,
8484

8585
@Override
8686
public void persistConfiguration(TableDescriptorBuilder builder) {
87-
if (StringUtils.isEmpty(builder.getValue(TRACK_IMPL))) {
88-
String trackerImpl = StoreFileTrackerFactory.
89-
getStoreFileTrackerImpl(conf).getName();
90-
builder.setValue(TRACK_IMPL, trackerImpl).build();
87+
if (StringUtils.isEmpty(builder.getValue(TRACKER_IMPL))) {
88+
builder.setValue(TRACKER_IMPL, getTrackerName());
9189
}
9290
}
9391

92+
protected final String getTrackerName() {
93+
return StoreFileTrackerFactory.getStoreFileTrackerName(getClass());
94+
}
95+
9496
private HFileContext createFileContext(Compression.Algorithm compression,
9597
boolean includeMVCCReadpoint, boolean includesTag, Encryption.Context encryptionContext) {
9698
if (compression == null) {

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

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616
package org.apache.hadoop.hbase.regionserver.storefiletracker;
1717

18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.Map;
1821
import org.apache.hadoop.conf.Configuration;
1922
import org.apache.hadoop.hbase.CompoundConfiguration;
2023
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
@@ -33,22 +36,81 @@
3336

3437
/**
3538
* Factory method for creating store file tracker.
39+
* <p/>
40+
* The current implementations are:
41+
* <ul>
42+
* <li><em>default</em>: DefaultStoreFileTracker, see {@link DefaultStoreFileTracker}.</li>
43+
* <li><em>file</em>:FileBasedStoreFileTracker, see {@link FileBasedStoreFileTracker}.</li>
44+
* <li><em>migration</em>:MigrationStoreFileTracker, see {@link MigrationStoreFileTracker}.</li>
45+
* </ul>
46+
* @see DefaultStoreFileTracker
47+
* @see FileBasedStoreFileTracker
48+
* @see MigrationStoreFileTracker
3649
*/
37-
@InterfaceAudience.Private public final class StoreFileTrackerFactory {
38-
public static final String TRACK_IMPL = "hbase.store.file-tracker.impl";
50+
@InterfaceAudience.Private
51+
public final class StoreFileTrackerFactory {
52+
3953
private static final Logger LOG = LoggerFactory.getLogger(StoreFileTrackerFactory.class);
4054

41-
public static Class<? extends StoreFileTracker> getStoreFileTrackerImpl(Configuration conf) {
42-
return conf.getClass(TRACK_IMPL, DefaultStoreFileTracker.class, StoreFileTracker.class);
55+
public static final String TRACKER_IMPL = "hbase.store.file-tracker.impl";
56+
57+
/**
58+
* Maps between configuration names for trackers and implementation classes.
59+
*/
60+
public enum Trackers {
61+
DEFAULT(DefaultStoreFileTracker.class), FILE(FileBasedStoreFileTracker.class),
62+
MIGRATION(MigrationStoreFileTracker.class);
63+
64+
final Class<? extends StoreFileTracker> clazz;
65+
66+
Trackers(Class<? extends StoreFileTracker> clazz) {
67+
this.clazz = clazz;
68+
}
69+
}
70+
71+
private static final Map<Class<? extends StoreFileTracker>, Trackers> CLASS_TO_ENUM = reverse();
72+
73+
private static Map<Class<? extends StoreFileTracker>, Trackers> reverse() {
74+
Map<Class<? extends StoreFileTracker>, Trackers> map = new HashMap<>();
75+
for (Trackers tracker : Trackers.values()) {
76+
map.put(tracker.clazz, tracker);
77+
}
78+
return Collections.unmodifiableMap(map);
79+
}
80+
81+
private StoreFileTrackerFactory() {
82+
}
83+
84+
public static String getStoreFileTrackerName(Configuration conf) {
85+
return conf.get(TRACKER_IMPL, Trackers.DEFAULT.name());
86+
}
87+
88+
static String getStoreFileTrackerName(Class<? extends StoreFileTracker> clazz) {
89+
Trackers name = CLASS_TO_ENUM.get(clazz);
90+
return name != null ? name.name() : clazz.getName();
91+
}
92+
93+
private static Class<? extends StoreFileTracker> getTrackerClass(Configuration conf) {
94+
try {
95+
Trackers tracker = Trackers.valueOf(getStoreFileTrackerName(conf).toUpperCase());
96+
return tracker.clazz;
97+
} catch (IllegalArgumentException e) {
98+
// Fall back to them specifying a class name
99+
return conf.getClass(TRACKER_IMPL, Trackers.DEFAULT.clazz, StoreFileTracker.class);
100+
}
43101
}
44102

45103
public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica,
46104
StoreContext ctx) {
47-
Class<? extends StoreFileTracker> tracker = getStoreFileTrackerImpl(conf);
105+
Class<? extends StoreFileTracker> tracker = getTrackerClass(conf);
48106
LOG.info("instantiating StoreFileTracker impl {}", tracker.getName());
49107
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
50108
}
51109

110+
/**
111+
* Used at master side when splitting/merging regions, as we do not have a Store, thus no
112+
* StoreContext at master side.
113+
*/
52114
public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica, String family,
53115
HRegionFileSystem regionFs) {
54116
ColumnFamilyDescriptorBuilder fDescBuilder =
@@ -64,15 +126,30 @@ public static Configuration mergeConfigurations(Configuration global, TableDescr
64126
.addStringMap(family.getConfiguration()).addBytesMap(family.getValues());
65127
}
66128

67-
static StoreFileTrackerBase create(Configuration conf, String configName,
129+
/**
130+
* Create store file tracker to be used as source or destination for
131+
* {@link MigrationStoreFileTracker}.
132+
*/
133+
static StoreFileTrackerBase createForMigration(Configuration conf, String configName,
68134
boolean isPrimaryReplica, StoreContext ctx) {
69-
String className =
135+
String trackerName =
70136
Preconditions.checkNotNull(conf.get(configName), "config %s is not set", configName);
71137
Class<? extends StoreFileTrackerBase> tracker;
72138
try {
73-
tracker = Class.forName(className).asSubclass(StoreFileTrackerBase.class);
74-
} catch (ClassNotFoundException e) {
75-
throw new RuntimeException(e);
139+
tracker =
140+
Trackers.valueOf(trackerName.toUpperCase()).clazz.asSubclass(StoreFileTrackerBase.class);
141+
} catch (IllegalArgumentException e) {
142+
// Fall back to them specifying a class name
143+
try {
144+
tracker = Class.forName(trackerName).asSubclass(StoreFileTrackerBase.class);
145+
} catch (ClassNotFoundException cnfe) {
146+
throw new RuntimeException(cnfe);
147+
}
148+
}
149+
// prevent nest of MigrationStoreFileTracker, it will cause infinite recursion.
150+
if (MigrationStoreFileTracker.class.isAssignableFrom(tracker)) {
151+
throw new IllegalArgumentException("Should not specify " + configName + " as " +
152+
Trackers.MIGRATION + " because it can not be nested");
76153
}
77154
LOG.info("instantiating StoreFileTracker impl {} as {}", tracker.getName(), configName);
78155
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
package org.apache.hadoop.hbase.client;
1919

2020
import static org.apache.hadoop.hbase.HBaseTestingUtil.countRows;
21-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
21+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2222
import static org.junit.Assert.assertEquals;
2323
import static org.junit.Assert.assertFalse;
2424
import static org.junit.Assert.assertNotEquals;
@@ -426,8 +426,8 @@ private void testCloneTableSchema(final TableName tableName, final TableName new
426426
assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled());
427427
assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive());
428428
// HBASE-26246 introduced persist of store file tracker into table descriptor
429-
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL,
430-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
429+
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL,
430+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
431431
build();
432432
TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc);
433433

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.client;
1919

20-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
20+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2121
import static org.junit.Assert.assertEquals;
2222
import static org.junit.Assert.assertFalse;
2323
import static org.junit.Assert.assertTrue;
@@ -239,8 +239,8 @@ public void testGetTableDescriptor() throws IOException {
239239
Table table = TEST_UTIL.getConnection().getTable(htd.getTableName());
240240
TableDescriptor confirmedHtd = table.getDescriptor();
241241
//HBASE-26246 introduced persist of store file tracker into table descriptor
242-
htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACK_IMPL,
243-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
242+
htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACKER_IMPL,
243+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
244244
build();
245245
assertEquals(0, TableDescriptor.COMPARATOR.compare(htd, confirmedHtd));
246246
MetaTableAccessor.fullScanMetaAndPrint(TEST_UTIL.getConnection());

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
package org.apache.hadoop.hbase.client;
1919

2020
import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME;
21-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
21+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2222
import static org.junit.Assert.assertEquals;
2323
import static org.junit.Assert.assertFalse;
2424
import static org.junit.Assert.assertTrue;
@@ -375,8 +375,8 @@ private void testCloneTableSchema(final TableName tableName,
375375
assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled());
376376
assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive());
377377
//HBASE-26246 introduced persist of store file tracker into table descriptor
378-
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL,
379-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
378+
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL,
379+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
380380
build();
381381
TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc);
382382

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
package org.apache.hadoop.hbase.client;
1919

2020
import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME;
21-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
21+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2222
import static org.hamcrest.CoreMatchers.instanceOf;
2323
import static org.hamcrest.MatcherAssert.assertThat;
2424
import static org.junit.Assert.assertEquals;
@@ -150,8 +150,8 @@ public void testGetTableDescriptor() throws Exception {
150150
admin.createTable(desc).join();
151151
TableDescriptor confirmedHtd = admin.getDescriptor(tableName).get();
152152
//HBASE-26246 introduced persist of store file tracker into table descriptor
153-
desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACK_IMPL,
154-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
153+
desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACKER_IMPL,
154+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
155155
build();
156156
assertEquals(0, TableDescriptor.COMPARATOR.compare(desc, confirmedHtd));
157157
}

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
package org.apache.hadoop.hbase.master.procedure;
2020

21-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
21+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2222
import static org.junit.Assert.assertEquals;
2323
import static org.junit.Assert.assertFalse;
2424
import static org.junit.Assert.assertTrue;
@@ -231,8 +231,8 @@ public static void validateTableCreation(final HMaster master, final TableName t
231231

232232
// checks store file tracker impl has been properly set in htd
233233
String storeFileTrackerImpl =
234-
StoreFileTrackerFactory.getStoreFileTrackerImpl(master.getConfiguration()).getName();
235-
assertEquals(storeFileTrackerImpl, htd.getValue(TRACK_IMPL));
234+
StoreFileTrackerFactory.getStoreFileTrackerName(master.getConfiguration());
235+
assertEquals(storeFileTrackerImpl, htd.getValue(TRACKER_IMPL));
236236
}
237237

238238
public static void validateTableDeletion(

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
*/
1818
package org.apache.hadoop.hbase.master.procedure;
1919

20-
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACK_IMPL;
20+
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL;
2121
import static org.junit.Assert.assertEquals;
2222
import static org.junit.Assert.assertTrue;
2323
import static org.junit.Assert.fail;
@@ -96,13 +96,13 @@ public void testCreateWithTrackImpl() throws Exception {
9696
ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
9797
TableDescriptor htd = MasterProcedureTestingUtility.createHTD(tableName, F1);
9898
String trackerName = TestStoreFileTracker.class.getName();
99-
htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACK_IMPL, trackerName).build();
99+
htd = TableDescriptorBuilder.newBuilder(htd).setValue(TRACKER_IMPL, trackerName).build();
100100
RegionInfo[] regions = ModifyRegionUtils.createRegionInfos(htd, null);
101101
long procId = ProcedureTestingUtility.submitAndWait(procExec,
102102
new CreateTableProcedure(procExec.getEnvironment(), htd, regions));
103103
ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId));
104104
htd = getMaster().getTableDescriptors().get(tableName);
105-
assertEquals(trackerName, htd.getValue(TRACK_IMPL));
105+
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
106106
}
107107

108108
@Test

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
package org.apache.hadoop.hbase.regionserver;
1919

2020
import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.
21-
TRACK_IMPL;
21+
TRACKER_IMPL;
2222
import static org.junit.Assert.assertTrue;
2323
import static org.junit.Assert.fail;
2424

@@ -74,7 +74,7 @@ public class TestMergesSplitsAddToTracker {
7474

7575
@BeforeClass
7676
public static void setupClass() throws Exception {
77-
TEST_UTIL.getConfiguration().set(TRACK_IMPL, TestStoreFileTracker.class.getName());
77+
TEST_UTIL.getConfiguration().set(TRACKER_IMPL, TestStoreFileTracker.class.getName());
7878
TEST_UTIL.startMiniCluster();
7979
}
8080

0 commit comments

Comments
 (0)