Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 65514186A6 for ; Fri, 6 Nov 2015 20:07:02 +0000 (UTC) Received: (qmail 91061 invoked by uid 500); 6 Nov 2015 20:07:02 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 91021 invoked by uid 500); 6 Nov 2015 20:07:02 -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 91012 invoked by uid 99); 6 Nov 2015 20:07:02 -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; Fri, 06 Nov 2015 20:07:02 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0643EE00DB; Fri, 6 Nov 2015 20:07:02 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: tedyu@apache.org To: commits@hbase.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-14632 Region server aborts due to unguarded dereference of Reader Date: Fri, 6 Nov 2015 20:07:02 +0000 (UTC) Repository: hbase Updated Branches: refs/heads/branch-1 7c9e17383 -> 0e2e5d328 HBASE-14632 Region server aborts due to unguarded dereference of Reader Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/0e2e5d32 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/0e2e5d32 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/0e2e5d32 Branch: refs/heads/branch-1 Commit: 0e2e5d328071a9f5a116a9fb0ed1df7bc1f562ab Parents: 7c9e173 Author: tedyu Authored: Fri Nov 6 12:06:58 2015 -0800 Committer: tedyu Committed: Fri Nov 6 12:06:58 2015 -0800 ---------------------------------------------------------------------- .../hadoop/hbase/regionserver/HStore.java | 43 +++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/0e2e5d32/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index 8f061a5..4495ef9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -1174,7 +1174,7 @@ public class HStore implements Store { * Since we already have this data, this will be idempotent but we will have a redundant * copy of the data. * - If RS fails between 2 and 3, the region will have a redundant copy of the data. The - * RS that failed won't be able to finish snyc() for WAL because of lease recovery in WAL. + * RS that failed won't be able to finish sync() for WAL because of lease recovery in WAL. * - If RS fails after 3, the region region server who opens the region will pick up the * the compaction marker from the WAL and replay it by removing the compaction input files. * Failed RS can also attempt to delete those files, but the operation will be idempotent @@ -1218,7 +1218,7 @@ public class HStore implements Store { + TraditionalBinaryPrefix.long2String(cr.getSize(), "", 1)); // Commence the compaction. - List newFiles = compaction.compact(throughputController, user); + List newFiles = compaction.compact(throughputController, user, region.lock.readLock()); // TODO: get rid of this! if (!this.conf.getBoolean("hbase.hstore.compaction.complete", true)) { @@ -1232,19 +1232,24 @@ public class HStore implements Store { } return sfs; } - // Do the steps necessary to complete the compaction. - sfs = moveCompatedFilesIntoPlace(cr, newFiles, user); - writeCompactionWalRecord(filesToCompact, sfs); - replaceStoreFiles(filesToCompact, sfs); - if (cr.isMajor()) { - majorCompactedCellsCount += getCompactionProgress().totalCompactingKVs; - majorCompactedCellsSize += getCompactionProgress().totalCompactedSize; - } else { - compactedCellsCount += getCompactionProgress().totalCompactingKVs; - compactedCellsSize += getCompactionProgress().totalCompactedSize; + // Do the steps necessary to complete the compaction. Hold region open for these operations. + region.lock.readLock().lock(); + try { + sfs = moveCompatedFilesIntoPlace(cr, newFiles, user); + writeCompactionWalRecord(filesToCompact, sfs); + replaceStoreFiles(filesToCompact, sfs); + if (cr.isMajor()) { + majorCompactedCellsCount += getCompactionProgress().totalCompactingKVs; + majorCompactedCellsSize += getCompactionProgress().totalCompactedSize; + } else { + compactedCellsCount += getCompactionProgress().totalCompactingKVs; + compactedCellsSize += getCompactionProgress().totalCompactedSize; + } + // At this point the store will use new files for all new scanners. + completeCompaction(filesToCompact, true); // Archive old files & update store size. + } finally { + region.lock.readLock().unlock(); } - // At this point the store will use new files for all new scanners. - completeCompaction(filesToCompact, true); // Archive old files & update store size. logCompactionEndMessage(cr, sfs, compactionStartTime); return sfs; @@ -1443,6 +1448,7 @@ public class HStore implements Store { * but instead makes a compaction candidate list by itself. * @param N Number of files. */ + @VisibleForTesting public void compactRecentForTestingAssumingDefaultPolicy(int N) throws IOException { List filesToCompact; boolean isMajor; @@ -2123,7 +2129,11 @@ public class HStore implements Store { public long getTotalStaticIndexSize() { long size = 0; for (StoreFile s : this.storeEngine.getStoreFileManager().getStorefiles()) { - size += s.getReader().getUncompressedDataIndexSize(); + StoreFile.Reader r = s.getReader(); + if (r == null) { + continue; + } + size += r.getUncompressedDataIndexSize(); } return size; } @@ -2133,6 +2143,9 @@ public class HStore implements Store { long size = 0; for (StoreFile s : this.storeEngine.getStoreFileManager().getStorefiles()) { StoreFile.Reader r = s.getReader(); + if (r == null) { + continue; + } size += r.getTotalBloomSize(); } return size;