diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index 7bfa79e21484..71b5a73cfa44 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -2112,10 +2112,6 @@ public List getRegionsInTransition() { return regionInTransitionTracker.getRegionsInTransition(); } - public boolean isRegionInTransition(final RegionInfo regionInfo) { - return regionInTransitionTracker.isRegionInTransition(regionInfo); - } - public int getRegionTransitScheduledCount() { return regionStates.getRegionTransitScheduledCount(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java index b5e0f03e4d75..bf49a3194802 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java @@ -41,22 +41,20 @@ public class RegionInTransitionTracker { private final List ENABLE_TABLE_REGION_STATE = List.of(RegionState.State.OPEN); + // DO NOT USE containsKey() on regionInTransition map as MutableRegionInfo.COMPARATOR considers + // offline/split flags. private final ConcurrentSkipListMap regionInTransition = new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR); private TableStateManager tableStateManager; - public boolean isRegionInTransition(final RegionInfo regionInfo) { - return regionInTransition.containsKey(regionInfo); - } - /** * Handles a region whose hosting RegionServer has crashed. When a RegionServer fails, all regions * it was hosting are automatically added to the RIT list since they need to be reassigned to * other servers. */ public void regionCrashed(RegionStateNode regionStateNode) { - if (regionStateNode.getRegionInfo().getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID) { + if (isReplica(regionStateNode) || isSplitOrMerged(regionStateNode)) { return; } @@ -76,7 +74,7 @@ public void regionCrashed(RegionStateNode regionStateNode) { */ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) { // only consider default replica for availability - if (regionStateNode.getRegionInfo().getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID) { + if (isReplica(regionStateNode)) { return; } @@ -86,10 +84,7 @@ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) { tableEnabled ? ENABLE_TABLE_REGION_STATE : DISABLE_TABLE_REGION_STATE; // if region is merged or split it should not be in RIT list - if ( - currentState == RegionState.State.SPLIT || currentState == RegionState.State.MERGED - || regionStateNode.getRegionInfo().isSplit() - ) { + if (isSplitOrMerged(regionStateNode)) { if (removeRegionInTransition(regionStateNode.getRegionInfo())) { LOG.debug("Removed {} from RIT list as it is split or merged", regionStateNode.getRegionInfo().getEncodedName()); @@ -107,6 +102,12 @@ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) { } } + private static boolean isSplitOrMerged(RegionStateNode regionStateNode) { + return regionStateNode.getState() == RegionState.State.SPLIT + || regionStateNode.getState() == RegionState.State.MERGED + || regionStateNode.getRegionInfo().isSplit(); + } + private boolean isTableEnabled(TableName tableName) { if (tableStateManager != null) { return tableStateManager.isTableState(tableName, TableState.State.ENABLED, @@ -153,4 +154,9 @@ public List getRegionsInTransition() { public void setTableStateManager(TableStateManager tableStateManager) { this.tableStateManager = tableStateManager; } + + private static boolean isReplica(RegionStateNode regionStateNode) { + return regionStateNode.getRegionInfo().getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID; + } + } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/AssignmentTestingUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/AssignmentTestingUtil.java index b63f02d2ff7a..9b8ba58f9e86 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/AssignmentTestingUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/AssignmentTestingUtil.java @@ -49,12 +49,17 @@ private AssignmentTestingUtil() { } public static void waitForRegionToBeInTransition(final HBaseTestingUtil util, - final RegionInfo hri) throws Exception { - while (!getMaster(util).getAssignmentManager().isRegionInTransition(hri)) { + final RegionInfo hri) { + while (!isRegionInTransition(hri, getMaster(util).getAssignmentManager())) { Threads.sleep(10); } } + public static boolean isRegionInTransition(RegionInfo hri, AssignmentManager am) { + return am.getRegionsInTransition().stream() + .anyMatch(rsn -> rsn.getRegionInfo().getEncodedName().equals(hri.getEncodedName())); + } + public static void waitForRsToBeDead(final HBaseTestingUtil util, final ServerName serverName) throws Exception { util.waitFor(60000, new ExplainingPredicate() { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionSplit.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionSplit.java index 13927082d175..828d0000badf 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionSplit.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionSplit.java @@ -19,8 +19,8 @@ import static org.apache.hadoop.hbase.master.assignment.AssignmentTestingUtil.insertData; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; import java.util.Map; @@ -117,8 +117,8 @@ public void testSplitTableRegion() throws Exception { int splitRowNum = startRowNum + rowCount / 2; byte[] splitKey = Bytes.toBytes("" + splitRowNum); - assertTrue(regions != null, "not able to find a splittable region"); - assertTrue(regions.length == 1, "not able to find a splittable region"); + assertNotNull(regions, "not able to find a splittable region"); + assertEquals(1, regions.length, "not able to find a splittable region"); // Split region of the table long procId = procExec.submitProcedure( @@ -127,7 +127,7 @@ public void testSplitTableRegion() throws Exception { ProcedureTestingUtility.waitProcedure(procExec, procId); ProcedureTestingUtility.assertProcNotFailed(procExec, procId); - assertTrue(UTIL.getHBaseCluster().getRegions(tableName).size() == 2, "not able to split table"); + assertEquals(2, UTIL.getHBaseCluster().getRegions(tableName).size(), "not able to split table"); // disable table UTIL.getAdmin().disableTable(tableName); @@ -155,6 +155,54 @@ public void testSplitTableRegion() throws Exception { regionInfoMap.get(tableRegions.get(1).getRegionInfo())); } + @Test + public void testRITWithSplitTableRegion() throws Exception { + final TableName tableName = TableName.valueOf(testMethodName); + final ProcedureExecutor procExec = getMasterProcedureExecutor(); + + RegionInfo[] regions = + MasterProcedureTestingUtility.createTable(procExec, tableName, null, columnFamilyName); + insertData(UTIL, tableName, rowCount, startRowNum, columnFamilyName); + int splitRowNum = startRowNum + rowCount / 2; + byte[] splitKey = Bytes.toBytes("" + splitRowNum); + + assertNotNull(regions, "not able to find a splittable region"); + assertEquals(1, regions.length, "not able to find a splittable region"); + assertEquals(0, + UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionsInTransitionCount()); + + ServerName targetRS = UTIL.getHBaseCluster().getMaster().getAssignmentManager() + .getRegionStates().getRegionServerOfRegion(regions[0]); + // Split region of the table + long procId = procExec.submitProcedure( + new SplitTableRegionProcedure(procExec.getEnvironment(), regions[0], splitKey)); + // Wait the completion + ProcedureTestingUtility.waitProcedure(procExec, procId); + ProcedureTestingUtility.assertProcNotFailed(procExec, procId); + + assertEquals(2, UTIL.getHBaseCluster().getRegions(tableName).size(), "not able to split table"); + assertEquals(0, + UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionsInTransitionCount()); + // As there are only 3 RS, start one more RS before expiring one + UTIL.getHBaseCluster().startRegionServer(); + + // stop RS holding split parent + UTIL.getHBaseCluster().getMaster().getServerManager().expireServer(targetRS); + + // stop master + UTIL.getHBaseCluster().stopMaster(0); + UTIL.getHBaseCluster().waitOnMaster(0); + Thread.sleep(500); + + // restart master + JVMClusterUtil.MasterThread t = UTIL.getHBaseCluster().startMaster(); + UTIL.getHBaseCluster().waitForActiveAndReadyMaster(10000); + UTIL.invalidateConnection(); + + assertFalse(AssignmentTestingUtil.isRegionInTransition(regions[0], + UTIL.getHBaseCluster().getMaster().getAssignmentManager())); + } + @Test public void testSplitStoreFiles() throws Exception { final TableName tableName = TableName.valueOf(testMethodName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index 6aa195123062..e98ad9189b00 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -907,7 +907,8 @@ public void testSplitRegionWithNoStoreFiles() throws Exception { } catch (DoNotRetryIOException e) { // Expected } - assertFalse(am.isRegionInTransition(hri), "Split region can't be assigned"); + assertFalse(AssignmentTestingUtil.isRegionInTransition(hri, am), + "Split region can't be assigned"); assertTrue(regionStates.isRegionInState(hri, State.SPLIT)); // We should not be able to unassign it either @@ -917,7 +918,8 @@ public void testSplitRegionWithNoStoreFiles() throws Exception { } catch (DoNotRetryIOException e) { // Expected } - assertFalse(am.isRegionInTransition(hri), "Split region can't be unassigned"); + assertFalse(AssignmentTestingUtil.isRegionInTransition(hri, am), + "Split region can't be unassigned"); assertTrue(regionStates.isRegionInState(hri, State.SPLIT)); } finally { admin.balancerSwitch(true, false);