HBASE-11632 Region split needs to clear force split flag at the end of SplitRequest run (Jerry He)

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

@@ -215,6 +215,9 @@ public class CompactSplitThread implements CompactionRequestor {
     if (midKey == null) {
       LOG.debug("Region " + r.getRegionNameAsString() +
         " not splittable because midkey=null");
+      if (r.shouldForceSplit()) {
+        r.clearSplit();
+      }
       return;
     }
     try {

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

@@ -5797,16 +5797,17 @@ public class HRegion implements HeapSize { // , Writable{
   }
 
   void forceSplit(byte[] sp) {
-    // NOTE : this HRegion will go away after the forced split is successful
-    //        therefore, no reason to clear this value
+    // 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_TESTS_ONLY() {
+  void clearSplit() {
     this.splitRequest = false;
+    this.explicitSplitPoint = null;
   }
 
   /**

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

@@ -124,6 +124,9 @@ class SplitRequest implements Runnable {
             io instanceof RemoteException ? ((RemoteException) io).unwrapRemoteException() : io);
         }
       }
+      if (parent.shouldForceSplit()) {
+        parent.clearSplit();
+      }
       releaseTableLock();
     }
   }

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

@@ -144,6 +144,7 @@ import org.junit.rules.TestName;
 import org.mockito.ArgumentMatcher;
 import org.mockito.Mockito;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.protobuf.ByteString;
 
@@ -2358,8 +2359,10 @@ public class TestHRegion {
     SplitTransaction st = new SplitTransaction(parent, midkey);
     // If prepare does not return true, for some reason -- logged inside in
     // the prepare call -- we are not ready to split just now. Just return.
-    if (!st.prepare())
+    if (!st.prepare()) {
+      parent.clearSplit();
       return null;
+    }
     try {
       result = st.execute(null, null);
     } catch (IOException ioe) {
@@ -2375,6 +2378,9 @@ public class TestHRegion {
             + " -- aborting server", e);
       }
     }
+    finally {
+      parent.clearSplit();
+    }
     return new HRegion[] { result.getFirst(), result.getSecond() };
   }
 
@@ -3329,6 +3335,74 @@ public class TestHRegion {
     }
   }
 
+  @Test
+  public void testClearForceSplit() throws IOException {
+    byte[] qualifier = Bytes.toBytes("qualifier");
+    Configuration hc = initSplit();
+    int numRows = 10;
+    byte[][] families = { fam1, fam3 };
+
+    // Setting up region
+    String method = this.getName();
+    this.region = initHRegion(tableName, method, hc, families);
+
+    // Put data in region
+    int startRow = 100;
+    putData(startRow, numRows, qualifier, families);
+    int splitRow = startRow + numRows;
+    byte[] splitRowBytes = Bytes.toBytes("" + splitRow);
+    putData(splitRow, numRows, qualifier, families);
+    region.flushcache();
+
+    HRegion[] regions = null;
+    try {
+      // Set force split
+      region.forceSplit(splitRowBytes);
+      assertTrue(region.shouldForceSplit());
+      // Split point should be the force split row
+      assertTrue(Bytes.equals(splitRowBytes, region.checkSplit()));
+
+      // Add a store that has references.
+      HStore storeMock = Mockito.mock(HStore.class);
+      when(storeMock.hasReferences()).thenReturn(true);
+      when(storeMock.getFamily()).thenReturn(new HColumnDescriptor("cf"));
+      when(storeMock.close()).thenReturn(ImmutableList.<StoreFile>of());
+      when(storeMock.getColumnFamilyName()).thenReturn("cf");
+      region.stores.put(Bytes.toBytes(storeMock.getColumnFamilyName()), storeMock);
+      assertTrue(region.hasReferences());
+
+      // Will not split since the store has references.
+      regions = splitRegion(region, splitRowBytes);
+      assertNull(regions);
+
+      // Region force split should be cleared after the split try.
+      assertFalse(region.shouldForceSplit());
+
+      // Remove the store that has references.
+      region.stores.remove(Bytes.toBytes(storeMock.getColumnFamilyName()));
+      assertFalse(region.hasReferences());
+
+      // Now we can split.
+      regions = splitRegion(region, splitRowBytes);
+
+      // Opening the regions returned.
+      for (int i = 0; i < regions.length; i++) {
+        regions[i] = HRegion.openHRegion(regions[i], null);
+      }
+      // Verifying that the region has been split
+      assertEquals(2, regions.length);
+
+      // Verifying that all data is still there and that data is in the right
+      // place
+      verifyData(regions[0], startRow, numRows, qualifier, families);
+      verifyData(regions[1], splitRow, numRows, qualifier, families);
+
+    } finally {
+      HRegion.closeHRegion(this.region);
+      this.region = null;
+    }
+  }
+
   /**
    * Flushes the cache in a thread while scanning. The tests verify that the
    * scan is coherent - e.g. the returned results are always of the same or

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

@@ -925,7 +925,7 @@ public class TestStore {
     Assert.assertNull(store.getSplitPoint());
     store.getHRegion().forceSplit(null);
     Assert.assertNull(store.getSplitPoint());
-    store.getHRegion().clearSplit_TESTS_ONLY();
+    store.getHRegion().clearSplit();
   }
 
   @Test