hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From te...@apache.org
Subject hbase git commit: HBASE-18665 ReversedScannerCallable invokes getRegionLocations incorrectly
Date Tue, 29 Aug 2017 14:59:31 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1.4 f0b6b988b -> ec6d35672


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 <yuzhihong@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/ec6d3567
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/ec6d3567
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/ec6d3567

Branch: refs/heads/branch-1.4
Commit: ec6d3567273ae7ab4d88a8346790527dddfdcd08
Parents: f0b6b98
Author: Peter Somogyi <psomogyi@cloudera.com>
Authored: Sat Aug 26 17:13:46 2017 +0200
Committer: tedyu <yuzhihong@gmail.com>
Committed: Tue Aug 29 07:59:22 2017 -0700

----------------------------------------------------------------------
 .../hbase/client/ReversedScannerCallable.java   |  8 +-
 .../RpcRetryingCallerWithReadReplicas.java      |  6 +-
 .../client/TestReversedScannerCallable.java     | 97 ++++++++++++++++++++
 3 files changed, 104 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/ec6d3567/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 cbf2de0..4e4abec 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
@@ -85,8 +85,8 @@ public class ReversedScannerCallable extends ScannerCallable {
       // 2. the start row is empty which means we need to locate to the last region.
       if (scan.includeStartRow() && !isEmptyStartRow(getRow())) {
         // 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 (location == null || location.getServerName() == null) {
           throw new IOException("Failed to find location, tableName="
@@ -146,8 +146,8 @@ public class ReversedScannerCallable extends ScannerCallable {
     List<HRegionLocation> regionList = new ArrayList<HRegionLocation>();
     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/ec6d3567/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 bfae3d2..cbbe9f4 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
@@ -345,10 +345,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/ec6d3567/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..6c2d0a6
--- /dev/null
+++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestReversedScannerCallable.java
@@ -0,0 +1,97 @@
+/**
+ * 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.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 RpcControllerFactory rpcFactory;
+  @Mock
+  private RegionLocations regionLocations;
+
+  private final byte[] ROW = Bytes.toBytes("row1");
+
+  @Before
+  public void setUp() throws Exception {
+    byte[] ROW_BEFORE = ConnectionUtils.createCloseRowBefore(ROW);
+
+    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_BEFORE)).thenReturn(true);
+    Mockito.when(scan.includeStartRow()).thenReturn(true);
+    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, rpcFactory);
+    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, rpcFactory);
+    callable.prepare(false);
+
+    Mockito.verify(connection).locateRegion(tableName, ROW, true, true, 0);
+  }
+}


Mime
View raw message