Repository: hbase Updated Branches: refs/heads/branch-1.3 25995a2bf -> 70daa23ea
HBASE-16304 HRegion#RegionScannerImpl#handleFileNotFoundException may lead to deadlock when trying to obtain write lock on updatesLock Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/70daa23e Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/70daa23e Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/70daa23e Branch: refs/heads/branch-1.3 Commit: 70daa23ea79b21b9ae337bcf962ada26ac95c1a2 Parents: 25995a2 Author: tedyu <yuzhih...@gmail.com> Authored: Wed Aug 24 11:28:24 2016 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Wed Aug 24 11:28:24 2016 -0700 ---------------------------------------------------------------------- .../hadoop/hbase/regionserver/HRegion.java | 52 ++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/70daa23e/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index b56c887..d43e838 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -252,6 +252,9 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi */ protected volatile long lastReplayedOpenRegionSeqId = -1L; protected volatile long lastReplayedCompactionSeqId = -1L; + + // collects Map(s) of Store to sequence Id when handleFileNotFound() is involved + protected List<Map> storeSeqIds = new ArrayList<>(); ////////////////////////////////////////////////////////////////////////////// // Members @@ -5036,6 +5039,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi startRegionOperation(); // obtain region close lock try { + Map<Store, Long> map = new HashMap<Store, Long>(); synchronized (writestate) { for (Store store : getStores()) { // TODO: some stores might see new data from flush, while others do not which @@ -5068,8 +5072,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } } - // Drop the memstore contents if they are now smaller than the latest seen flushed file - totalFreedSize += dropMemstoreContentsForSeqId(storeSeqId, store); + map.put(store, storeSeqId); } } @@ -5092,6 +5095,19 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi this.lastReplayedOpenRegionSeqId = smallestSeqIdInStores; } } + if (!map.isEmpty()) { + if (!force) { + for (Map.Entry<Store, Long> entry : map.entrySet()) { + // Drop the memstore contents if they are now smaller than the latest seen flushed file + totalFreedSize += dropMemstoreContentsForSeqId(entry.getValue(), entry.getKey()); + } + } else { + synchronized (storeSeqIds) { + // don't try to acquire write lock of updatesLock now + storeSeqIds.add(map); + } + } + } // C. Finally notify anyone waiting on memstore to clear: // e.g. checkResources(). synchronized (this) { @@ -7364,6 +7380,28 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // We should refactor append and increment as local get-mutate-put // transactions, so all stores only go through one code path for puts. + // dropMemstoreContentsForSeqId() would acquire write lock of updatesLock + // We perform this operation outside of the read lock of updatesLock to avoid dead lock + // See HBASE-16304 + @SuppressWarnings("unchecked") + private void dropMemstoreContents() throws IOException { + long totalFreedSize = 0; + while (!storeSeqIds.isEmpty()) { + Map<Store, Long> map = null; + synchronized (storeSeqIds) { + if (storeSeqIds.isEmpty()) break; + map = storeSeqIds.remove(storeSeqIds.size()-1); + } + for (Map.Entry<Store, Long> entry : map.entrySet()) { + // Drop the memstore contents if they are now smaller than the latest seen flushed file + totalFreedSize += dropMemstoreContentsForSeqId(entry.getValue(), entry.getKey()); + } + } + if (totalFreedSize > 0) { + LOG.debug("Freed " + totalFreedSize + " bytes from memstore"); + } + } + @Override public Result append(Append mutate, long nonceGroup, long nonce) throws IOException { Operation op = Operation.APPEND; @@ -7522,6 +7560,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } } finally { this.updatesLock.readLock().unlock(); + // For increment/append, a region scanner for doing a get operation could throw + // FileNotFoundException. So we call dropMemstoreContents() in finally block + // after releasing read lock + dropMemstoreContents(); } } finally { @@ -7745,6 +7787,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi } } finally { this.updatesLock.readLock().unlock(); + // For increment/append, a region scanner for doing a get operation could throw + // FileNotFoundException. So we call dropMemstoreContents() in finally block + // after releasing read lock + dropMemstoreContents(); } } finally { rowLock.release(); @@ -7928,7 +7974,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi public static final long FIXED_OVERHEAD = ClassSize.align( ClassSize.OBJECT + ClassSize.ARRAY + - 45 * ClassSize.REFERENCE + 3 * Bytes.SIZEOF_INT + + 46 * ClassSize.REFERENCE + 3 * Bytes.SIZEOF_INT + (14 * Bytes.SIZEOF_LONG) + 5 * Bytes.SIZEOF_BOOLEAN);
