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 <[email protected]> Authored: Fri Jun 17 18:00:16 2016 +0800 Committer: Yu Li <[email protected]> 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; + } } /**
