HBASE-28158 resolve Test failures

Author: ukumawat

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

@@ -2007,7 +2007,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi
     // But if there are zero regions in transition, it can skip sleep to speed up.
     while (
       !interrupted && EnvironmentEdgeManager.currentTime() < nextBalanceStartTime
-        && this.assignmentManager.getScheduledRegionTransitionCount() > 0
+        && this.assignmentManager.getRegionTransitScheduledCount() > 0
     ) {
       try {
         Thread.sleep(100);
@@ -2019,7 +2019,7 @@ private void balanceThrottling(long nextBalanceStartTime, int maxRegionsInTransi
     // Throttling by max number regions in transition
     while (
       !interrupted && maxRegionsInTransition > 0
-        && this.assignmentManager.getScheduledRegionTransitionCount() >= maxRegionsInTransition
+        && this.assignmentManager.getRegionTransitScheduledCount() >= maxRegionsInTransition
         && EnvironmentEdgeManager.currentTime() <= cutoffTime
     ) {
       try {
@@ -2098,7 +2098,7 @@ public BalanceResponse balance(BalanceRequest request) throws IOException {
 
     synchronized (this.balancer) {
       // Only allow one balance run at at time.
-      if (this.assignmentManager.getScheduledRegionTransitionCount() > 0) {
+      if (this.assignmentManager.getRegionTransitScheduledCount() > 0) {
         List<RegionStateNode> regionsInTransition = assignmentManager.getRegionsInTransition();
         // if hbase:meta region is in transition, result of assignment cannot be recorded
         // ignore the force flag in that case
@@ -2113,8 +2113,8 @@ public BalanceResponse balance(BalanceRequest request) throws IOException {
 
         if (!request.isIgnoreRegionsInTransition() || metaInTransition) {
           LOG.info("Not running balancer (ignoreRIT=false" + ", metaRIT=" + metaInTransition
-            + ") because " + assignmentManager.getScheduledRegionTransitionCount() + " region(s) in transition: "
-            + toPrint + (truncated ? "(truncated list)" : ""));
+            + ") because " + assignmentManager.getRegionTransitScheduledCount()
+            + " region(s) in transition: " + toPrint + (truncated ? "(truncated list)" : ""));
           return responseBuilder.build();
         }
       }
@@ -2250,7 +2250,7 @@ public boolean normalizeRegions(final NormalizeTableFilterParams ntfp,
     if (skipRegionManagementAction("region normalizer")) {
       return false;
     }
-    if (assignmentManager.getScheduledRegionTransitionCount() > 0) {
+    if (assignmentManager.getRegionTransitScheduledCount() > 0) {
       return false;
     }
 

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

@@ -390,7 +390,7 @@ public void setupRIT(List<TransitRegionStateProcedure> procs) {
           return;
         }
       }
-      LOG.info("Attach {} to {} to restore RIT", proc, regionNode);
+      LOG.info("Attach {} to {} to restore", proc, regionNode);
       regionNode.setProcedure(proc);
     });
   }
@@ -1714,7 +1714,6 @@ protected void update(final AssignmentManager am) {
       final RegionStates regionStates = am.getRegionStates();
       this.statTimestamp = EnvironmentEdgeManager.currentTime();
       update(am.getRegionsStateInTransition(), statTimestamp);
-      update(regionStates.getRegionFailedOpen(), statTimestamp);
 
       if (LOG.isDebugEnabled() && ritsOverThreshold != null && !ritsOverThreshold.isEmpty()) {
         LOG.debug("RITs over threshold: {}",
@@ -1882,7 +1881,11 @@ public void visitRegionState(Result result, final RegionInfo regionInfo, final S
       if (regionNode.getProcedure() != null) {
         regionNode.getProcedure().stateLoaded(AssignmentManager.this, regionNode);
       }
+      // add regions to RIT while visiting the meta
       regionInTransitionTracker.handleRegionStateNodeOperation(regionNode);
+      if (master.getServerManager().isServerDead(regionNode.getRegionLocation())) {
+        regionInTransitionTracker.regionCrashed(regionNode);
+      }
     }
   };
 
@@ -2085,8 +2088,8 @@ public boolean isRegionInTransition(final RegionInfo regionInfo) {
     return regionInTransitionTracker.isRegionInTransition(regionInfo);
   }
 
-  public int getScheduledRegionTransitionCount() {
-    return regionStates.getScheduledTransitionCount();
+  public int getRegionTransitScheduledCount() {
+    return regionStates.getRegionTransitScheduledCount();
   }
 
   /**
@@ -2096,7 +2099,7 @@ public int getRegionsInTransitionCount() {
     return regionInTransitionTracker.getRegionsInTransition().size();
   }
 
-  public SortedSet<RegionState>  getRegionsStateInTransition() {
+  public SortedSet<RegionState> getRegionsStateInTransition() {
     final SortedSet<RegionState> rit = new TreeSet<RegionState>(REGION_STATE_STAMP_COMPARATOR);
     for (RegionStateNode node : getRegionsInTransition()) {
       rit.add(node.toRegionState());
@@ -2312,6 +2315,17 @@ public CompletableFuture<Void> regionClosedAbnormally(RegionStateNode regionNode
   // The above methods can only be called in TransitRegionStateProcedure(and related procedures)
   // ============================================================================================
 
+  // As soon as a server a crashed, region hosting on that are un-available, this method helps to
+  // track those un-available regions. This method can only be called from ServerCrashProcedure.
+  public void markRegionsAsCrashed(List<RegionInfo> regionsOnCrashedServer,
+    ServerName crashedServerName) {
+    for (RegionInfo regionInfo : regionsOnCrashedServer) {
+      RegionStateNode node = regionStates.getOrCreateRegionStateNode(regionInfo);
+      if (node.getRegionLocation() == crashedServerName)
+        regionInTransitionTracker.regionCrashed(node);
+    }
+  }
+
   public void markRegionAsSplit(final RegionInfo parent, final ServerName serverName,
     final RegionInfo daughterA, final RegionInfo daughterB) throws IOException {
     // Update hbase:meta. Parent will be marked offline and split up in hbase:meta.

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

@@ -50,32 +50,43 @@ public boolean isRegionInTransition(final RegionInfo regionInfo) {
     return regionInTransition.containsKey(regionInfo);
   }
 
+  public void regionCrashed(RegionStateNode regionStateNode) {
+    if (addRegionInTransition(regionStateNode)) {
+      LOG.debug("{} added to RIT list because hosting region server is crashed ",
+        regionStateNode.getRegionInfo().getEncodedName());
+    }
+  }
+
   public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
     // only consider default replica for availability
     if (regionStateNode.getRegionInfo().getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID) {
       return;
     }
 
     RegionState.State currentState = regionStateNode.getState();
-    boolean isTableEnabled = tableStateManager.isTableState(regionStateNode.getTable(), TableState.State.ENABLED,
-      TableState.State.ENABLING);
-    List<RegionState.State> terminalStates = isTableEnabled ? ENABLE_TABLE_REGION_STATE : DISABLE_TABLE_REGION_STATE;
+    boolean isTableEnabled = tableStateManager.isTableState(regionStateNode.getTable(),
+      TableState.State.ENABLED, TableState.State.ENABLING);
+    List<RegionState.State> terminalStates =
+      isTableEnabled ? 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 (removeRegionInTransition(regionStateNode.getRegionInfo())){
-        LOG.debug("Removed {} from RIT list as it is split or merged",regionStateNode.getRegionInfo().getEncodedName());
+      if (removeRegionInTransition(regionStateNode.getRegionInfo())) {
+        LOG.debug("Removed {} from RIT list as it is split or merged",
+          regionStateNode.getRegionInfo().getEncodedName());
       }
     } else if (!terminalStates.contains(currentState)) {
-      if (addRegionInTransition(regionStateNode)){
-        LOG.debug("{} added to RIT list because it is in-between state, region state : {} ", regionStateNode.getRegionInfo().getEncodedName(), currentState);
+      if (addRegionInTransition(regionStateNode)) {
+        LOG.debug("{} added to RIT list because it is in-between state, region state : {} ",
+          regionStateNode.getRegionInfo().getEncodedName(), currentState);
       }
     } else {
-      if (removeRegionInTransition(regionStateNode.getRegionInfo())){
-        LOG.debug("Removed {} from RIT list as reached to terminal state {}",regionStateNode.getRegionInfo().getEncodedName(), currentState);
+      if (removeRegionInTransition(regionStateNode.getRegionInfo())) {
+        LOG.debug("Removed {} from RIT list as reached to terminal state {}",
+          regionStateNode.getRegionInfo().getEncodedName(), currentState);
       }
     }
   }

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

@@ -601,7 +601,7 @@ public void addToOfflineRegions(final RegionStateNode regionNode) {
     regionOffline.put(regionNode.getRegionInfo(), regionNode);
   }
 
-  public int getScheduledTransitionCount() {
+  public int getRegionTransitScheduledCount() {
     return trspCounter.get();
   }
 

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

@@ -252,7 +252,8 @@ protected static void waitRegionInTransition(final MasterProcedureEnv env,
         new ProcedureSyncWait.Predicate<Boolean>() {
           @Override
           public Boolean evaluate() throws IOException {
-            return !assignmentManager.getRegionStates().getRegionStateNode(region).isTransitionScheduled();
+            return !assignmentManager.getRegionStates().getRegionStateNode(region)
+              .isTransitionScheduled();
           }
         });
     }

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

@@ -201,6 +201,7 @@ protected Flow executeFromState(MasterProcedureEnv env, ServerCrashState state)
           break;
         case SERVER_CRASH_GET_REGIONS:
           this.regionsOnCrashedServer = getRegionsOnCrashedServer(env);
+          env.getAssignmentManager().markRegionsAsCrashed(regionsOnCrashedServer, this.serverName);
           // Where to go next? Depends on whether we should split logs at all or
           // if we should do distributed log splitting.
           if (regionsOnCrashedServer != null) {

hbase-server/src/main/resources/hbase-webapps/master/assignmentManagerStatus.jsp

@@ -32,7 +32,7 @@
   AssignmentManager assignmentManager = master.getAssignmentManager();
   int limit = 100;
 
-  SortedSet<RegionState> rit = assignmentManager.getRegionsInTransitionOrderedByTimestamp();
+  SortedSet<RegionState> rit = assignmentManager.getRegionsStateInTransition();
 
 if (!rit.isEmpty()) {
   long currentTime = System.currentTimeMillis();

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

@@ -3448,6 +3448,30 @@ public boolean evaluate() throws IOException {
     };
   }
 
+  /**
+   * Returns a {@link Predicate} for checking that there are no procedure to region transition in
+   * master
+   */
+  public ExplainingPredicate<IOException> predicateNoRegionTransitScheduled() {
+    return new ExplainingPredicate<IOException>() {
+      @Override
+      public String explainFailure() throws IOException {
+        final AssignmentManager am = getMiniHBaseCluster().getMaster().getAssignmentManager();
+        return "Number of procedure scheduled for region transit: "
+          + am.getRegionTransitScheduledCount();
+      }
+
+      @Override
+      public boolean evaluate() throws IOException {
+        HMaster master = getMiniHBaseCluster().getMaster();
+        if (master == null) return false;
+        AssignmentManager am = master.getAssignmentManager();
+        if (am == null) return false;
+        return am.getRegionTransitScheduledCount() == 0;
+      }
+    };
+  }
+
   /**
    * Returns a {@link Predicate} for checking that table is enabled
    */
@@ -3525,13 +3549,28 @@ public void waitUntilNoRegionsInTransition(final long timeout) throws IOExceptio
     waitFor(timeout, predicateNoRegionsInTransition());
   }
 
+  /**
+   * Wait until no regions in transition.
+   * @param timeout How long to wait.
+   */
+  public void waitUntilNoRegionTransitScheduled(final long timeout) throws IOException {
+    waitFor(timeout, predicateNoRegionTransitScheduled());
+  }
+
   /**
    * Wait until no regions in transition. (time limit 15min)
    */
   public void waitUntilNoRegionsInTransition() throws IOException {
     waitUntilNoRegionsInTransition(15 * 60000);
   }
 
+  /**
+   * Wait until no TRSP is present
+   */
+  public void waitUntilNoRegionTransitScheduled() throws IOException {
+    waitUntilNoRegionTransitScheduled(15 * 60000);
+  }
+
   /**
    * Wait until labels is ready in VisibilityLabelsCache.
    */

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

@@ -154,7 +154,7 @@ public void testRITAssignmentManagerMetrics() throws Exception {
       // Sleep 5 seconds, wait for doMetrics chore catching up
       // the rit count consists of rit and failed opens. see RegionInTransitionStat#update
       // Waiting for the completion of rit makes the assert stable.
-      TEST_UTIL.waitUntilNoRegionsInTransition();
+      TEST_UTIL.waitUntilNoRegionTransitScheduled();
       Thread.sleep(MSG_INTERVAL * 5);
       METRICS_HELPER.assertGauge(MetricsAssignmentManagerSource.RIT_COUNT_NAME, 1, amSource);
       METRICS_HELPER.assertGauge(MetricsAssignmentManagerSource.RIT_COUNT_OVER_THRESHOLD_NAME, 1,

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

@@ -93,6 +93,7 @@ public void teardown() throws Exception {
   }
 
   @Test
+  // umesh testing
   public void testTakeZkCoordinatedSnapshot() {
     for (int i = 0; i < 10; i++) {
       try {