HBASE-26618 Involving primary meta region in meta scan with CatalogRe… (#4321) (#4326)

Signed-off-by: Michael Stack <stack@apache.org>

hbase-client/src/main/java/org/apache/hadoop/hbase/client/CatalogReplicaLoadBalanceSimpleSelector.java

@@ -46,15 +46,15 @@ import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
  * balancing algorithm. It maintains a stale location cache for each table. Whenever client looks
  * up location, it first check if the row is the stale location cache. If yes, the location from
  * catalog replica is stale, it will go to the primary region to look up update-to-date location;
- * otherwise, it will randomly pick up a replica region for lookup. When clients receive
+ * otherwise, it will randomly pick up a replica region or primary region for lookup. When clients
- * RegionNotServedException from region servers, it will add these region locations to the stale
+ * receive RegionNotServedException from region servers, it will add these region locations to the
- * location cache. The stale cache will be cleaned up periodically by a chore.</p>
+ * stale location cache. The stale cache will be cleaned up periodically by a chore.</p>
  *
- * It follows a simple algorithm to choose a replica to go:
+ * It follows a simple algorithm to choose a meta replica region (including primary meta) to go:
  *
  * <ol>
  *  <li>If there is no stale location entry for rows it looks up, it will randomly
- *     pick a replica region to do lookup. </li>
+ *     pick a meta replica region (including primary meta) to do lookup. </li>
  *  <li>If the location from the replica region is stale, client gets RegionNotServedException
  *     from region server, in this case, it will create StaleLocationCacheEntry in
  *     CatalogReplicaLoadBalanceReplicaSimpleSelector.</li>
@@ -141,7 +141,7 @@ class CatalogReplicaLoadBalanceSimpleSelector implements
   }
 
   /**
-   * Select an random replica id. In case there is no replica region configured, return
+   * Select an random replica id (including the primary replica id). In case there is no replica region configured, return
    * the primary replica id.
    * @return Replica id
    */
@@ -155,7 +155,7 @@ class CatalogReplicaLoadBalanceSimpleSelector implements
     if (cachedNumOfReplicas <= 1) {
       return RegionInfo.DEFAULT_REPLICA_ID;
     }
-    return 1 + ThreadLocalRandom.current().nextInt(cachedNumOfReplicas - 1);
+    return ThreadLocalRandom.current().nextInt(cachedNumOfReplicas);
   }
 
   /**

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCatalogReplicaLoadBalanceSimpleSelector.java

@@ -24,6 +24,7 @@ import static org.junit.Assert.assertNotEquals;
 
 import java.io.IOException;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
@@ -105,9 +106,13 @@ public class TestCatalogReplicaLoadBalanceSimpleSelector {
         return numOfReplicas;
       });
 
-    assertNotEquals(
+    // Loop for 100 times, it should cover all replica ids.
-      metaSelector.select(TableName.valueOf("test"), EMPTY_START_ROW, RegionLocateType.CURRENT),
+    int[] replicaIdCount = new int[numOfMetaReplica];
-      RegionReplicaUtil.DEFAULT_REPLICA_ID);
+    IntStream.range(1, 100).forEach(i -> replicaIdCount[metaSelector.select(
+      TableName.valueOf("test"), EMPTY_START_ROW, RegionLocateType.CURRENT)] ++);
+
+    // Make sure each replica id is returned by select() call, including primary replica id.
+    IntStream.range(0, numOfMetaReplica).forEach(i -> assertNotEquals(replicaIdCount[i], 0));
 
     // Change to No meta replica
     HBaseTestingUtility.setReplicas(admin, TableName.META_TABLE_NAME, 1);

hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestMetaRegionReplicaReplicationEndpoint.java

@@ -519,6 +519,16 @@ public class TestMetaRegionReplicaReplicationEndpoint {
     }
   }
 
+  private void primaryIncreaseReplicaIncrease(final long[] before, final long[] after) {
+    // There are read requests increase for primary meta replica.
+    assertTrue(after[RegionInfo.DEFAULT_REPLICA_ID] > before[RegionInfo.DEFAULT_REPLICA_ID]);
+
+    // There are read requests incrase for meta replica regions.
+    for (int i = 1; i < after.length; i++) {
+      assertTrue(after[i] > before[i]);
+    }
+  }
+
   private void getMetaReplicaReadRequests(final Region[] metaRegions, final long[] counters) {
     int i = 0;
     for (Region r : metaRegions) {
@@ -579,9 +589,8 @@ public class TestMetaRegionReplicaReplicationEndpoint {
 
       getMetaReplicaReadRequests(metaRegions, readReqsForMetaReplicasAfterGet);
 
-      // There is no read requests increase for primary meta replica.
+      // There are more reads against all meta replica regions, including the primary region.
-      // For rest of meta replicas, there are more reads against them.
+      primaryIncreaseReplicaIncrease(readReqsForMetaReplicas, readReqsForMetaReplicasAfterGet);
-      primaryNoChangeReplicaIncrease(readReqsForMetaReplicas, readReqsForMetaReplicasAfterGet);
 
       // move one of regions so it meta cache may be invalid.
       HTU.moveRegionAndWait(userRegion.getRegionInfo(), destRs.getServerName());