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 B816118C00 for ; Mon, 4 Jan 2016 16:56:51 +0000 (UTC) Received: (qmail 74639 invoked by uid 500); 4 Jan 2016 16:56:48 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 74519 invoked by uid 500); 4 Jan 2016 16:56:48 -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 73306 invoked by uid 99); 4 Jan 2016 16:56:47 -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; Mon, 04 Jan 2016 16:56:47 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 32923DFFB9; Mon, 4 Jan 2016 16:56:47 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: syuanjiang@apache.org To: commits@hbase.apache.org Date: Mon, 04 Jan 2016 16:57:14 -0000 Message-Id: In-Reply-To: <51767155c1144ae68c5c50a871d63d25@git.apache.org> References: <51767155c1144ae68c5c50a871d63d25@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [29/29] hbase git commit: HBASE-14987 Compaction marker whose region name doesn't match current region's needs to be handled HBASE-14987 Compaction marker whose region name doesn't match current region's needs to be handled Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/00656688 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/00656688 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/00656688 Branch: refs/heads/hbase-12439 Commit: 00656688f73c85ea9e6f5241ac852f72e774eeea Parents: 9589a7d Author: tedyu Authored: Mon Jan 4 07:10:10 2016 -0800 Committer: tedyu Committed: Mon Jan 4 07:10:10 2016 -0800 ---------------------------------------------------------------------- .../hadoop/hbase/protobuf/ProtobufUtil.java | 9 +++- .../hadoop/hbase/regionserver/HRegion.java | 44 ++++++++++++++++---- .../hadoop/hbase/regionserver/TestHRegion.java | 26 ++++++++++-- 3 files changed, 65 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/00656688/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java index b2d5994..c02309b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java @@ -2563,12 +2563,19 @@ public final class ProtobufUtil { @SuppressWarnings("deprecation") public static CompactionDescriptor toCompactionDescriptor(HRegionInfo info, byte[] family, List inputPaths, List outputPaths, Path storeDir) { + return toCompactionDescriptor(info, null, family, inputPaths, outputPaths, storeDir); + } + + @SuppressWarnings("deprecation") + public static CompactionDescriptor toCompactionDescriptor(HRegionInfo info, byte[] regionName, + byte[] family, List inputPaths, List outputPaths, Path storeDir) { // compaction descriptor contains relative paths. // input / output paths are relative to the store dir // store dir is relative to region dir CompactionDescriptor.Builder builder = CompactionDescriptor.newBuilder() .setTableName(ByteStringer.wrap(info.getTable().toBytes())) - .setEncodedRegionName(ByteStringer.wrap(info.getEncodedNameAsBytes())) + .setEncodedRegionName(ByteStringer.wrap( + regionName == null ? info.getEncodedNameAsBytes() : regionName)) .setFamilyName(ByteStringer.wrap(family)) .setStoreHomeDir(storeDir.getName()); //make relative for (Path inputPath : inputPaths) { http://git-wip-us.apache.org/repos/asf/hbase/blob/00656688/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 9549a13..ccf2eb0 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 @@ -4130,11 +4130,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi continue; } } + boolean checkRowWithinBoundary = false; // Check this edit is for this region. if (!Bytes.equals(key.getEncodedRegionName(), this.getRegionInfo().getEncodedNameAsBytes())) { - skippedEdits++; - continue; + checkRowWithinBoundary = true; } boolean flush = false; @@ -4142,11 +4142,14 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // Check this edit is for me. Also, guard against writing the special // METACOLUMN info such as HBASE::CACHEFLUSH entries if (CellUtil.matchingFamily(cell, WALEdit.METAFAMILY)) { - //this is a special edit, we should handle it - CompactionDescriptor compaction = WALEdit.getCompaction(cell); - if (compaction != null) { - //replay the compaction - replayWALCompactionMarker(compaction, false, true, Long.MAX_VALUE); + // if region names don't match, skipp replaying compaction marker + if (!checkRowWithinBoundary) { + //this is a special edit, we should handle it + CompactionDescriptor compaction = WALEdit.getCompaction(cell); + if (compaction != null) { + //replay the compaction + replayWALCompactionMarker(compaction, false, true, Long.MAX_VALUE); + } } skippedEdits++; continue; @@ -4162,6 +4165,12 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi skippedEdits++; continue; } + if (checkRowWithinBoundary && !rowIsInRange(this.getRegionInfo(), + cell.getRowArray(), cell.getRowOffset(), cell.getRowLength())) { + LOG.warn("Row of " + cell + " is not within region boundary"); + skippedEdits++; + continue; + } // Now, figure if we should skip this edit. if (key.getLogSeqNum() <= maxSeqIdInStores.get(store.getFamily() .getName())) { @@ -4232,8 +4241,16 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi void replayWALCompactionMarker(CompactionDescriptor compaction, boolean pickCompactionFiles, boolean removeFiles, long replaySeqId) throws IOException { - checkTargetRegion(compaction.getEncodedRegionName().toByteArray(), - "Compaction marker from WAL ", compaction); + try { + checkTargetRegion(compaction.getEncodedRegionName().toByteArray(), + "Compaction marker from WAL ", compaction); + } catch (WrongRegionException wre) { + if (RegionReplicaUtil.isDefaultReplica(this.getRegionInfo())) { + // skip the compaction marker since it is not for this region + return; + } + throw wre; + } synchronized (writestate) { if (replaySeqId < lastReplayedOpenRegionSeqId) { @@ -6590,6 +6607,15 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi (Bytes.compareTo(info.getEndKey(), row) > 0)); } + public static boolean rowIsInRange(HRegionInfo info, final byte [] row, final int offset, + final short length) { + return ((info.getStartKey().length == 0) || + (Bytes.compareTo(info.getStartKey(), 0, info.getStartKey().length, + row, offset, length) <= 0)) && + ((info.getEndKey().length == 0) || + (Bytes.compareTo(info.getEndKey(), 0, info.getEndKey().length, row, offset, length) > 0)); + } + /** * Merge two HRegions. The regions must be adjacent and must not overlap. * http://git-wip-us.apache.org/repos/asf/hbase/blob/00656688/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 35de488..4582e31 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -839,6 +839,10 @@ public class TestHRegion { @Test public void testRecoveredEditsReplayCompaction() throws Exception { + testRecoveredEditsReplayCompaction(false); + testRecoveredEditsReplayCompaction(true); + } + public void testRecoveredEditsReplayCompaction(boolean mismatchedRegionName) throws Exception { String method = name.getMethodName(); TableName tableName = TableName.valueOf(method); byte[] family = Bytes.toBytes("family"); @@ -884,9 +888,17 @@ public class TestHRegion { Path newFile = region.getRegionFileSystem().commitStoreFile(Bytes.toString(family), files[0].getPath()); + byte[] encodedNameAsBytes = this.region.getRegionInfo().getEncodedNameAsBytes(); + byte[] fakeEncodedNameAsBytes = new byte [encodedNameAsBytes.length]; + for (int i=0; i < encodedNameAsBytes.length; i++) { + // Mix the byte array to have a new encodedName + fakeEncodedNameAsBytes[i] = (byte) (encodedNameAsBytes[i] + 1); + } + CompactionDescriptor compactionDescriptor = ProtobufUtil.toCompactionDescriptor(this.region - .getRegionInfo(), family, storeFiles, Lists.newArrayList(newFile), region - .getRegionFileSystem().getStoreDir(Bytes.toString(family))); + .getRegionInfo(), mismatchedRegionName ? fakeEncodedNameAsBytes : null, family, + storeFiles, Lists.newArrayList(newFile), + region.getRegionFileSystem().getStoreDir(Bytes.toString(family))); WALUtil.writeCompactionMarker(region.getWAL(), this.region.getTableDesc(), this.region.getRegionInfo(), compactionDescriptor, region.getMVCC()); @@ -908,14 +920,20 @@ public class TestHRegion { region.getTableDesc(); region.getRegionInfo(); region.close(); - region = HRegion.openHRegion(region, null); + try { + region = HRegion.openHRegion(region, null); + } catch (WrongRegionException wre) { + fail("Matching encoded region name should not have produced WrongRegionException"); + } // now check whether we have only one store file, the compacted one Collection sfs = region.getStore(family).getStorefiles(); for (StoreFile sf : sfs) { LOG.info(sf.getPath()); } - assertEquals(1, region.getStore(family).getStorefilesCount()); + if (!mismatchedRegionName) { + assertEquals(1, region.getStore(family).getStorefilesCount()); + } files = FSUtils.listStatus(fs, tmpDir); assertTrue("Expected to find 0 files inside " + tmpDir, files == null || files.length == 0);