HBASE-24390 Remove RegionInfoBuilder.FIRST_META_REGIONINFO (#1877)

Signed-off-by: stack <[email protected]>
apache · Jul 29, 2021 · 2f250f3 · 2f250f3
1 parent 459d5ca
commit 2f250f3
Show file tree

Hide file tree

Showing 67 changed files with 443 additions and 487 deletions.
diff --git a/...-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java b/...-balancer/src/test/java/org/apache/hadoop/hbase/master/balancer/TestBaseLoadBalancer.java
@@ -128,7 +128,6 @@ protected List<RegionPlan> balanceTable(TableName tableName,
   public void testBulkAssignment() throws Exception {
     List<ServerName> tmp = getListOfServerNames(randomServers(5, 0));
     List<RegionInfo> hris = randomRegions(20);
-    hris.add(RegionInfoBuilder.FIRST_META_REGIONINFO);
     tmp.add(master);
     Map<ServerName, List<RegionInfo>> plans = loadBalancer.roundRobinAssignment(hris, tmp);
     int totalRegion = 0;

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java
@@ -110,14 +110,6 @@ private static int checkReplicaId(int regionId) {
     return regionId;
   }
 
-  /**
-   * Package private constructor used constructing MutableRegionInfo for the first meta regions
-   */
-  MutableRegionInfo(long regionId, TableName tableName, int replicaId) {
-    this(tableName, HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW, false, regionId,
-      replicaId, false);
-  }
-
   MutableRegionInfo(final TableName tableName, final byte[] startKey, final byte[] endKey,
       final boolean split, final long regionId, final int replicaId, boolean offLine) {
     this.tableName = checkTableName(tableName);

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -2426,13 +2426,6 @@ private CompletableFuture<RegionInfo> getRegionInfo(byte[] regionNameOrEncodedRe
       return failedFuture(new IllegalArgumentException("Passed region name can't be null"));
     }
 
-    if (Bytes.equals(regionNameOrEncodedRegionName,
-      RegionInfoBuilder.FIRST_META_REGIONINFO.getRegionName()) ||
-      Bytes.equals(regionNameOrEncodedRegionName,
-        RegionInfoBuilder.FIRST_META_REGIONINFO.getEncodedNameAsBytes())) {
-      return CompletableFuture.completedFuture(RegionInfoBuilder.FIRST_META_REGIONINFO);
-    }
-
     CompletableFuture<RegionInfo> future = new CompletableFuture<>();
     addListener(getRegionLocation(regionNameOrEncodedRegionName), (location, err) -> {
       if (err != null) {

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
@@ -73,8 +73,9 @@ public interface RegionInfo extends Comparable<RegionInfo> {
   @Deprecated
   @InterfaceAudience.Private
   // Not using RegionInfoBuilder intentionally to avoid a static loading deadlock: HBASE-24896
-  RegionInfo UNDEFINED = new MutableRegionInfo(0, TableName.valueOf("__UNDEFINED__"),
-    RegionInfo.DEFAULT_REPLICA_ID);
+  RegionInfo UNDEFINED =
+    new MutableRegionInfo(TableName.valueOf("__UNDEFINED__"), HConstants.EMPTY_START_ROW,
+      HConstants.EMPTY_END_ROW, false, 0, RegionInfo.DEFAULT_REPLICA_ID, false);
 
   /**
    * Separator used to demarcate the encodedName in a region name

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java
@@ -34,18 +34,6 @@ public class RegionInfoBuilder {
   public static final RegionInfo UNDEFINED =
     RegionInfoBuilder.newBuilder(TableName.valueOf("__UNDEFINED__")).build();
 
-  /**
-   * RegionInfo for first meta region
-   * You cannot use this builder to make an instance of the {@link #FIRST_META_REGIONINFO}.
-   * Just refer to this instance. Also, while the instance is actually a MutableRI, its type is
-   * just RI so the mutable methods are not available (unless you go casting); it appears
-   * as immutable (I tried adding Immutable type but it just makes a mess).
-   */
-  // TODO: How come Meta regions still do not have encoded region names? Fix.
-  // hbase:meta,,1.1588230740 should be the hbase:meta first region name.
-  public static final RegionInfo FIRST_META_REGIONINFO =
-    new MutableRegionInfo(1L, TableName.META_TABLE_NAME, RegionInfo.DEFAULT_REPLICA_ID);
-
   private final TableName tableName;
   private byte[] startKey = HConstants.EMPTY_START_ROW;
   private byte[] endKey = HConstants.EMPTY_END_ROW;
@@ -111,5 +99,4 @@ public RegionInfo build() {
     return new MutableRegionInfo(tableName, startKey, endKey, split,
         regionId, replicaId, offLine);
   }
-
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java
@@ -18,9 +18,6 @@
 package org.apache.hadoop.hbase.client;
 
 import static org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID;
-import static org.apache.hadoop.hbase.client.RegionInfoBuilder.FIRST_META_REGIONINFO;
-import static org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForDefaultReplica;
-import static org.apache.hadoop.hbase.client.RegionReplicaUtil.getRegionInfoForReplica;
 import static org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil.lengthOfPBMagic;
 import static org.apache.hadoop.hbase.trace.TraceUtil.tracedFuture;
 import static org.apache.hadoop.hbase.util.FutureUtils.addListener;
@@ -36,6 +33,7 @@
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.RegionLocations;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.exceptions.DeserializationException;
 import org.apache.hadoop.hbase.master.RegionState;
 import org.apache.hadoop.hbase.util.Pair;
@@ -161,7 +159,8 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
             LOG.warn("Meta region is in state " + stateAndServerName.getFirst());
           }
           locs[DEFAULT_REPLICA_ID] = new HRegionLocation(
-            getRegionInfoForDefaultReplica(FIRST_META_REGIONINFO), stateAndServerName.getSecond());
+            RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).setRegionId(1).build(),
+            stateAndServerName.getSecond());
           tryComplete(remaining, locs, future);
         });
       } else {
@@ -183,8 +182,8 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
               locs[replicaId] = null;
             } else {
               locs[replicaId] =
-                new HRegionLocation(getRegionInfoForReplica(FIRST_META_REGIONINFO, replicaId),
-                  stateAndServerName.getSecond());
+                new HRegionLocation(RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME)
+                  .setRegionId(1).setReplicaId(replicaId).build(), stateAndServerName.getSecond());
             }
           }
           tryComplete(remaining, locs, future);

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
@@ -89,7 +89,6 @@
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
 import org.apache.hadoop.hbase.client.RegionLoadStats;
 import org.apache.hadoop.hbase.client.RegionLocateType;
