hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From l...@apache.org
Subject hbase git commit: HBASE-16032 Possible memory leak in StoreScanner
Date Tue, 21 Jun 2016 18:25:59 GMT
Repository: hbase
Updated Branches:
  refs/heads/0.98 a35162aff -> 55810a2e0


HBASE-16032 Possible memory leak in StoreScanner


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/55810a2e
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/55810a2e
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/55810a2e

Branch: refs/heads/0.98
Commit: 55810a2e0c56ab6bfc0590e66e064970782d92ee
Parents: a35162a
Author: Yu Li <liyu@apache.org>
Authored: Fri Jun 17 18:00:16 2016 +0800
Committer: Yu Li <liyu@apache.org>
Committed: Wed Jun 22 02:25:32 2016 +0800

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/HRegion.java      | 44 +++++++++++++-------
 .../hadoop/hbase/regionserver/StoreScanner.java | 43 +++++++++++--------
 2 files changed, 53 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/55810a2e/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 90e9296..d9214fa 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
@@ -4078,28 +4078,40 @@ public class HRegion implements HeapSize { // , Writable{
       List<KeyValueScanner> scanners = new ArrayList<KeyValueScanner>(scan.getFamilyMap().size());
       List<KeyValueScanner> joinedScanners =
           new ArrayList<KeyValueScanner>(scan.getFamilyMap().size());
-      if (additionalScanners != null) {
+      // Store all already instantiated scanners for exception handling
+      List<KeyValueScanner> instantiatedScanners = new ArrayList<KeyValueScanner>();
+      // handle additionalScanners
+      if (additionalScanners != null && !additionalScanners.isEmpty()) {
         scanners.addAll(additionalScanners);
+        instantiatedScanners.addAll(additionalScanners);
       }
 
-      for (Map.Entry<byte[], NavigableSet<byte[]>> entry :
-          scan.getFamilyMap().entrySet()) {
-        Store store = stores.get(entry.getKey());
-        KeyValueScanner scanner;
-        try {
-          scanner = store.getScanner(scan, entry.getValue(), this.readPt);
-        } catch (FileNotFoundException e) {
-          abortRegionServer(e.getMessage());
-          throw new NotServingRegionException(region.getRegionNameAsString() + " is closing");
+      try {
+        for (Map.Entry<byte[], NavigableSet<byte[]>> entry : scan.getFamilyMap().entrySet())
{
+          Store store = stores.get(entry.getKey());
+          KeyValueScanner scanner;
+          try {
+            scanner = store.getScanner(scan, entry.getValue(), this.readPt);
+          } catch (FileNotFoundException e) {
+            abortRegionServer(e.getMessage());
+            throw new NotServingRegionException(region.getRegionNameAsString() + " is closing");
+          }
+          instantiatedScanners.add(scanner);
+          if (this.filter == null || !scan.doLoadColumnFamiliesOnDemand()
+              || this.filter.isFamilyEssential(entry.getKey())) {
+            scanners.add(scanner);
+          } else {
+            joinedScanners.add(scanner);
+          }
         }
-        if (this.filter == null || !scan.doLoadColumnFamiliesOnDemand()
-          || this.filter.isFamilyEssential(entry.getKey())) {
-          scanners.add(scanner);
-        } else {
-          joinedScanners.add(scanner);
+        initializeKVHeap(scanners, joinedScanners, region);
+      } catch (IOException e) {
+        // close all already instantiated scanners before throwing the exception
+        for (KeyValueScanner scanner : instantiatedScanners) {
+          scanner.close();
         }
+        throw e;
       }
-      initializeKVHeap(scanners, joinedScanners, region);
     }
 
     RegionScannerImpl(Scan scan, HRegion region) throws IOException {

http://git-wip-us.apache.org/repos/asf/hbase/blob/55810a2e/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
index fc034ed..2dd0625 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
@@ -178,24 +178,31 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
 
     this.store.addChangedReaderObserver(this);
 
-    // Pass columns to try to filter out unnecessary StoreFiles.
-    List<KeyValueScanner> scanners = getScannersNoCompaction();
-
-    // Seek all scanners to the start of the Row (or if the exact matching row
-    // key does not exist, then to the start of the next matching Row).
-    // Always check bloom filter to optimize the top row seek for delete
-    // family marker.
-    seekScanners(scanners, matcher.getStartKey(), explicitColumnQuery
-        && lazySeekEnabledGlobally, isParallelSeekEnabled);
-
-    // set storeLimit
-    this.storeLimit = scan.getMaxResultsPerColumnFamily();
-
-    // set rowOffset
-    this.storeOffset = scan.getRowOffsetPerColumnFamily();
-
-    // Combine all seeked scanners with a heap
-    resetKVHeap(scanners, store.getComparator());
+    try {
+      // Pass columns to try to filter out unnecessary StoreFiles.
+      List<KeyValueScanner> scanners = getScannersNoCompaction();
+
+      // Seek all scanners to the start of the Row (or if the exact matching row
+      // key does not exist, then to the start of the next matching Row).
+      // Always check bloom filter to optimize the top row seek for delete
+      // family marker.
+      seekScanners(scanners, matcher.getStartKey(), explicitColumnQuery && lazySeekEnabledGlobally,
+        isParallelSeekEnabled);
+
+      // set storeLimit
+      this.storeLimit = scan.getMaxResultsPerColumnFamily();
+
+      // set rowOffset
+      this.storeOffset = scan.getRowOffsetPerColumnFamily();
+
+      // Combine all seeked scanners with a heap
+      resetKVHeap(scanners, store.getComparator());
+    } catch (IOException e) {
+      // remove us from the HStore#changedReaderObservers here or we'll have no chance to
+      // and might cause memory leak
+      this.store.deleteChangedReaderObserver(this);
+      throw e;
+    }
   }
 
   /**


Mime
View raw message