HBASE-29363: CompactSplit should not attempt to split secondary region replicas

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java

@@ -36,6 +36,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.IntSupplier;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.conf.ConfigurationManager;
 import org.apache.hadoop.hbase.conf.PropagatingConfigurationObserver;
 import org.apache.hadoop.hbase.quotas.RegionServerSpaceQuotaManager;
@@ -205,7 +206,7 @@ public synchronized boolean requestSplit(final Region r) {
     // continuously growing, as well as the number of store files, see HBASE-26242.
     HRegion hr = (HRegion) r;
     try {
-      if (shouldSplitRegion() && hr.getCompactPriority() >= PRIORITY_USER) {
+      if (shouldSplitRegion(r.getRegionInfo()) && hr.getCompactPriority() >= PRIORITY_USER) {
         byte[] midKey = hr.checkSplit().orElse(null);
         if (midKey != null) {
           requestSplit(r, midKey);
@@ -503,12 +504,15 @@ public int getSplitQueueSize() {
     return splits.getQueue().size();
   }
 
-  private boolean shouldSplitRegion() {
+  private boolean shouldSplitRegion(RegionInfo ri) {
     if (server.getNumberOfOnlineRegions() > 0.9 * regionSplitLimit) {
       LOG.warn("Total number of regions is approaching the upper limit " + regionSplitLimit + ". "
         + "Please consider taking a look at http://hbase.apache.org/book.html#ops.regionmgt");
     }
-    return (regionSplitLimit > server.getNumberOfOnlineRegions());
+    return (regionSplitLimit > server.getNumberOfOnlineRegions()
+      // Do not attempt to split secondary region replicas, as this is not allowed and our request
+      // to do so will be rejected
+      && ri.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID);
   }
 
   /** Returns the regionSplitLimit */
@@ -807,6 +811,11 @@ protected int getSplitThreadNum() {
     return this.splits.getCorePoolSize();
   }
 
+  /** Exposed for unit testing */
+  long getSubmittedSplitsCount() {
+    return this.splits.getTaskCount();
+  }
+
   /**
    * {@inheritDoc}
    */

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

@@ -36,9 +36,8 @@
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.CommonFSUtils;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Assert;
-import org.junit.BeforeClass;
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -64,8 +63,8 @@ public class TestCompactSplitThread {
   /**
    * Setup the config for the cluster
    */
-  @BeforeClass
-  public static void setupCluster() throws Exception {
+  @Before
+  public void setupCluster() throws Exception {
     setupConf(TEST_UTIL.getConfiguration());
     TEST_UTIL.startMiniCluster(NUM_RS);
     fs = TEST_UTIL.getDFSCluster().getFileSystem();
@@ -92,12 +91,7 @@ private static void setupConf(Configuration conf) {
   }
 
   @After
-  public void tearDown() throws Exception {
-    TEST_UTIL.deleteTable(tableName);
-  }
-
-  @AfterClass
-  public static void cleanupTest() throws Exception {
+  public void cleanupTest() throws Exception {
     try {
       TEST_UTIL.shutdownMiniCluster();
     } catch (Exception e) {
@@ -173,4 +167,22 @@ public void testFlushWithTableCompactionDisabled() throws Exception {
     Collection<String> hfiles = SnapshotTestingUtils.listHFileNames(fs, tableDir);
     assert (hfiles.size() > blockingStoreFiles + 1);
   }
+
+  @Test
+  public void testFlushWithRegionReplicas() throws Exception {
+    TableDescriptor htd =
+      TableDescriptorBuilder.newBuilder(tableName).setRegionReplication(2).build();
+    TEST_UTIL.createTable(htd, new byte[][] { family }, null);
+
+    // load the table
+    for (int i = 0; i < blockingStoreFiles + 1; i++) {
+      TEST_UTIL.loadTable(TEST_UTIL.getConnection().getTable(tableName), family);
+      TEST_UTIL.flush(tableName);
+    }
+
+    // One region split should have taken place, because the primary replica gets split, and not the
+    // secondary replica.
+    assertEquals(1, TEST_UTIL.getRSForFirstRegionInTable(tableName).getCompactSplitThread()
+      .getSubmittedSplitsCount());
+  }
 }