diff --git a/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index f4625f6..c94d0e0 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -586,6 +586,8 @@ public class AssignmentManager extends ZooKeeperListener { this.regionsInTransition.notifyAll(); } } + // remove the region plan as well just in case. + this.regionPlans.remove(regionInfo.getEncodedName()); setOffline(regionInfo); } diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index dcea027..9245ef7 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -465,6 +465,8 @@ public class HRegion implements HeapSize { // , Writable{ return close(false); } + private final Object closeLock = new Object(); + /** * Close down this HRegion. Flush the cache unless abort parameter is true, * Shut down each HStore, don't service any more calls. @@ -481,59 +483,64 @@ public class HRegion implements HeapSize { // , Writable{ */ public List close(final boolean abort) throws IOException { - if (isClosed()) { - LOG.warn("Region " + this + " already closed"); - return null; - } - boolean wasFlushing = false; - synchronized (writestate) { - // Disable compacting and flushing by background threads for this - // region. - writestate.writesEnabled = false; - wasFlushing = writestate.flushing; - LOG.debug("Closing " + this + ": disabling compactions & flushes"); - while (writestate.compacting || writestate.flushing) { - LOG.debug("waiting for" + - (writestate.compacting ? " compaction" : "") + - (writestate.flushing ? - (writestate.compacting ? "," : "") + " cache flush" : - "") + " to complete for region " + this); - try { - writestate.wait(); - } catch (InterruptedException iex) { - // continue - } - } - } - // If we were not just flushing, is it worth doing a preflush...one - // that will clear out of the bulk of the memstore before we put up - // the close flag? - if (!abort && !wasFlushing && worthPreFlushing()) { - LOG.info("Running close preflush of " + this.getRegionNameAsString()); - internalFlushcache(); - } - this.closing.set(true); - lock.writeLock().lock(); - try { - if (this.isClosed()) { - // SplitTransaction handles the null + // Only allow one thread to close at a time. Serialize them so dual + // threads attempting to close will run up against each other. + + synchronized(closeLock) { + if (isClosed()) { + LOG.warn("Region " + this + " already closed"); return null; } - LOG.debug("Updates disabled for region " + this); - // Don't flush the cache if we are aborting - if (!abort) { + boolean wasFlushing = false; + synchronized (writestate) { + // Disable compacting and flushing by background threads for this + // region. + writestate.writesEnabled = false; + wasFlushing = writestate.flushing; + LOG.debug("Closing " + this + ": disabling compactions & flushes"); + while (writestate.compacting || writestate.flushing) { + LOG.debug("waiting for" + + (writestate.compacting ? " compaction" : "") + + (writestate.flushing ? + (writestate.compacting ? "," : "") + " cache flush" : + "") + " to complete for region " + this); + try { + writestate.wait(); + } catch (InterruptedException iex) { + // continue + } + } + } + // If we were not just flushing, is it worth doing a preflush...one + // that will clear out of the bulk of the memstore before we put up + // the close flag? + if (!abort && !wasFlushing && worthPreFlushing()) { + LOG.info("Running close preflush of " + this.getRegionNameAsString()); internalFlushcache(); } + this.closing.set(true); + lock.writeLock().lock(); + try { + if (this.isClosed()) { + // SplitTransaction handles the null + return null; + } + LOG.debug("Updates disabled for region " + this); + // Don't flush the cache if we are aborting + if (!abort) { + internalFlushcache(); + } - List result = new ArrayList(); - for (Store store : stores.values()) { - result.addAll(store.close()); + List result = new ArrayList(); + for (Store store : stores.values()) { + result.addAll(store.close()); + } + this.closed.set(true); + LOG.info("Closed " + this); + return result; + } finally { + lock.writeLock().unlock(); } - this.closed.set(true); - LOG.info("Closed " + this); - return result; - } finally { - lock.writeLock().unlock(); } }