From 824212f14cb64833213f9a548a37f162a7913237 Mon Sep 17 00:00:00 2001
From: Andrew Purtell <apurtell@apache.org>
Date: Sun, 2 May 2021 14:23:05 -0700
Subject: [PATCH] HBASE-25836 RegionStates#getAssignmentsForBalancer should
 only care about OPEN or OPENING regions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RegionStates#getAssignmentsForBalancer is used by the HMaster to
collect all regions of interest to the balancer for the next chore
iteration. We check if a table is in disabled state to exclude
regions that will not be of interest (because disabled regions are
or will be offline) or are in a state where they shouldn't be
mutated (like SPLITTING). The current checks are not actually
comprehensive.

Filter out regions not in OPEN or OPENING state when building the
set of interesting regions for the balancer to consider. Only
regions open (or opening) on the cluster are of interest to
balancing calculations for the current iteration. Regions in all
other states can be expected to not be of interest – either offline
(OFFLINE, or FAILED_*), not subject to balancer decisions now
(SPLITTING, SPLITTING_NEW, MERGING, MERGING_NEW), or will be
offline shortly (CLOSING) – until at least the next chore
iteration.

Add TRACE level logging.
---
 .../hbase/master/assignment/RegionStates.java | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
index 147a112152bb..0f72bdc4b456 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
@@ -571,23 +571,37 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
    * wants to iterate this exported list.  We need to synchronize on regions
    * since all access to this.servers is under a lock on this.regions.
    *
-   * @return A clone of current assignments.
+   * @return A clone of current open or opening assignments.
    */
   public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
       TableStateManager tableStateManager, List<ServerName> onlineServers) {
     final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
     for (RegionStateNode node : regionsMap.values()) {
+      // DisableTableProcedure first sets the table state to DISABLED and then force unassigns
+      // the regions in a loop. The balancer should ignore all regions for tables in DISABLED
+      // state because even if still currently open we expect them to be offlined very soon.
       if (isTableDisabled(tableStateManager, node.getTable())) {
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("Ignoring {} because table is disabled", node);
+        }
         continue;
       }
-      if (node.getRegionInfo().isSplitParent()) {
+      // When balancing, we are only interested in OPEN or OPENING regions. These can be
+      // expected to remain online until the next balancer iteration or unless the balancer
+      // decides to move it. Regions in other states are not eligible for balancing, because
+      // they are closing, splitting, merging, or otherwise already in transition.
+      if (!node.isInState(State.OPEN, State.OPENING)) {
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("Ignoring {} because region is not OPEN or OPENING", node);
+        }
         continue;
       }
       Map<ServerName, List<RegionInfo>> tableResult =
           result.computeIfAbsent(node.getTable(), t -> new HashMap<>());
       final ServerName serverName = node.getRegionLocation();
+      // A region in ONLINE or OPENING state should have a location.
       if (serverName == null) {
-        LOG.info("Skipping, no server for " + node);
+        LOG.warn("Skipping, no server for {}", node);
         continue;
       }
       List<RegionInfo> serverResult =