hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From te...@apache.org
Subject hbase git commit: HBASE-16304 HRegion#RegionScannerImpl#handleFileNotFoundException may lead to deadlock when trying to obtain write lock on updatesLock
Date Wed, 24 Aug 2016 16:04:53 GMT
Repository: hbase
Updated Branches:
  refs/heads/master dda8f67b2 -> bf7015d32


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/bf7015d3
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/bf7015d3
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/bf7015d3

Branch: refs/heads/master
Commit: bf7015d3204818fdc88ef505e0a06cac4ea2774b
Parents: dda8f67
Author: tedyu <yuzhihong@gmail.com>
Authored: Wed Aug 24 09:04:47 2016 -0700
Committer: tedyu <yuzhihong@gmail.com>
Committed: Wed Aug 24 09:04:47 2016 -0700

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/HRegion.java      | 48 ++++++++++++++++++--
 1 file changed, 45 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/bf7015d3/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 86c02ea..f97f6b2 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
@@ -238,6 +238,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
@@ -4970,6 +4973,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
@@ -5002,8 +5006,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);
           }
         }
 
@@ -5026,6 +5029,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) {
@@ -7141,6 +7157,28 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver,
Regi
     return increment(increment, HConstants.NO_NONCE, HConstants.NO_NONCE);
   }
 
+  // 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 increment(Increment mutation, long nonceGroup, long nonce)
   throws IOException {
@@ -7206,6 +7244,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver,
Regi
         writeEntry = null;
       } 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();
       }
       // If results is null, then client asked that we not return the calculated results.
       return results != null && returnResults? Result.create(results): null;
@@ -7546,7 +7588,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver,
Regi
   public static final long FIXED_OVERHEAD = ClassSize.align(
       ClassSize.OBJECT +
       ClassSize.ARRAY +
-      48 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
+      49 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
       (14 * Bytes.SIZEOF_LONG) +
       5 * Bytes.SIZEOF_BOOLEAN);
 


Mime
View raw message