Repository: hbase Updated Branches: refs/heads/branch-1 f0385b4b8 -> f9490aaf4
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/f9490aaf Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/f9490aaf Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/f9490aaf Branch: refs/heads/branch-1 Commit: f9490aaf43e1df0caead47cb03127cd61206ba00 Parents: f0385b4 Author: tedyu <yuzhih...@gmail.com> Authored: Wed Aug 24 10:57:14 2016 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Wed Aug 24 10:57:14 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/f9490aaf/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 02428bd..bfb9171 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 @@ -253,6 +253,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 @@ -5037,6 +5040,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 @@ -5069,8 +5073,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); } } @@ -5093,6 +5096,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) { @@ -7365,6 +7381,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; @@ -7523,6 +7561,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 { @@ -7746,6 +7788,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(); @@ -7929,7 +7975,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);
