HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker… (#3721)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Josh Elser <elserj@apache.org>
This commit is contained in:
Wellington Ramos Chevreuil 2021-10-13 15:48:13 +01:00 committed by Andrew Purtell
parent 4ae866f3bc
commit 4b493b226b
8 changed files with 49 additions and 30 deletions

View File

@ -32,7 +32,6 @@ import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.client.TableState;
import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
import org.apache.hadoop.hbase.master.MasterFileSystem;
@ -270,9 +269,8 @@ public class CreateTableProcedure
(newRegions != null ? newRegions.size() : 0));
}
TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableDescriptor);
StoreFileTrackerFactory.persistTrackerConfig(env.getMasterConfiguration(), builder);
tableDescriptor = builder.build();
tableDescriptor = StoreFileTrackerFactory.updateWithTrackerConfigs(env.getMasterConfiguration(),
tableDescriptor);
final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost();
if (cpHost != null) {

View File

@ -56,7 +56,14 @@ class FileBasedStoreFileTracker extends StoreFileTrackerBase {
public FileBasedStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) {
super(conf, isPrimaryReplica, ctx);
backedFile = new StoreFileListFile(ctx);
//CreateTableProcedure needs to instantiate the configured SFT impl, in order to update table
//descriptors with the SFT impl specific configs. By the time this happens, the table has no
//regions nor stores yet, so it can't create a proper StoreContext.
if (ctx != null) {
backedFile = new StoreFileListFile(ctx);
} else {
backedFile = null;
}
}
@Override

View File

@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.Collection;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.procedure2.util.StringUtils;
import org.apache.hadoop.hbase.regionserver.StoreContext;
@ -88,17 +89,6 @@ class MigrationStoreFileTracker extends StoreFileTrackerBase {
"Should not call this method on " + getClass().getSimpleName());
}
@Override
public void persistConfiguration(TableDescriptorBuilder builder) {
super.persistConfiguration(builder);
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());
}
}
static Class<? extends StoreFileTracker> getSrcTrackerClass(Configuration conf) {
return StoreFileTrackerFactory.getStoreFileTrackerClassForMigration(conf, SRC_IMPL);
}

View File

@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.Collection;
import java.util.List;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.regionserver.CreateStoreFileWriterParams;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
@ -75,7 +76,7 @@ public interface StoreFileTracker {
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
* Saves StoreFileTracker implementations specific configurations into the table descriptors.
* Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* 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.
@ -83,5 +84,5 @@ public interface StoreFileTracker {
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
void persistConfiguration(TableDescriptorBuilder builder);
TableDescriptorBuilder updateWithTrackerConfigs(TableDescriptorBuilder builder);
}

View File

@ -25,6 +25,7 @@ import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.io.crypto.Encryption;
@ -32,7 +33,6 @@ import org.apache.hadoop.hbase.io.hfile.CacheConfig;
import org.apache.hadoop.hbase.io.hfile.HFile;
import org.apache.hadoop.hbase.io.hfile.HFileContext;
import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
import org.apache.hadoop.hbase.procedure2.util.StringUtils;
import org.apache.hadoop.hbase.regionserver.CreateStoreFileWriterParams;
import org.apache.hadoop.hbase.regionserver.StoreContext;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
@ -83,10 +83,9 @@ abstract class StoreFileTrackerBase implements StoreFileTracker {
}
@Override
public void persistConfiguration(TableDescriptorBuilder builder) {
if (StringUtils.isEmpty(builder.getValue(TRACKER_IMPL))) {
builder.setValue(TRACKER_IMPL, getTrackerName());
}
public TableDescriptorBuilder updateWithTrackerConfigs(TableDescriptorBuilder builder) {
builder.setValue(TRACKER_IMPL, getTrackerName());
return builder;
}
protected final String getTrackerName() {

View File

@ -24,8 +24,10 @@ import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.procedure2.util.StringUtils;
import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
import org.apache.hadoop.hbase.regionserver.StoreContext;
import org.apache.hadoop.hbase.regionserver.StoreUtils;
import org.apache.hadoop.hbase.util.ReflectionUtils;
import org.apache.yetus.audience.InterfaceAudience;
@ -158,12 +160,18 @@ public final class StoreFileTrackerFactory {
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
tracker.persistConfiguration(builder);
public static TableDescriptor updateWithTrackerConfigs(Configuration conf,
TableDescriptor descriptor) {
//CreateTableProcedure needs to instantiate the configured SFT impl, in order to update table
//descriptors with the SFT impl specific configs. By the time this happens, the table has no
//regions nor stores yet, so it can't create a proper StoreContext.
if (StringUtils.isEmpty(descriptor.getValue(TRACKER_IMPL))) {
StoreFileTracker tracker =
StoreFileTrackerFactory.create(conf, true, null);
TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(descriptor);
return tracker.updateWithTrackerConfigs(builder).build();
}
return descriptor;
}
// should not use MigrationStoreFileTracker for new family

View File

@ -39,6 +39,7 @@ import org.apache.hadoop.hbase.master.MasterFileSystem;
import org.apache.hadoop.hbase.procedure2.Procedure;
import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
import org.apache.hadoop.hbase.regionserver.storefiletracker.TestStoreFileTracker;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
@ -105,6 +106,21 @@ public class TestCreateTableProcedure extends TestTableDDLProcedureBase {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
@Test
public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
StoreFileTrackerFactory.Trackers.FILE.name());
final TableName tableName = TableName.valueOf(name.getMethodName());
TableDescriptor htd = MasterProcedureTestingUtility.createHTD(tableName, F1);
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(StoreFileTrackerFactory.Trackers.FILE.name(), htd.getValue(TRACKER_IMPL));
}
@Test
public void testCreateWithoutColumnFamily() throws Exception {
final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();

View File

@ -37,7 +37,7 @@ public class TestStoreFileTracker extends DefaultStoreFileTracker {
public TestStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) {
super(conf, isPrimaryReplica, ctx);
if (ctx.getRegionFileSystem() != null) {
if (ctx != null && ctx.getRegionFileSystem() != null) {
this.storeId = ctx.getRegionInfo().getEncodedName() + "-" + ctx.getFamily().getNameAsString();
LOG.info("created storeId: {}", storeId);
trackedFiles.computeIfAbsent(storeId, v -> new ArrayList<>());