You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2021/11/09 16:40:07 UTC
[hbase] 09/13: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker… (#3721)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch HBASE-26067
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 25909e8f2d61af50c887869600cbb287d96074c9
Author: Wellington Ramos Chevreuil <wc...@apache.org>
AuthorDate: Wed Oct 13 15:48:13 2021 +0100
HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker… (#3721)
Signed-off-by: Duo Zhang <zh...@apache.org>
Signed-off-by: Josh Elser <el...@apache.org>
---
.../hbase/master/procedure/CreateTableProcedure.java | 6 ++----
.../storefiletracker/FileBasedStoreFileTracker.java | 9 ++++++++-
.../storefiletracker/MigrationStoreFileTracker.java | 12 +-----------
.../storefiletracker/StoreFileTracker.java | 5 +++--
.../storefiletracker/StoreFileTrackerBase.java | 9 ++++-----
.../storefiletracker/StoreFileTrackerFactory.java | 20 ++++++++++++++------
.../master/procedure/TestCreateTableProcedure.java | 16 ++++++++++++++++
.../storefiletracker/TestStoreFileTracker.java | 2 +-
8 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
index ee8e51f..0a6a469 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
@@ -34,7 +34,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;
@@ -290,9 +289,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) {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
index c370b87..4da7911 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
@@ -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
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 1946d4b..230c1ec 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
@@ -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);
}
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 59fe7ef..fd8f7c9 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
@@ -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);
}
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 a786add..edbaace 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
@@ -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() {
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 b586027..1c683ae 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
@@ -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
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 f432c80..51ea9f5 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
@@ -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;
@@ -106,6 +107,21 @@ public class TestCreateTableProcedure extends TestTableDDLProcedureBase {
}
@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();
final TableName tableName = TableName.valueOf(name.getMethodName());
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java
index 1dc9c4e..b30ca47 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java
@@ -40,7 +40,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<>());