HBASE-24648 Remove the legacy 'forceSplit' related code at region server side (#1990)

Signed-off-by: Viraj Jasani <vjasani@apache.org>

Author: Apache9

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

@@ -167,12 +167,15 @@ public RegionInfo getDaughterTwoRI() {
     return daughterTwoRI;
   }
 
+  private boolean hasBestSplitRow() {
+    return bestSplitRow != null && bestSplitRow.length > 0;
+  }
+
   /**
    * Check whether the region is splittable
    * @param env MasterProcedureEnv
    * @param regionToSplit parent Region to be split
    * @param splitRow if splitRow is not specified, will first try to get bestSplitRow from RS
-   * @throws IOException
    */
   private void checkSplittable(final MasterProcedureEnv env,
       final RegionInfo regionToSplit, final byte[] splitRow) throws IOException {
@@ -187,19 +190,20 @@ private void checkSplittable(final MasterProcedureEnv env,
     boolean splittable = false;
     if (node != null) {
       try {
-        if (bestSplitRow == null || bestSplitRow.length == 0) {
-          LOG
-            .info("splitKey isn't explicitly specified, will try to find a best split key from RS");
-        }
-        // Always set bestSplitRow request as true here,
-        // need to call Region#checkSplit to check it splittable or not
-        GetRegionInfoResponse response = AssignmentManagerUtil.getRegionInfoResponse(env,
-          node.getRegionLocation(), node.getRegionInfo(), true);
-        if(bestSplitRow == null || bestSplitRow.length == 0) {
-          bestSplitRow = response.hasBestSplitRow() ? response.getBestSplitRow().toByteArray() : null;
+        GetRegionInfoResponse response;
+        if (!hasBestSplitRow()) {
+          LOG.info(
+            "{} splitKey isn't explicitly specified, will try to find a best split key from RS {}",
+            node.getRegionInfo().getRegionNameAsString(), node.getRegionLocation());
+          response = AssignmentManagerUtil.getRegionInfoResponse(env, node.getRegionLocation(),
+            node.getRegionInfo(), true);
+          bestSplitRow =
+            response.hasBestSplitRow() ? response.getBestSplitRow().toByteArray() : null;
+        } else {
+          response = AssignmentManagerUtil.getRegionInfoResponse(env, node.getRegionLocation(),
+            node.getRegionInfo(), false);
         }
         splittable = response.hasSplittable() && response.getSplittable();
-
         if (LOG.isDebugEnabled()) {
           LOG.debug("Splittable=" + splittable + " " + node.toShortString());
         }

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

@@ -191,7 +191,7 @@ public synchronized boolean requestSplit(final Region r) {
     HRegion hr = (HRegion)r;
     try {
       if (shouldSplitRegion() && hr.getCompactPriority() >= PRIORITY_USER) {
-        byte[] midKey = hr.checkSplit();
+        byte[] midKey = hr.checkSplit().orElse(null);
         if (midKey != null) {
           requestSplit(r, midKey);
           return true;
@@ -216,9 +216,6 @@ public synchronized void requestSplit(final Region r, byte[] midKey, User user)
     if (midKey == null) {
       LOG.debug("Region " + r.getRegionInfo().getRegionNameAsString() +
         " not splittable because midkey=null");
-      if (((HRegion)r).shouldForceSplit()) {
-        ((HRegion)r).clearSplit();
-      }
       return;
     }
     try {

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

@@ -68,7 +68,6 @@ protected void configureForRegion(HRegion region) {
 
   @Override
   protected boolean shouldSplit() {
-    boolean force = region.shouldForceSplit();
     boolean foundABigStore = false;
 
     for (HStore store : region.getStores()) {
@@ -84,7 +83,7 @@ protected boolean shouldSplit() {
       }
     }
 
-    return foundABigStore || force;
+    return foundABigStore;
   }
 
   long getDesiredMaxFileSize() {

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

@@ -22,15 +22,20 @@
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 
 /**
- * A {@link RegionSplitPolicy} that disables region splits.
- * This should be used with care, since it will disable automatic sharding.
- * Most of the time, using {@link ConstantSizeRegionSplitPolicy} with a
- * large region size (10GB, etc) is safer.
+ * A {@link RegionSplitPolicy} that disables region splits. This should be used with care, since it
+ * will disable automatic sharding. Most of the time, using {@link ConstantSizeRegionSplitPolicy}
+ * with a large region size (10GB, etc) is safer.
  */
 @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
 public class DisabledRegionSplitPolicy extends RegionSplitPolicy {
+
   @Override
   protected boolean shouldSplit() {
     return false;
   }
+
+  @Override
+  protected boolean canSplit() {
+    return false;
+  }
 }

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

@@ -696,8 +696,6 @@ void sawNoSuchFamily() {
 
   // Stop updates lock
   private final ReentrantReadWriteLock updatesLock = new ReentrantReadWriteLock();
-  private boolean splitRequest;
-  private byte[] explicitSplitPoint = null;
 
   private final MultiVersionConcurrencyControl mvcc;
 
@@ -1478,7 +1476,7 @@ public boolean isAvailable() {
 
   @Override
   public boolean isSplittable() {
-    return isAvailable() && !hasReferences();
+    return splitPolicy.canSplit();
   }
 
   @Override
@@ -1977,7 +1975,8 @@ public void setMobFileCache(MobFileCache mobFileCache) {
   /**
    * @return split policy for this region.
    */
-  public RegionSplitPolicy getSplitPolicy() {
+  @VisibleForTesting
+  RegionSplitPolicy getSplitPolicy() {
     return this.splitPolicy;
   }
 
@@ -8408,9 +8407,9 @@ private static List<Cell> sort(List<Cell> cells, final CellComparator comparator
 
   public static final long FIXED_OVERHEAD = ClassSize.align(
       ClassSize.OBJECT +
-      ClassSize.ARRAY +
-      55 * ClassSize.REFERENCE + 3 * Bytes.SIZEOF_INT +
-      (15 * Bytes.SIZEOF_LONG) +
+      56 * ClassSize.REFERENCE +
+      3 * Bytes.SIZEOF_INT +
+      14 * Bytes.SIZEOF_LONG +
       3 * Bytes.SIZEOF_BOOLEAN);
 
   // woefully out of date - currently missing:
@@ -8537,50 +8536,26 @@ public void run(Message message) {
     return responseBuilder.build();
   }
 
-  boolean shouldForceSplit() {
-    return this.splitRequest;
-  }
-
-  byte[] getExplicitSplitPoint() {
-    return this.explicitSplitPoint;
-  }
-
-  void forceSplit(byte[] sp) {
-    // This HRegion will go away after the forced split is successful
-    // But if a forced split fails, we need to clear forced split.
-    this.splitRequest = true;
-    if (sp != null) {
-      this.explicitSplitPoint = sp;
-    }
-  }
-
-  void clearSplit() {
-    this.splitRequest = false;
-    this.explicitSplitPoint = null;
+  public Optional<byte[]> checkSplit() {
+    return checkSplit(false);
   }
 
   /**
-   * Return the splitpoint. null indicates the region isn't splittable
-   * If the splitpoint isn't explicitly specified, it will go over the stores
-   * to find the best splitpoint. Currently the criteria of best splitpoint
-   * is based on the size of the store.
+   * Return the split point. An empty result indicates the region isn't splittable.
    */
-  public byte[] checkSplit() {
+  public Optional<byte[]> checkSplit(boolean force) {
     // Can't split META
     if (this.getRegionInfo().isMetaRegion()) {
-      if (shouldForceSplit()) {
-        LOG.warn("Cannot split meta region in HBase 0.20 and above");
-      }
-      return null;
+      return Optional.empty();
     }
 
     // Can't split a region that is closing.
     if (this.isClosing()) {
-      return null;
+      return Optional.empty();
     }
 
-    if (!splitPolicy.shouldSplit()) {
-      return null;
+    if (!force && !splitPolicy.shouldSplit()) {
+      return Optional.empty();
     }
 
     byte[] ret = splitPolicy.getSplitPoint();
@@ -8590,10 +8565,12 @@ public byte[] checkSplit() {
         checkRow(ret, "calculated split");
       } catch (IOException e) {
         LOG.error("Ignoring invalid split for region {}", this, e);
-        return null;
+        return Optional.empty();
       }
+      return Optional.of(ret);
+    } else {
+      return Optional.empty();
     }
-    return ret;
   }
 
   /**

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

@@ -70,7 +70,6 @@ protected void configureForRegion(HRegion region) {
 
   @Override
   protected boolean shouldSplit() {
-    boolean force = region.shouldForceSplit();
     boolean foundABigStore = false;
     // Get count of regions that have the same common table as this.region
     int tableRegionsCount = getCountOfCommonTableRegions();
@@ -95,7 +94,7 @@ protected boolean shouldSplit() {
       }
     }
 
-    return foundABigStore || force;
+    return foundABigStore;
   }
 
   /**

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

@@ -620,7 +620,7 @@ private boolean flushRegion(HRegion region, boolean emergencyFlush,
       FlushResult flushResult = region.flushcache(families, false, tracker);
       boolean shouldCompact = flushResult.isCompactionNeeded();
       // We just want to check the size
-      boolean shouldSplit = region.checkSplit() != null;
+      boolean shouldSplit = region.checkSplit().isPresent();
       if (shouldSplit) {
         this.server.compactSplitThread.requestSplit(region);
       } else if (shouldCompact) {

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

@@ -1837,35 +1837,30 @@ public GetOnlineRegionResponse getOnlineRegion(final RpcController controller,
   @Override
   @QosPriority(priority=HConstants.ADMIN_QOS)
   public GetRegionInfoResponse getRegionInfo(final RpcController controller,
-      final GetRegionInfoRequest request) throws ServiceException {
+    final GetRegionInfoRequest request) throws ServiceException {
     try {
       checkOpen();
       requestCount.increment();
       HRegion region = getRegion(request.getRegion());
       RegionInfo info = region.getRegionInfo();
-      byte[] bestSplitRow = null;
-      boolean shouldSplit = true;
+      byte[] bestSplitRow;
       if (request.hasBestSplitRow() && request.getBestSplitRow()) {
-        HRegion r = region;
-        region.startRegionOperation(Operation.SPLIT_REGION);
-        r.forceSplit(null);
-        // Even after setting force split if split policy says no to split then we should not split.
-        shouldSplit = region.getSplitPolicy().shouldSplit() && !info.isMetaRegion();
-        bestSplitRow = r.checkSplit();
+        bestSplitRow = region.checkSplit(true).orElse(null);
         // when all table data are in memstore, bestSplitRow = null
         // try to flush region first
-        if(bestSplitRow == null) {
-          r.flush(true);
-          bestSplitRow = r.checkSplit();
+        if (bestSplitRow == null) {
+          region.flush(true);
+          bestSplitRow = region.checkSplit(true).orElse(null);
         }
-        r.clearSplit();
+      } else {
+        bestSplitRow = null;
       }
       GetRegionInfoResponse.Builder builder = GetRegionInfoResponse.newBuilder();
       builder.setRegionInfo(ProtobufUtil.toRegionInfo(info));
       if (request.hasCompactionState() && request.getCompactionState()) {
         builder.setCompactionState(ProtobufUtil.createCompactionState(region.getCompactionState()));
       }
-      builder.setSplittable(region.isSplittable() && shouldSplit);
+      builder.setSplittable(region.isSplittable());
       builder.setMergeable(region.isMergeable());
       if (request.hasBestSplitRow() && request.getBestSplitRow() && bestSplitRow != null) {
         builder.setBestSplitRow(UnsafeByteOperations.unsafeWrap(bestSplitRow));

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

@@ -70,16 +70,20 @@ protected void configureForRegion(HRegion region) {
    */
   protected abstract boolean shouldSplit();
 
+  /**
+   * @return {@code true} if the specified region can be split.
+   */
+  protected boolean canSplit() {
+    return !region.getRegionInfo().isMetaRegion() && region.isAvailable() &&
+      !region.hasReferences();
+  }
+
   /**
    * @return the key at which the region should be split, or null
    * if it cannot be split. This will only be called if shouldSplit
    * previously returned true.
    */
   protected byte[] getSplitPoint() {
-    byte[] explicitSplitPoint = this.region.getExplicitSplitPoint();
-    if (explicitSplitPoint != null) {
-      return explicitSplitPoint;
-    }
     List<HStore> stores = region.getStores();
 
     byte[] splitPointFromLargestStore = null;

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java

@@ -3139,9 +3139,8 @@ public void unassignRegionByRow(byte[] row, RegionLocator table) throws IOExcept
     unassignRegion(hrl.getRegion().getRegionName());
   }
 
-  /*
+  /**
    * Retrieves a splittable region randomly from tableName
-   *
    * @param tableName name of table
    * @param maxAttempts maximum number of attempts, unlimited for value of -1
    * @return the HRegion chosen, null if none was found within limit of maxAttempts
@@ -3164,15 +3163,14 @@ public HRegion getSplittableRegion(TableName tableName, int maxAttempts) {
       if (regCount > 0) {
         idx = random.nextInt(regCount);
         // if we have just tried this region, there is no need to try again
-        if (attempted.contains(idx))
+        if (attempted.contains(idx)) {
           continue;
-        try {
-          regions.get(idx).checkSplit();
-          return regions.get(idx);
-        } catch (Exception ex) {
-          LOG.warn("Caught exception", ex);
-          attempted.add(idx);
         }
+        HRegion region = regions.get(idx);
+        if (region.checkSplit().isPresent()) {
+          return region;
+        }
+        attempted.add(idx);
       }
       attempts++;
     } while (maxAttempts == -1 || attempts < maxAttempts);

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

@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -70,6 +71,7 @@
 import org.junit.rules.TestName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.apache.hbase.thirdparty.com.google.common.collect.Iterators;
 import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
 import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
@@ -121,7 +123,6 @@ public void testCanSplitJustAfterASplit() throws Exception {
       admin.createTable(htd);
       TEST_UTIL.loadTable(source, fam);
       compactSplit.setCompactionsEnabled(false);
-      TEST_UTIL.getHBaseCluster().getRegions(tableName).get(0).forceSplit(null);
       admin.split(tableName);
       TEST_UTIL.waitFor(60000, () -> TEST_UTIL.getHBaseCluster().getRegions(tableName).size() == 2);
 

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

@@ -873,9 +873,6 @@ public void testMultipleTimestamps() throws IOException {
   public void testSplitWithEmptyColFam() throws IOException {
     init(this.name.getMethodName());
     assertFalse(store.getSplitPoint().isPresent());
-    store.getHRegion().forceSplit(null);
-    assertFalse(store.getSplitPoint().isPresent());
-    store.getHRegion().clearSplit();
   }
 
   @Test