HBASE-24833: Bootstrap should not delete the META table directory if it's not partial

Add a check on meta bootstrap to skip removing meta table directory
if ZK data does not exist when hmaster restart. here the existence
of clusterID in ZK indicate if the meta is partial if we hit the
INIT_META_WRITE_FS_LAYOUT in InitMetaProcedure

Author: taklwu

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

@@ -463,6 +463,8 @@ public void run() {
   private final boolean maintenanceMode;
   static final String MAINTENANCE_MODE = "hbase.master.maintenance_mode";
 
+  private boolean isClusterRestartWithExistingZNodes;
+
   // Cached clusterId on stand by masters to serve clusterID requests from clients.
   private final CachedClusterId cachedClusterId;
 
@@ -915,6 +917,11 @@ private void finishActiveMasterInitialization(MonitoredTask status)
       this.tableDescriptors.getAll();
     }
 
+    // check cluster Id stored in ZNode before, and use it to indicate if a cluster has been
+    // restarted with an existing Zookeeper quorum.
+    isClusterRestartWithExistingZNodes =
+      ZKClusterId.readClusterIdZNode(this.zooKeeper) != null ? true : false;
+
     // Publish cluster ID; set it in Master too. The superclass RegionServer does this later but
     // only after it has checked in with the Master. At least a few tests ask Master for clusterId
     // before it has called its run method and before RegionServer has done the reportForDuty.
@@ -3894,4 +3901,8 @@ public String getClusterId() {
   public MetaRegionLocationCache getMetaRegionLocationCache() {
     return this.metaRegionLocationCache;
   }
+
+  public boolean isClusterRestartWithExistingZNodes() {
+    return isClusterRestartWithExistingZNodes;
+  }
 }

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

