HBASE-28158 handle split region scenarios and spotless

Author: ukumawat

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

@@ -2019,8 +2019,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi
     // Throttling by max number regions in transition
     while (
       !interrupted && maxRegionsInTransition > 0
-        && this.assignmentManager.getRegionsInTransitionCount()
-            >= maxRegionsInTransition
+        && this.assignmentManager.getRegionsInTransitionCount() >= maxRegionsInTransition
         && EnvironmentEdgeManager.currentTime() <= cutoffTime
     ) {
       try {
@@ -3080,8 +3079,7 @@ public ClusterMetrics getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
         }
         case REGIONS_IN_TRANSITION: {
           if (assignmentManager != null) {
-            builder.setRegionsInTransition(
-              assignmentManager.getRegionsStateInTransition());
+            builder.setRegionsInTransition(assignmentManager.getRegionsStateInTransition());
           }
           break;
         }

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

@@ -20,6 +20,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -337,6 +338,8 @@ public void start() throws IOException, KeeperException {
             regionNode.setLastHost(lastHost);
             regionNode.setRegionLocation(regionLocation);
             regionNode.setOpenSeqNum(openSeqNum);
+            regionInTransitionTracker.handleRegionStateNodeOperation(regionNode);
+
             if (regionNode.getProcedure() != null) {
               regionNode.getProcedure().stateLoaded(this, regionNode);
             }
@@ -1801,7 +1804,7 @@ public void joinCluster() throws IOException {
    */
   // Public so can be run by the Master as part of the startup. Needs hbase:meta to be online.
   // Needs to be done after the table state manager has been started.
-  //TODO umesh we can update the rit map here
+  // TODO umesh we can update the rit map here
   public void processOfflineRegions() {
     TransitRegionStateProcedure[] procs =
       regionStates.getRegionStateNodes().stream().filter(rsn -> rsn.isInState(State.OFFLINE))
@@ -2077,7 +2080,8 @@ public boolean hasRegionsInTransition() {
   }
 
   public List<RegionStateNode> getRegionsInTransition() {
-    return new ArrayList<RegionStateNode>(regionInTransitionTracker.getRegionsInTransition().values());
+    return new ArrayList<RegionStateNode>(
+      regionInTransitionTracker.getRegionsInTransition().values());
   }
 
   public boolean isRegionInTransition(final RegionInfo regionInfo) {
@@ -2107,7 +2111,6 @@ public SortedSet<RegionState> getRegionsInTransitionOrderedByTimestamp() {
     return rit;
   }
 
-
   public List<RegionInfo> getAssignedRegions() {
     return regionStates.getAssignedRegions();
   }
@@ -2173,7 +2176,7 @@ private CompletableFuture<Void> transitStateAndUpdate(RegionStateNode regionNode
       if (e != null) {
         // revert
         regionNode.setState(state);
-      }else{
+      } else {
         regionInTransitionTracker.handleRegionStateNodeOperation(regionNode);
       }
     });
@@ -2340,6 +2343,9 @@ public void markRegionAsSplit(final RegionInfo parent, final ServerName serverNa
     // it is a split parent. And usually only one of them can match, as after restart, the region
     // state will be changed from SPLIT to CLOSED.
     regionStateStore.splitRegion(parent, daughterA, daughterB, serverName, td);
+    regionInTransitionTracker.handleRegionStateNodeOperation(node);
+    regionInTransitionTracker.handleRegionStateNodeOperation(nodeA);
+    regionInTransitionTracker.handleRegionStateNodeOperation(nodeB);
     if (shouldAssignFavoredNodes(parent)) {
       List<ServerName> onlineServers = this.master.getServerManager().getOnlineServersList();
       getFavoredNodePromoter().generateFavoredNodesForDaughter(onlineServers, parent, daughterA,
@@ -2363,9 +2369,12 @@ public void markRegionAsMerged(final RegionInfo child, final ServerName serverNa
     for (RegionInfo ri : mergeParents) {
       regionStates.deleteRegion(ri);
     }
-    //TODO need to handle delete and new region
+    // TODO need to handle delete and new region
     TableDescriptor td = master.getTableDescriptors().get(child.getTable());
     regionStateStore.mergeRegions(child, mergeParents, serverName, td);
+    regionInTransitionTracker.handleRegionStateNodeOperation(node);
+    Arrays.stream(mergeParents).forEach(regionInfo -> regionInTransitionTracker
+      .handleRegionStateNodeOperation(regionStates.getRegionStateNode(regionInfo)));
     if (shouldAssignFavoredNodes(child)) {
       getFavoredNodePromoter().generateFavoredNodesForMergedRegion(child, mergeParents);
     }

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

@@ -1,3 +1,20 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.apache.hadoop.hbase.master.assignment;
 
 import java.util.List;
@@ -12,16 +29,16 @@
 
 @InterfaceAudience.Private
 public class RegionInTransitionTracker {
+  private static final Logger LOG = LoggerFactory.getLogger(RegionInTransitionTracker.class);
 
   private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
     List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
-  private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
-    List.of(RegionState.State.OPEN, RegionState.State.SPLIT, RegionState.State.MERGED);
 
-  private static final Logger LOG = LoggerFactory.getLogger(RegionInTransitionTracker.class);
+  private final List<RegionState.State> ENABLE_TABLE_REGION_STATE = List.of(RegionState.State.OPEN);
 
   private final ConcurrentSkipListMap<RegionInfo, RegionStateNode> regionInTransition =
     new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
   private final TableStateManager tableStateManager;
 
   public RegionInTransitionTracker(TableStateManager tableStateManager) {
@@ -34,8 +51,13 @@ public boolean isRegionInTransition(final RegionInfo regionInfo) {
 
   public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
     RegionState.State currentState = regionStateNode.getState();
-
-    if (!getExceptedRegionStates(regionStateNode).contains(currentState)) {
+    // if reiong is merged or split it should not be in RIT list
+    if (
+      currentState == RegionState.State.SPLIT || currentState == RegionState.State.MERGED
+        || regionStateNode.getRegionInfo().isSplit()
+    ) {
+      removeRegionInTransition(regionStateNode.getRegionInfo());
+    } else if (!getExceptedRegionStates(regionStateNode).contains(currentState)) {
       addRegionInTransition(regionStateNode);
     } else {
       removeRegionInTransition(regionStateNode.getRegionInfo());
@@ -47,8 +69,10 @@ public void handleRegionDelete(RegionStateNode regionStateNode) {
   }
 
   private List<RegionState.State> getExceptedRegionStates(RegionStateNode regionStateNode) {
-    if (tableStateManager.isTableState(regionStateNode.getTable(), TableState.State.ENABLED,
-      TableState.State.ENABLING)) {
+    if (
+      tableStateManager.isTableState(regionStateNode.getTable(), TableState.State.ENABLED,
+        TableState.State.ENABLING)
+    ) {
       return ENABLE_TABLE_REGION_STATE;
     } else {
       return DISABLE_TABLE_REGION_STATE;
@@ -57,13 +81,13 @@ private List<RegionState.State> getExceptedRegionStates(RegionStateNode regionSt
 
   public void addRegionInTransition(final RegionStateNode regionStateNode) {
     if (regionInTransition.putIfAbsent(regionStateNode.getRegionInfo(), regionStateNode) == null) {
-      LOG.info("Added to RIT list" + regionStateNode.getRegionInfo().getEncodedName());
+      LOG.info("Added to RIT list " + regionStateNode.getRegionInfo().getEncodedName());
     }
   }
 
   public void removeRegionInTransition(final RegionInfo regionInfo) {
     if (regionInTransition.remove(regionInfo) != null) {
-      LOG.info("Removed from RIT list" + regionInfo);
+      LOG.info("Removed from RIT list " + regionInfo.getEncodedName());
     }
   }
 

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

@@ -94,8 +94,7 @@ public void clear() {
   }
 
   public boolean isRegionInRegionStates(final RegionInfo hri) {
-    return (regionsMap.containsKey(hri.getRegionName())
-      || regionOffline.containsKey(hri));
+    return (regionsMap.containsKey(hri.getRegionName()) || regionOffline.containsKey(hri));
   }
 
   // ==========================================================================
@@ -591,7 +590,6 @@ private boolean isTableDisabled(final TableStateManager tableStateManager,
       TableState.State.DISABLING);
   }
 
-
   // ==========================================================================
   // Region offline helpers
   // ==========================================================================

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ProcedureSyncWait.java

@@ -33,7 +33,6 @@
 import org.apache.hadoop.hbase.master.RegionState;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
 import org.apache.hadoop.hbase.master.assignment.RegionStateNode;
-import org.apache.hadoop.hbase.master.assignment.RegionStates;
 import org.apache.hadoop.hbase.procedure2.Procedure;
 import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
 import org.apache.hadoop.hbase.quotas.MasterQuotaManager;

hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java

@@ -1191,8 +1191,8 @@ public BalanceResponse balanceRSGroup(String groupName, BalanceRequest request)
       Map<String, RegionState> groupRIT = rsGroupGetRegionsInTransition(groupName);
       if (groupRIT.size() > 0 && !request.isIgnoreRegionsInTransition()) {
         LOG.debug("Not running balancer because {} region(s) in transition: {}", groupRIT.size(),
-          StringUtils.abbreviate(masterServices.getAssignmentManager()
-            .getRegionsInTransition().toString(), 256));
+          StringUtils.abbreviate(
+            masterServices.getAssignmentManager().getRegionsInTransition().toString(), 256));
         return responseBuilder.build();
       }
 

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

@@ -3433,8 +3433,7 @@ public ExplainingPredicate<IOException> predicateNoRegionsInTransition() {
     return new ExplainingPredicate<IOException>() {
       @Override
       public String explainFailure() throws IOException {
-        final AssignmentManager am =
-          getMiniHBaseCluster().getMaster().getAssignmentManager();
+        final AssignmentManager am = getMiniHBaseCluster().getMaster().getAssignmentManager();
         return "found in transition: " + am.getRegionsInTransition().toString();
       }
 

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

@@ -193,8 +193,8 @@ public void testSplitRegionReplicaRitRecovery() throws Exception {
     ProcedureTestingUtility.waitProcedure(procExec, procId2);
     AssignmentTestingUtil.killRs(TEST_UTIL, serverName);
     Threads.sleepWithoutInterrupt(5000);
-    boolean hasRegionsInTransition = TEST_UTIL.getMiniHBaseCluster().getMaster()
-      .getAssignmentManager().hasRegionsInTransition();
+    boolean hasRegionsInTransition =
+      TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager().hasRegionsInTransition();
     assertEquals(false, hasRegionsInTransition);
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterBalanceThrottling.java

@@ -126,8 +126,8 @@ private Thread startBalancerChecker(final HMaster master, final AtomicInteger ma
       @Override
       public void run() {
         while (!stop.get()) {
-          maxCount.set(Math.max(maxCount.get(),
-            master.getAssignmentManager().getRegionsInTransitionCount()));
+          maxCount.set(
+            Math.max(maxCount.get(), master.getAssignmentManager().getRegionsInTransitionCount()));
           try {
             Thread.sleep(10);
           } catch (InterruptedException e) {

hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestFavoredStochasticLoadBalancer.java

@@ -381,7 +381,6 @@ public void testAllFavoredNodesDead() throws Exception {
     // Lets kill all the RS that are favored nodes for this region.
     stopServersAndWaitUntilProcessed(currentFN);
 
-
     final AssignmentManager am = master.getAssignmentManager();
     final RegionStates regionStates = am.getRegionStates();
     TEST_UTIL.waitFor(10000, new Waiter.Predicate<Exception>() {