Skip to content

Commit 864fb9a

Browse files
Apache9apurtell
Apache9
authored and
apurtell
committed
HBASE-26248 Should find a suitable way to let users specify the store file tracker implementation (#3665)
Signed-off-by: Wellington Chevreuil <[email protected]> Conflicts: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestRegionWithFileBasedStoreFileTracker.java
1 parent b80efea commit 864fb9a

File tree

14 files changed

+204
-57
lines changed

14 files changed

+204
-57
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.client.ColumnFamilyDescriptor;
2023
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
@@ -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 =
@@ -63,15 +125,30 @@ public static Configuration mergeConfigurations(Configuration global, TableDescr
63125
return StoreUtils.createStoreConfiguration(global, table, family);
64126
}
65127

66-
static StoreFileTrackerBase create(Configuration conf, String configName,
128+
/**
129+
* Create store file tracker to be used as source or destination for
130+
* {@link MigrationStoreFileTracker}.
131+
*/
132+
static StoreFileTrackerBase createForMigration(Configuration conf, String configName,
67133
boolean isPrimaryReplica, StoreContext ctx) {
68-
String className =
134+
String trackerName =
69135
Preconditions.checkNotNull(conf.get(configName), "config %s is not set", configName);
70136
Class<? extends StoreFileTrackerBase> tracker;
71137
try {
72-
tracker = Class.forName(className).asSubclass(StoreFileTrackerBase.class);
73-
} catch (ClassNotFoundException e) {
74-
throw new RuntimeException(e);
138+
tracker =
139+
Trackers.valueOf(trackerName.toUpperCase()).clazz.asSubclass(StoreFileTrackerBase.class);
140+
} catch (IllegalArgumentException e) {
141+
// Fall back to them specifying a class name
142+
try {
143+
tracker = Class.forName(trackerName).asSubclass(StoreFileTrackerBase.class);
144+
} catch (ClassNotFoundException cnfe) {
145+
throw new RuntimeException(cnfe);
146+
}
147+
}
148+
// prevent nest of MigrationStoreFileTracker, it will cause infinite recursion.
149+
if (MigrationStoreFileTracker.class.isAssignableFrom(tracker)) {
150+
throw new IllegalArgumentException("Should not specify " + configName + " as " +
151+
Trackers.MIGRATION + " because it can not be nested");
75152
}
76153
LOG.info("instantiating StoreFileTracker impl {} as {}", tracker.getName(), configName);
77154
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
@@ -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.assertNotEquals;
@@ -423,8 +423,8 @@ private void testCloneTableSchema(final TableName tableName, final TableName new
423423
assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled());
424424
assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive());
425425
// HBASE-26246 introduced persist of store file tracker into table descriptor
426-
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL,
427-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
426+
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL,
427+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
428428
build();
429429
TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc);
430430

‎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;
@@ -413,8 +413,8 @@ private void testCloneTableSchema(final TableName tableName,
413413
assertEquals(BLOCK_CACHE, newTableDesc.getColumnFamily(FAMILY_1).isBlockCacheEnabled());
414414
assertEquals(TTL, newTableDesc.getColumnFamily(FAMILY_1).getTimeToLive());
415415
//HBASE-26246 introduced persist of store file tracker into table descriptor
416-
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACK_IMPL,
417-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
416+
tableDesc = TableDescriptorBuilder.newBuilder(tableDesc).setValue(TRACKER_IMPL,
417+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
418418
build();
419419
TEST_UTIL.verifyTableDescriptorIgnoreTableName(tableDesc, newTableDesc);
420420

‎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;
@@ -152,8 +152,8 @@ public void testGetTableDescriptor() throws Exception {
152152
ModifyableTableDescriptor modifyableDesc = ((ModifyableTableDescriptor) desc);
153153
TableDescriptor confirmedHtd = admin.getDescriptor(tableName).get();
154154
//HBASE-26246 introduced persist of store file tracker into table descriptor
155-
desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACK_IMPL,
156-
StoreFileTrackerFactory.getStoreFileTrackerImpl(TEST_UTIL.getConfiguration()).getName()).
155+
desc = TableDescriptorBuilder.newBuilder(desc).setValue(TRACKER_IMPL,
156+
StoreFileTrackerFactory.getStoreFileTrackerName(TEST_UTIL.getConfiguration())).
157157
build();
158158
assertEquals(0, TableDescriptor.COMPARATOR.compare(desc, confirmedHtd));
159159
}

‎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;
@@ -229,8 +229,8 @@ public static void validateTableCreation(final HMaster master, final TableName t
229229

230230
// checks store file tracker impl has been properly set in htd
231231
String storeFileTrackerImpl =
232-
StoreFileTrackerFactory.getStoreFileTrackerImpl(master.getConfiguration()).getName();
233-
assertEquals(storeFileTrackerImpl, htd.getValue(TRACK_IMPL));
232+
StoreFileTrackerFactory.getStoreFileTrackerName(master.getConfiguration());
233+
assertEquals(storeFileTrackerImpl, htd.getValue(TRACKER_IMPL));
234234
}
235235

236236
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

0 commit comments

Comments
 (0)