Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 013C9200B17 for ; Tue, 21 Jun 2016 14:08:05 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id F3CE5160A36; Tue, 21 Jun 2016 12:08:04 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 21E2E160A4F for ; Tue, 21 Jun 2016 14:08:03 +0200 (CEST) Received: (qmail 48572 invoked by uid 500); 21 Jun 2016 12:08:03 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 48561 invoked by uid 99); 21 Jun 2016 12:08:03 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 21 Jun 2016 12:08:03 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 23D32E020A; Tue, 21 Jun 2016 12:08:03 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: liyu@apache.org To: commits@hbase.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-16032 Possible memory leak in StoreScanner Date: Tue, 21 Jun 2016 12:08:03 +0000 (UTC) archived-at: Tue, 21 Jun 2016 12:08:05 -0000 Repository: hbase Updated Branches: refs/heads/branch-1 f06945ae6 -> c6b8c9bb0 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/c6b8c9bb Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/c6b8c9bb Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/c6b8c9bb Branch: refs/heads/branch-1 Commit: c6b8c9bb02da8fb0a4b6ae4d610b2c11c4ce1655 Parents: f06945a Author: Yu Li Authored: Tue Jun 21 20:06:33 2016 +0800 Committer: Yu Li Committed: Tue Jun 21 20:07:52 2016 +0800 ---------------------------------------------------------------------- .../hadoop/hbase/regionserver/HRegion.java | 41 ++++++++++++------- .../hadoop/hbase/regionserver/StoreScanner.java | 43 ++++++++++++-------- 2 files changed, 52 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/c6b8c9bb/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 4bdd825..a661d24 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 @@ -5688,26 +5688,39 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi List scanners = new ArrayList(scan.getFamilyMap().size()); List joinedScanners = new ArrayList(scan.getFamilyMap().size()); - if (additionalScanners != null) { + // Store all already instantiated scanners for exception handling + List instantiatedScanners = new ArrayList(); + // handle additionalScanners + if (additionalScanners != null && !additionalScanners.isEmpty()) { scanners.addAll(additionalScanners); + instantiatedScanners.addAll(additionalScanners); } - for (Map.Entry> entry : scan.getFamilyMap().entrySet()) { - Store store = stores.get(entry.getKey()); - KeyValueScanner scanner; - try { - scanner = store.getScanner(scan, entry.getValue(), this.readPt); - } catch (FileNotFoundException e) { - throw handleFileNotFound(e); + try { + for (Map.Entry> entry : scan.getFamilyMap().entrySet()) { + Store store = stores.get(entry.getKey()); + KeyValueScanner scanner; + try { + scanner = store.getScanner(scan, entry.getValue(), this.readPt); + } catch (FileNotFoundException e) { + throw handleFileNotFound(e); + } + 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); } protected void initializeKVHeap(List scanners, http://git-wip-us.apache.org/repos/asf/hbase/blob/c6b8c9bb/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 d8882a0..beb0758 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 @@ -198,24 +198,31 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner this.store.addChangedReaderObserver(this); - // Pass columns to try to filter out unnecessary StoreFiles. - List 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, parallelSeekEnabled); - - // set storeLimit - this.storeLimit = scan.getMaxResultsPerColumnFamily(); - - // set rowOffset - this.storeOffset = scan.getRowOffsetPerColumnFamily(); - addCurrentScanners(scanners); - // Combine all seeked scanners with a heap - resetKVHeap(scanners, store.getComparator()); + try { + // Pass columns to try to filter out unnecessary StoreFiles. + List 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, + parallelSeekEnabled); + + // set storeLimit + this.storeLimit = scan.getMaxResultsPerColumnFamily(); + + // set rowOffset + this.storeOffset = scan.getRowOffsetPerColumnFamily(); + addCurrentScanners(scanners); + // 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; + } } /**