HBASE-26586 Should not rely on the global config when setting SFT implementation for a table while upgrading (#4006)

Signed-off-by: GeorryHuang <huangzhuoyue@apache.org>
Signed-off-by: Josh Elser <elserj@apache.org>
Signed-off-by: Wellington Ramos Chevreuil <wchevreuil@apache.org>

Author: Apache9

hbase-server/src/main/java/org/apache/hadoop/hbase/master/migrate/RollingUpgradeChore.java

@@ -34,7 +34,7 @@
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
 import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
-import org.apache.hadoop.hbase.regionserver.storefiletracker.MigrateStoreFileTrackerProcedure;
+import org.apache.hadoop.hbase.regionserver.storefiletracker.InitializeStoreFileTrackerProcedure;
 import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
@@ -60,7 +60,7 @@ public class RollingUpgradeChore extends ScheduledChore {
   private final static Logger LOG = LoggerFactory.getLogger(RollingUpgradeChore.class);
   ProcedureExecutor<MasterProcedureEnv> procedureExecutor;
   private TableDescriptors tableDescriptors;
-  private List<MigrateStoreFileTrackerProcedure> processingProcs = new ArrayList<>();
+  private List<InitializeStoreFileTrackerProcedure> processingProcs = new ArrayList<>();
 
   public RollingUpgradeChore(MasterServices masterServices) {
     this(masterServices.getConfiguration(), masterServices.getMasterProcedureExecutor(),
@@ -89,9 +89,9 @@ protected void chore() {
   }
 
   private boolean isCompletelyMigrateSFT(int concurrentCount){
-    Iterator<MigrateStoreFileTrackerProcedure> iter = processingProcs.iterator();
+    Iterator<InitializeStoreFileTrackerProcedure> iter = processingProcs.iterator();
     while(iter.hasNext()){
-      MigrateStoreFileTrackerProcedure proc = iter.next();
+      InitializeStoreFileTrackerProcedure proc = iter.next();
       if(procedureExecutor.isFinished(proc.getProcId())){
         iter.remove();
       }
@@ -120,8 +120,8 @@ private boolean isCompletelyMigrateSFT(int concurrentCount){
 
     for (Map.Entry<String, TableDescriptor> entry : migrateSFTTables.entrySet()) {
       TableDescriptor tableDescriptor = entry.getValue();
-      MigrateStoreFileTrackerProcedure proc =
-        new MigrateStoreFileTrackerProcedure(procedureExecutor.getEnvironment(), tableDescriptor);
+      InitializeStoreFileTrackerProcedure proc = new InitializeStoreFileTrackerProcedure(
+        procedureExecutor.getEnvironment(), tableDescriptor);
       procedureExecutor.submitProcedure(proc);
       processingProcs.add(proc);
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrateStoreFileTrackerProcedure.java renamed to hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/InitializeStoreFileTrackerProcedure.java

@@ -19,28 +19,32 @@
 
 import java.util.Optional;
 import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
 import org.apache.hadoop.hbase.master.procedure.ModifyTableDescriptorProcedure;
 import org.apache.hadoop.hbase.procedure2.util.StringUtils;
 import org.apache.yetus.audience.InterfaceAudience;
 
 /**
- * Procedure for migrating StoreFileTracker information to table descriptor.
+ * Procedure for setting StoreFileTracker information to table descriptor.
  */
 @InterfaceAudience.Private
-public class MigrateStoreFileTrackerProcedure extends ModifyTableDescriptorProcedure {
+public class InitializeStoreFileTrackerProcedure extends ModifyTableDescriptorProcedure {
 
-  public MigrateStoreFileTrackerProcedure(){}
+  public InitializeStoreFileTrackerProcedure(){}
 
-  public MigrateStoreFileTrackerProcedure(MasterProcedureEnv env, TableDescriptor unmodified) {
+  public InitializeStoreFileTrackerProcedure(MasterProcedureEnv env, TableDescriptor unmodified) {
     super(env, unmodified);
   }
 
   @Override
   protected Optional<TableDescriptor> modify(MasterProcedureEnv env, TableDescriptor current) {
     if (StringUtils.isEmpty(current.getValue(StoreFileTrackerFactory.TRACKER_IMPL))) {
+      // no tracker impl means it is a table created in previous version, the tracker impl can only
+      // be default.
       TableDescriptor td =
-        StoreFileTrackerFactory.updateWithTrackerConfigs(env.getMasterConfiguration(), current);
+        TableDescriptorBuilder.newBuilder(current).setValue(StoreFileTrackerFactory.TRACKER_IMPL,
+          StoreFileTrackerFactory.Trackers.DEFAULT.name()).build();
       return Optional.of(td);
     }
     return Optional.empty();

hbase-server/src/test/java/org/apache/hadoop/hbase/master/migrate/TestMigrateStoreFileTracker.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/master/migrate/TestInitializeStoreFileTracker.java

@@ -18,6 +18,8 @@
  */
 package org.apache.hadoop.hbase.master.migrate;
 
+import static org.junit.Assert.assertEquals;
+
 import java.io.IOException;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
@@ -31,6 +33,7 @@
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.After;
@@ -40,11 +43,11 @@
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-@Category(MediumTests.class)
-public class TestMigrateStoreFileTracker {
+@Category({ MediumTests.class, MasterTests.class })
+public class TestInitializeStoreFileTracker {
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
-    HBaseClassTestRule.forClass(TestMigrateStoreFileTracker.class);
+    HBaseClassTestRule.forClass(TestInitializeStoreFileTracker.class);
   private final static String[] tables = new String[] { "t1", "t2", "t3", "t4", "t5", "t6" };
   private final static String famStr = "f1";
   private final static byte[] fam = Bytes.toBytes(famStr);
@@ -56,9 +59,12 @@ public class TestMigrateStoreFileTracker {
   @Before
   public void setUp() throws Exception {
     conf = HBaseConfiguration.create();
-    //Speed up the launch of RollingUpgradeChore
+    // Speed up the launch of RollingUpgradeChore
     conf.setInt(RollingUpgradeChore.ROLLING_UPGRADE_CHORE_PERIOD_SECONDS_KEY, 1);
     conf.setLong(RollingUpgradeChore.ROLLING_UPGRADE_CHORE_DELAY_SECONDS_KEY, 1);
+    // Set the default implementation to file instead of default, to confirm we will not set SFT to
+    // file
+    conf.set(StoreFileTrackerFactory.TRACKER_IMPL, StoreFileTrackerFactory.Trackers.FILE.name());
     HTU = new HBaseTestingUtility(conf);
     HTU.startMiniCluster();
   }
@@ -89,7 +95,7 @@ public void testMigrateStoreFileTracker() throws IOException, InterruptedExcepti
     HTU.getMiniHBaseCluster().stopMaster(0).join();
     HTU.getMiniHBaseCluster().startMaster();
     HTU.getMiniHBaseCluster().waitForActiveAndReadyMaster(30000);
-    //wait until all tables have been migrated
+    // wait until all tables have been migrated
     TableDescriptors tds = HTU.getMiniHBaseCluster().getMaster().getTableDescriptors();
     HTU.waitFor(30000, () -> {
       try {
@@ -104,5 +110,10 @@ public void testMigrateStoreFileTracker() throws IOException, InterruptedExcepti
         return false;
       }
     });
+    for (String table : tables) {
+      TableDescriptor td = tds.get(TableName.valueOf(table));
+      assertEquals(StoreFileTrackerFactory.Trackers.DEFAULT.name(),
+        td.getValue(StoreFileTrackerFactory.TRACKER_IMPL));
+    }
   }
 }