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

 ##########
 File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java
 ##########
 @@ -52,85 +54,101 @@
   // have a running queue of user regions to close?
   private static final Logger LOG = 
LoggerFactory.getLogger(CloseRegionHandler.class);
 
-  private final RegionServerServices rsServices;
+  private final RegionServerServices rss;
   private final RegionInfo regionInfo;
 
   // If true, the hosting server is aborting.  Region close process is 
different
   // when we are aborting.
   private final boolean abort;
-  private ServerName destination;
+
+  /**
+   * The target the Region is going to after successful close; usually null as 
it depends on
+   * context whether this is passed in or not.
+   */
+  private final ServerName destination;
 
   /**
    * This method used internally by the RegionServer to close out regions.
-   * @param server
-   * @param rsServices
-   * @param regionInfo
+   *
    * @param abort If the regionserver is aborting.
-   * @param destination
    */
-  public CloseRegionHandler(final Server server,
-      final RegionServerServices rsServices,
-      final RegionInfo regionInfo, final boolean abort,
-      ServerName destination) {
-    this(server, rsServices, regionInfo, abort,
-      EventType.M_RS_CLOSE_REGION, destination);
+  public CloseRegionHandler(final RegionServerServices rsServices, final 
RegionInfo regionInfo,
+    final boolean abort, @Nullable ServerName destination) {
+    this(rsServices, regionInfo, abort, EventType.M_RS_CLOSE_REGION, 
destination);
   }
 
-  protected CloseRegionHandler(final Server server,
-      final RegionServerServices rsServices, RegionInfo regionInfo,
-      boolean abort, EventType eventType, ServerName destination) {
-    super(server, eventType);
-    this.server = server;
-    this.rsServices = rsServices;
+  protected CloseRegionHandler(RegionServerServices rsServices, RegionInfo 
regionInfo,
+      boolean abort, EventType eventType, @Nullable ServerName destination) {
+    super(rsServices, eventType);
     this.regionInfo = regionInfo;
     this.abort = abort;
+    this.rss = rsServices;
     this.destination = destination;
   }
 
-  public RegionInfo getRegionInfo() {
-    return regionInfo;
-  }
-
-  @Override
-  public void process() {
+  @Override public void process() throws IOException {
     try {
-      String name = regionInfo.getEncodedName();
-      LOG.trace("Processing close of {}", name);
-      String encodedRegionName = regionInfo.getEncodedName();
       // Check that this region is being served here
-      HRegion region = (HRegion)rsServices.getRegion(encodedRegionName);
+      HRegion region = 
(HRegion)this.rss.getRegion(this.regionInfo.getEncodedName());
       if (region == null) {
-        LOG.warn("Received CLOSE for region {} but currently not serving - 
ignoring", name);
+        LOG.warn("Received CLOSE for {} but not ONLINE; ignoring CLOSE 
request",
+          this.regionInfo.getRegionNameAsString());
         // TODO: do better than a simple warning
         return;
       }
-
-      // Close the region
-      try {
-        if (region.close(abort) == null) {
-          // This region got closed.  Most likely due to a split.
-          // The split message will clean up the master state.
-          LOG.warn("Can't close region {}, was already closed during close()", 
name);
-          return;
-        }
-      } catch (IOException ioe) {
-        // An IOException here indicates that we couldn't successfully flush 
the
-        // memstore before closing. So, we need to abort the server and allow
-        // the master to split our logs in order to recover the data.
-        server.abort("Unrecoverable exception while closing region " +
-          regionInfo.getRegionNameAsString() + ", still finishing close", ioe);
-        throw new RuntimeException(ioe);
+      if (region.getCoprocessorHost() != null) {
+        // XXX: The behavior is a bit broken. At master side there is no 
FAILED_CLOSE state, so if
+        // there are exceptions thrown from the CP, we can not report the 
error to the master, and
 
 Review comment:
   😱 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to