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 1D91B200498 for ; Tue, 29 Aug 2017 19:12:57 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 1BA7416728B; Tue, 29 Aug 2017 17:12:57 +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 24DD516728A for ; Tue, 29 Aug 2017 19:12:55 +0200 (CEST) Received: (qmail 62471 invoked by uid 500); 29 Aug 2017 17:12:55 -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 62462 invoked by uid 99); 29 Aug 2017 17:12:55 -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, 29 Aug 2017 17:12:55 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 1B7C9F32F1; Tue, 29 Aug 2017 17:12:55 +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-18665 ReversedScannerCallable invokes getRegionLocations incorrectly Date: Tue, 29 Aug 2017 17:12:55 +0000 (UTC) archived-at: Tue, 29 Aug 2017 17:12:57 -0000 Repository: hbase Updated Branches: refs/heads/branch-1.3 a5be59bcc -> fe61a7373 HBASE-18665 ReversedScannerCallable invokes getRegionLocations incorrectly The way how ReversedScannerCallable#prepare called getRegionLocations was faulty. Calling prepare with force reload used cache and vica versa. Signed-off-by: tedyu Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/fe61a737 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/fe61a737 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/fe61a737 Branch: refs/heads/branch-1.3 Commit: fe61a7373eeb35f4cafe420806210ddf8738b4f2 Parents: a5be59b Author: Peter Somogyi Authored: Tue Aug 29 17:36:32 2017 +0200 Committer: tedyu Committed: Tue Aug 29 10:11:04 2017 -0700 ---------------------------------------------------------------------- .../hbase/client/ReversedScannerCallable.java | 8 +- .../RpcRetryingCallerWithReadReplicas.java | 6 +- .../client/TestReversedScannerCallable.java | 94 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/fe61a737/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java index e169f7a..8bd5865 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ReversedScannerCallable.java @@ -102,8 +102,8 @@ public class ReversedScannerCallable extends ScannerCallable { if (!instantiated || reload) { if (locateStartRow == null) { // Just locate the region with the row - RegionLocations rl = RpcRetryingCallerWithReadReplicas.getRegionLocations(reload, id, - getConnection(), tableName, row); + RegionLocations rl = RpcRetryingCallerWithReadReplicas.getRegionLocations(!reload, id, + getConnection(), getTableName(), getRow()); this.location = id < rl.size() ? rl.getRegionLocation(id) : null; if (this.location == null) { throw new IOException("Failed to find location, tableName=" @@ -162,8 +162,8 @@ public class ReversedScannerCallable extends ScannerCallable { List regionList = new ArrayList(); byte[] currentKey = startKey; do { - RegionLocations rl = RpcRetryingCallerWithReadReplicas.getRegionLocations(reload, id, - getConnection(), tableName, currentKey); + RegionLocations rl = RpcRetryingCallerWithReadReplicas.getRegionLocations(!reload, id, + getConnection(), getTableName(), currentKey); HRegionLocation regionLocation = id < rl.size() ? rl.getRegionLocation(id) : null; if (regionLocation != null && regionLocation.getRegionInfo().containsRow(currentKey)) { regionList.add(regionLocation); http://git-wip-us.apache.org/repos/asf/hbase/blob/fe61a737/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java index 80c187c..688a37c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCallerWithReadReplicas.java @@ -299,10 +299,10 @@ public class RpcRetryingCallerWithReadReplicas { RegionLocations rl; try { - if (!useCache) { - rl = cConnection.relocateRegion(tableName, row, replicaId); + if (useCache) { + rl = cConnection.locateRegion(tableName, row, true, true, replicaId); } else { - rl = cConnection.locateRegion(tableName, row, useCache, true, replicaId); + rl = cConnection.relocateRegion(tableName, row, replicaId); } } catch (DoNotRetryIOException e) { throw e; http://git-wip-us.apache.org/repos/asf/hbase/blob/fe61a737/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestReversedScannerCallable.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestReversedScannerCallable.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestReversedScannerCallable.java new file mode 100644 index 0000000..673cc54 --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestReversedScannerCallable.java @@ -0,0 +1,94 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.client; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.HRegionLocation; +import org.apache.hadoop.hbase.RegionLocations; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.ipc.RpcControllerFactory; +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +@Category({ ClientTests.class, SmallTests.class }) +public class TestReversedScannerCallable { + + @Mock + private ClusterConnection connection; + @Mock + private Scan scan; + @Mock + private RegionLocations regionLocations; + + private final byte[] ROW = Bytes.toBytes("row1"); + + @Before + public void setUp() throws Exception { + Configuration conf = Mockito.mock(Configuration.class); + HRegionLocation regionLocation = Mockito.mock(HRegionLocation.class); + ServerName serverName = Mockito.mock(ServerName.class); + HRegionInfo regionInfo = Mockito.mock(HRegionInfo.class); + + Mockito.when(connection.getConfiguration()).thenReturn(conf); + Mockito.when(regionLocations.size()).thenReturn(1); + Mockito.when(regionLocations.getRegionLocation(0)).thenReturn(regionLocation); + Mockito.when(regionLocation.getHostname()).thenReturn("localhost"); + Mockito.when(regionLocation.getRegionInfo()).thenReturn(regionInfo); + Mockito.when(regionLocation.getServerName()).thenReturn(serverName); + Mockito.when(regionInfo.containsRow(ROW)).thenReturn(true); + Mockito.when(regionInfo.getEndKey()).thenReturn(HConstants.EMPTY_END_ROW); + Mockito.when(scan.getStartRow()).thenReturn(ROW); + } + + @Test + public void testPrepareDoesNotUseCache() throws Exception { + TableName tableName = TableName.valueOf("MyTable"); + Mockito.when(connection.relocateRegion(tableName, ROW, 0)).thenReturn(regionLocations); + + ReversedScannerCallable callable = + new ReversedScannerCallable(connection, tableName, scan, null, ROW, null, 0); + callable.prepare(true); + + Mockito.verify(connection).relocateRegion(tableName, ROW, 0); + } + + @Test + public void testPrepareUsesCache() throws Exception { + TableName tableName = TableName.valueOf("MyTable"); + Mockito.when(connection.locateRegion(tableName, ROW, true, true, 0)) + .thenReturn(regionLocations); + + ReversedScannerCallable callable = + new ReversedScannerCallable(connection, tableName, scan, null, ROW, null, 0); + callable.prepare(false); + + Mockito.verify(connection).locateRegion(tableName, ROW, true, true, 0); + } +}