saintstack 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_r394709869
########## 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: Thanks Nick ---------------------------------------------------------------- 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