-import org.apache.hadoop.hbase.client.RegionReplicaUtil;
 import org.apache.hadoop.hbase.client.RegionStatesCount;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.RowMutations;
@@ -3237,8 +3236,8 @@ public static RegionState parseMetaRegionStateFrom(final byte[] data, int replic
     if (serverName == null) {
       state = RegionState.State.OFFLINE;
     }
-    return new RegionState(RegionReplicaUtil.getRegionInfoForReplica(
-        RegionInfoBuilder.FIRST_META_REGIONINFO, replicaId), state, serverName);
+    return new RegionState(RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).setRegionId(1)
+      .setReplicaId(replicaId).build(), state, serverName);
   }
 
   /**
@@ -3354,9 +3353,6 @@ public static org.apache.hadoop.hbase.client.RegionInfo toRegionInfo(final HBase
     long regionId = proto.getRegionId();
     int defaultReplicaId = org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID;
     int replicaId = proto.hasReplicaId()? proto.getReplicaId(): defaultReplicaId;
-    if (tableName.equals(TableName.META_TABLE_NAME) && replicaId == defaultReplicaId) {
-      return RegionInfoBuilder.FIRST_META_REGIONINFO;
-    }
     byte[] startKey = null;
     byte[] endKey = null;
     if (proto.hasStartKey()) {

diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionLocatorTracing.java
@@ -130,7 +130,7 @@ public static void setUpBeforeClass() {
 
   @Before
   public void setUp() throws IOException {
-    RegionInfo metaRegionInfo = RegionInfoBuilder.FIRST_META_REGIONINFO;
+    RegionInfo metaRegionInfo = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).build();
     locs = new RegionLocations(
       new HRegionLocation(metaRegionInfo,
         ServerName.valueOf("127.0.0.1", 12345, EnvironmentEdgeManager.currentTime())),

diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRegionInfoBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRegionInfoBuilder.java
@@ -84,10 +84,10 @@ public void testBuilder() {
 
   @Test
   public void testPb() throws DeserializationException {
-    RegionInfo ri = RegionInfoBuilder.FIRST_META_REGIONINFO;
+    RegionInfo ri = RegionInfoBuilder.newBuilder(name.getTableName()).build();
     byte[] bytes = RegionInfo.toByteArray(ri);
     RegionInfo pbri = RegionInfo.parseFrom(bytes);
-    assertTrue(RegionInfo.COMPARATOR.compare(ri, pbri) == 0);
+    assertEquals(0, RegionInfo.COMPARATOR.compare(ri, pbri));
   }
 
   @Test
@@ -183,7 +183,7 @@ public void testLastRegionCompare() {
 
   @Test
   public void testMetaTables() {
-    assertTrue(RegionInfoBuilder.FIRST_META_REGIONINFO.isMetaRegion());
+    assertTrue(RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).build().isMetaRegion());
   }
 
   @Test

diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestIPCUtil.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestIPCUtil.java
@@ -30,6 +30,8 @@
 import java.util.concurrent.TimeoutException;
 import org.apache.commons.lang3.mutable.MutableInt;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
 import org.apache.hadoop.hbase.exceptions.ClientExceptionsUtil;
 import org.apache.hadoop.hbase.exceptions.TimeoutIOException;
@@ -106,13 +108,12 @@ public void testWrapConnectionException() throws Exception {
       if (exception instanceof TimeoutException) {
         assertThat(IPCUtil.wrapException(addr, null, exception), instanceOf(TimeoutIOException.class));
       } else {
-        IOException ioe = IPCUtil.wrapException(addr, RegionInfoBuilder.FIRST_META_REGIONINFO,
-          exception);
+        RegionInfo ri = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).build();
+        IOException ioe = IPCUtil.wrapException(addr, ri, exception);
         // Assert that the exception contains the Region name if supplied. HBASE-25735.
         // Not all exceptions get the region stuffed into it.
         if (ioe.getMessage() != null) {
-          assertTrue(ioe.getMessage().
-            contains(RegionInfoBuilder.FIRST_META_REGIONINFO.getRegionNameAsString()));
+          assertTrue(ioe.getMessage().contains(ri.getRegionNameAsString()));
         }
         assertThat(ioe, instanceOf(exception.getClass()));
       }

diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/DistributedHBaseCluster.java b/hbase-it/src/test/java/org/apache/hadoop/hbase/DistributedHBaseCluster.java
@@ -306,6 +306,19 @@ public boolean waitForActiveAndReadyMaster(long timeout) throws IOException {
     return false;
   }
 
+  @Override
+  public ServerName getServerHoldingMeta() throws IOException {
+    HRegionLocation regionLoc = null;
+    try (RegionLocator locator = connection.getRegionLocator(TableName.META_TABLE_NAME)) {
+      regionLoc = locator.getRegionLocation(HConstants.EMPTY_START_ROW, true);
+    }
+    if (regionLoc == null) {
+      LOG.warn("Cannot find region server holding first meta region");
+      return null;
+    }
+    return regionLoc.getServerName();
+  }
+
   @Override
   public ServerName getServerHoldingRegion(TableName tn, byte[] regionName) throws IOException {
     byte[] startKey = RegionInfo.getStartKey(regionName);

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -89,8 +89,8 @@
 import org.apache.hadoop.hbase.client.NormalizeTableFilterParams;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.RegionInfo;
-import org.apache.hadoop.hbase.client.RegionInfoBuilder;
 import org.apache.hadoop.hbase.client.RegionLocateType;
+import org.apache.hadoop.hbase.client.RegionReplicaUtil;
 import org.apache.hadoop.hbase.client.RegionStatesCount;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.Scan;
@@ -816,13 +816,16 @@ private void tryMigrateRootTableFromZooKeeper() throws IOException, KeeperExcept
       }
     }
     // start migrating
-    byte[] row = CatalogFamilyFormat.getMetaKeyForRegion(RegionInfoBuilder.FIRST_META_REGIONINFO);
-    Put put = new Put(row);
+    Put put = null;
     List<String> metaReplicaNodes = zooKeeper.getMetaReplicaNodes();
     StringBuilder info = new StringBuilder("Migrating meta location:");
     for (String metaReplicaNode : metaReplicaNodes) {
       int replicaId = zooKeeper.getZNodePaths().getMetaReplicaIdFromZNode(metaReplicaNode);
       RegionState state = getMetaRegionState(zooKeeper, replicaId);
+      if (put == null) {
+        byte[] row = CatalogFamilyFormat.getMetaKeyForRegion(state.getRegion());
+        put = new Put(row);
+      }
       info.append(" ").append(state);
       put.setTimestamp(state.getStamp());
       MetaTableAccessor.addRegionInfo(put, state.getRegion());
@@ -834,9 +837,10 @@ private void tryMigrateRootTableFromZooKeeper() throws IOException, KeeperExcept
         .setQualifier(RegionStateStore.getStateColumn(replicaId)).setTimestamp(put.getTimestamp())
         .setType(Cell.Type.Put).setValue(Bytes.toBytes(state.getState().name())).build());
     }
-    if (!put.isEmpty()) {
+    if (put != null) {
       LOG.info(info.toString());
-      masterRegion.update(r -> r.put(put));
+      final Put p = put;
+      masterRegion.update(r -> r.put(p));
     } else {
       LOG.info("No meta location avaiable on zookeeper, skip migrating...");
     }
@@ -1276,11 +1280,14 @@ && getMasterProcedureExecutor().isRunning() && tries > 0) {
   /**
    * Check hbase:meta is up and ready for reading. For use during Master startup only.
    * @return True if meta is UP and online and startup can progress. Otherwise, meta is not online
-   *   and we will hold here until operator intervention.
+   *         and we will hold here until operator intervention.
    */
   @InterfaceAudience.Private
-  public boolean waitForMetaOnline() {
-    return isRegionOnline(RegionInfoBuilder.FIRST_META_REGIONINFO);
+  public boolean waitForMetaOnline() throws InterruptedException {
+    Optional<RegionInfo> firstMetaRegion =
+      this.assignmentManager.getRegionStates().getRegionsOfTable(TableName.META_TABLE_NAME).stream()
+        .filter(RegionInfo::isFirst).filter(RegionReplicaUtil::isDefaultReplica).findFirst();
+    return firstMetaRegion.isPresent() ? isRegionOnline(firstMetaRegion.get()) : false;
   }
 
   /**