@@ -524,4 +524,10 @@ default SplitWALManager getSplitWALManager(){
    * Run the ReplicationBarrierChore.
    */
   void runReplicationBarrierCleaner();
+
+  /**
+   * @return true as running on existing ZNodes when the master restarts, otherwise false.
+   */
+  boolean isClusterRestartWithExistingZNodes();
+
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/InitMetaProcedure.java

@@ -67,11 +67,14 @@ public TableOperationType getTableOperationType() {
     return TableOperationType.CREATE;
   }
 
-  private static void writeFsLayout(Path rootDir, Configuration conf) throws IOException {
+  private static void writeFsLayout(boolean isClusterRestartWithExistingZNodes,
+    Path rootDir, Configuration conf) throws IOException {
     LOG.info("BOOTSTRAP: creating hbase:meta region");
     FileSystem fs = rootDir.getFileSystem(conf);
     Path tableDir = CommonFSUtils.getTableDir(rootDir, TableName.META_TABLE_NAME);
-    if (fs.exists(tableDir) && !fs.delete(tableDir, true)) {
+    // when entering the state of INIT_META_WRITE_FS_LAYOUT, we use existing zookeeper data to
+    // tell if this is a partial created meta, if so we should delete and recreate the meta table.
+    if (isClusterRestartWithExistingZNodes && fs.exists(tableDir) && !fs.delete(tableDir, true)) {
       LOG.warn("Can not delete partial created meta table, continue...");
     }
     // Bootstrapping, make sure blockcache is off. Else, one will be
@@ -96,7 +99,8 @@ protected Flow executeFromState(MasterProcedureEnv env, InitMetaState state)
         case INIT_META_WRITE_FS_LAYOUT:
           Configuration conf = env.getMasterConfiguration();
           Path rootDir = CommonFSUtils.getRootDir(conf);
-          writeFsLayout(rootDir, conf);
+          writeFsLayout(env.getMasterServices().isClusterRestartWithExistingZNodes(), rootDir,
+            conf);
           setNextState(InitMetaState.INIT_META_ASSIGN_META);
           return Flow.HAS_MORE_STATE;
         case INIT_META_ASSIGN_META:

hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java

@@ -482,4 +482,9 @@ public List<RegionPlan> executeRegionPlansWithThrottling(List<RegionPlan> plans)
 
   @Override
   public void runReplicationBarrierCleaner() {}
+
+  @Override
+  public boolean isClusterRestartWithExistingZNodes() {
+    return false;
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRecreateCluster.java

@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.time.Duration;
+import java.util.List;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.MiniHBaseCluster;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
+import org.apache.hadoop.hbase.procedure2.store.wal.WALProcedureStore;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.CommonFSUtils;
+import org.apache.hadoop.hbase.zookeeper.ZKUtil;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+/**
+ * Test reuse storefiles within data directory when cluster failover with a set of new region
+ * servers with different hostnames with or without WALs and Zookeeper ZNodes support. For any
+ * hbase system table and user table can be assigned normally after cluster restart.
+ */
+@Category({ LargeTests.class })
+public class TestRecreateCluster {
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestRecreateCluster.class);
+
+  @Rule
+  public TestName name = new TestName();
+
+  private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static final int NUM_RS = 3;
+  private static final long TIMEOUT_MS = Duration.ofMinutes(2).toMillis();
+
+  @Test
+  public void testRecreateCluster_UserTableDisabled_WithoutCleanupWALsAndZNodes()
+    throws Exception {
+    validateRecreateClusterWithUserDisabled(false, false);
+  }
+
+  @Test
+  public void testRecreateCluster_UserTableEnabled_WithoutCleanupWALsAndZNodes() throws Exception {
+    validateRecreateClusterWithUserTableEnabled(false, false);
+  }
+
+  private void validateRecreateClusterWithUserDisabled(boolean cleanupWALs,
+    boolean cleanUpZNodes) throws Exception {
+    TEST_UTIL.startMiniCluster(NUM_RS);
+    try {
+      TableName tableName = TableName.valueOf("t1");
+      prepareDataBeforeRecreate(TEST_UTIL, tableName);
+      TEST_UTIL.getAdmin().disableTable(tableName);
+      TEST_UTIL.waitTableDisabled(tableName.getName());
+      restartHBaseCluster(cleanupWALs, cleanUpZNodes);
+      TEST_UTIL.getAdmin().enableTable(tableName);
+      validateDataAfterRecreate(TEST_UTIL, tableName);
+    } finally {
+      TEST_UTIL.shutdownMiniCluster();
+    }
+  }
+
+  private void validateRecreateClusterWithUserTableEnabled(boolean cleanupWALs,
+    boolean cleanUpZNodes) throws Exception {
+    TEST_UTIL.startMiniCluster(NUM_RS);
+    try {
+      TableName tableName = TableName.valueOf("t1");
+      prepareDataBeforeRecreate(TEST_UTIL, tableName);
+      restartHBaseCluster(cleanupWALs, cleanUpZNodes);
+      validateDataAfterRecreate(TEST_UTIL, tableName);
+    } finally {
+      TEST_UTIL.shutdownMiniCluster();
+    }
+  }
+
+  private void restartHBaseCluster(boolean cleanUpWALs, boolean cleanUpZnodes) throws Exception {
+    // flush cache so that everything is on disk
+    TEST_UTIL.getMiniHBaseCluster().flushcache();
+
+    List<ServerName> oldServers =
+      TEST_UTIL.getHBaseCluster().getMaster().getServerManager().getOnlineServersList();
+
+    // make sure there is no procedures pending
+    TEST_UTIL.waitFor(TIMEOUT_MS, () -> TEST_UTIL.getHBaseCluster().getMaster()
+      .getProcedures().stream().filter(p -> p.isFinished()).findAny().isPresent());
+
+    // shutdown and delete data if needed
+    Path walRootDirPath = TEST_UTIL.getMiniHBaseCluster().getMaster().getWALRootDir();
+    Path rootDirPath = CommonFSUtils.getRootDir(TEST_UTIL.getConfiguration());
+    TEST_UTIL.shutdownMiniHBaseCluster();
+
+    if (cleanUpWALs) {
+      TEST_UTIL.getDFSCluster().getFileSystem()
+        .delete(new Path(rootDirPath, MasterRegionFactory.MASTER_STORE_DIR), true);
+      TEST_UTIL.getDFSCluster().getFileSystem()
+        .delete(new Path(walRootDirPath, MasterRegionFactory.MASTER_STORE_DIR), true);
+      TEST_UTIL.getDFSCluster().getFileSystem()
+        .delete(new Path(walRootDirPath, WALProcedureStore.MASTER_PROCEDURE_LOGDIR), true);
+
+      TEST_UTIL.getDFSCluster().getFileSystem()
+        .delete(new Path(walRootDirPath, HConstants.HREGION_LOGDIR_NAME), true);
+      TEST_UTIL.getDFSCluster().getFileSystem()
+        .delete(new Path(walRootDirPath, HConstants.HREGION_OLDLOGDIR_NAME), true);
+    }
+
+    if (cleanUpZnodes) {
+      // delete all zk data
+      // we cannot keep ZK data because it will hold the meta region states as open and
+      // didn't submit a InitMetaProcedure
+      ZKUtil.deleteChildrenRecursively(TEST_UTIL.getZooKeeperWatcher(),
+        TEST_UTIL.getZooKeeperWatcher().getZNodePaths().baseZNode);
+      TEST_UTIL.shutdownMiniZKCluster();
+      TEST_UTIL.startMiniZKCluster();
+    }
+
+    TEST_UTIL.restartHBaseCluster(NUM_RS);
+    TEST_UTIL.waitFor(TIMEOUT_MS,
+      () -> TEST_UTIL.getMiniHBaseCluster().getNumLiveRegionServers() == NUM_RS);
+
+    // make sure we have a new set of region servers with different hostnames and ports
+    List<ServerName> newServers =
+      TEST_UTIL.getHBaseCluster().getMaster().getServerManager().getOnlineServersList();
+    assertFalse(newServers.stream().filter(newServer -> oldServers.contains(newServer)).findAny()
+      .isPresent());
+  }
+
+  private void prepareDataBeforeRecreate(
+      HBaseTestingUtility testUtil, TableName tableName) throws Exception {
+    Table table = testUtil.createTable(tableName, "f");
+    Put put = new Put(Bytes.toBytes("r1"));
+    put.addColumn(Bytes.toBytes("f"), Bytes.toBytes("c"), Bytes.toBytes("v"));
+    table.put(put);
+
+    ensureTableNotColocatedWithSystemTable(tableName, TableName.NAMESPACE_TABLE_NAME);
+  }
+
+  private void ensureTableNotColocatedWithSystemTable(TableName userTable, TableName systemTable)
+      throws IOException, InterruptedException {
+    MiniHBaseCluster hbaseCluster = TEST_UTIL.getHBaseCluster();
+    assertTrue("Please start more than 1 regionserver",
+        hbaseCluster.getRegionServerThreads().size() > 1);
+
+    int userTableServerNum = getServerNumForTableWithOnlyOneRegion(userTable);
+    int systemTableServerNum = getServerNumForTableWithOnlyOneRegion(systemTable);
+
+    if (userTableServerNum != systemTableServerNum) {
+      // no-ops if user table and system are already on a different host
+      return;
+    }
+
+    int destServerNum = (systemTableServerNum + 1) % NUM_RS;
+    assertTrue(systemTableServerNum != destServerNum);
+
+    HRegionServer systemTableServer = hbaseCluster.getRegionServer(systemTableServerNum);
+    HRegionServer destServer = hbaseCluster.getRegionServer(destServerNum);
+    assertTrue(!systemTableServer.equals(destServer));
+    // make sure the dest server is live before moving region
+    hbaseCluster.waitForRegionServerToStart(destServer.getServerName().getHostname(),
+        destServer.getServerName().getPort(), TIMEOUT_MS);
+    // move region of userTable to a different regionserver not co-located with system table
+    TEST_UTIL.moveRegionAndWait(TEST_UTIL.getAdmin().getRegions(userTable).get(0),
+        destServer.getServerName());
+  }
+
+  private int getServerNumForTableWithOnlyOneRegion(TableName tableName) throws IOException {
+    List<RegionInfo> tableRegionInfos = TEST_UTIL.getAdmin().getRegions(tableName);
+    assertEquals(1, tableRegionInfos.size());
+    return TEST_UTIL.getHBaseCluster()
+        .getServerWith(tableRegionInfos.get(0).getRegionName());
+  }
+
+  private void validateDataAfterRecreate(
+      HBaseTestingUtility testUtil, TableName tableName) throws Exception {
+    Table t1 = testUtil.getConnection().getTable(tableName);
+    Get get = new Get(Bytes.toBytes("r1"));
+    get.addColumn(Bytes.toBytes("f"), Bytes.toBytes("c"));
+    Result result = t1.get(get);
+    assertTrue(result.advance());
+    Cell cell = result.current();
+    assertEquals("v", Bytes.toString(cell.getValueArray(),
+        cell.getValueOffset(), cell.getValueLength()));
+    assertFalse(result.advance());
+  }
+
+}