ndimiduk commented on a change in pull request #1311: HBASE-23984 [Flakey 
Tests] TestMasterAbortAndRSGotKilled fails in tea…
URL: https://github.com/apache/hbase/pull/1311#discussion_r395318061
 
 

 ##########
 File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/UnassignRegionHandler.java
 ##########
 @@ -94,40 +93,42 @@ public void process() throws IOException {
       }
       return;
     }
-    HRegion region = rs.getRegion(encodedName);
-    if (region == null) {
-      LOG.debug(
-        "Received CLOSE for a region {} which is not online, and we're not 
opening/closing.",
-        encodedName);
-      rs.getRegionsInTransitionInRS().remove(encodedNameBytes, Boolean.FALSE);
-      return;
-    }
-    String regionName = region.getRegionInfo().getEncodedName();
-    LOG.info("Close {}", regionName);
-    if (region.getCoprocessorHost() != null) {
-      // XXX: The behavior is a bit broken. At master side there is no 
FAILED_CLOSE state, so if
-      // there are exception thrown from the CP, we can not report the error 
to master, and if here
-      // we just return without calling reportRegionStateTransition, the TRSP 
at master side will
-      // hang there for ever. So here if the CP throws an exception out, the 
only way is to abort
-      // the RS...
-      region.getCoprocessorHost().preClose(abort);
-    }
-    if (region.close(abort) == null) {
-      // XXX: Is this still possible? The old comment says about split, but 
now split is done at
-      // master side, so...
-      LOG.warn("Can't close region {}, was already closed during close()", 
regionName);
+    try {
+      HRegion region = rs.getRegion(encodedName);
+      if (region == null) {
+        LOG.debug(
+          "Received CLOSE for a region {} which is not online, and we're not 
opening/closing.",
+          encodedName);
+        return;
+      }
+      String regionName = region.getRegionInfo().getEncodedName();
+      LOG.info("Close {}", regionName);
+      if (region.getCoprocessorHost() != null) {
+        // XXX: The behavior is a bit broken. At master side there is no 
FAILED_CLOSE state, so if
+        // there are exception thrown from the CP, we can not report the error 
to master, and if
+        // here we just return without calling reportRegionStateTransition, 
the TRSP at master side
+        // will hang there for ever. So here if the CP throws an exception 
out, the only way is to
+        // abort the RS...
+        region.getCoprocessorHost().preClose(abort);
+      }
+      if (region.close(abort) == null) {
+        // XXX: Is this still possible? The old comment says about split, but 
now split is done at
+        // master side, so...
+        LOG.warn("Can't close region {}, was already closed during close()", 
regionName);
+        return;
+      }
+      rs.removeRegion(region, destination);
+      if (!rs.reportRegionStateTransition(
+        new RegionStateTransitionContext(TransitionCode.CLOSED, 
HConstants.NO_SEQNUM, closeProcId,
+          -1, region.getRegionInfo()))) {
+        throw new IOException("Failed to report close to master: " + 
regionName);
+      }
+      // Cache the close region procedure id after report region transition 
succeed.
+      rs.finishRegionProcedure(closeProcId);
+      LOG.info("Closed {}", regionName);
+    } finally {
       rs.getRegionsInTransitionInRS().remove(encodedNameBytes, Boolean.FALSE);
 
 Review comment:
   good.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to