From 164c250691c8bba56ee108728aa4a7494c5b006b Mon Sep 17 00:00:00 2001
From: zhangduo <zhangduo@apache.org>
Date: Wed, 20 Feb 2019 19:46:57 +0800
Subject: [PATCH] HBASE-21927 Always fail the locate request when error occur

Signed-off-by: Zheng Hu <openinx@gmail.com>
---
 .../hadoop/hbase/client/AsyncMetaRegionLocator.java    |  2 +-
 .../hadoop/hbase/client/AsyncNonMetaRegionLocator.java |  5 +++--
 .../hadoop/hbase/client/AsyncRegionLocatorHelper.java  | 10 ++++++++--
 hbase-server/pom.xml                                   |  1 -
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncMetaRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncMetaRegionLocator.java
index f5b3f92f65c..175f8f22192 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncMetaRegionLocator.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncMetaRegionLocator.java
@@ -78,7 +78,7 @@ class AsyncMetaRegionLocator {
             metaRelocateFuture.getAndSet(null).completeExceptionally(error);
             return;
           }
-          LOG.debug("The fetched meta region location is {}" + locs);
+          LOG.debug("The fetched meta region location is {}", locs);
           // Here we update cache before reset future, so it is possible that someone can get a
           // stale value. Consider this:
           // 1. update cache
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncNonMetaRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncNonMetaRegionLocator.java
index 5d5c7c3d6cf..1f23a1d985b 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncNonMetaRegionLocator.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncNonMetaRegionLocator.java
@@ -45,7 +45,6 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ConcurrentNavigableMap;
 import java.util.concurrent.ConcurrentSkipListMap;
 import org.apache.commons.lang3.ObjectUtils;
-import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseIOException;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionLocation;
@@ -275,7 +274,9 @@ class AsyncNonMetaRegionLocator {
     }
     synchronized (tableCache) {
       tableCache.pendingRequests.remove(req);
-      if (error instanceof DoNotRetryIOException) {
+      if (error != null) {
+        // fail the request itself, no matter whether it is a DoNotRetryIOException, as we have
+        // already retried several times
         CompletableFuture<?> future = tableCache.allRequests.remove(req);
         if (future != null) {
           future.completeExceptionally(error);
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocatorHelper.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocatorHelper.java
index 5c9c09154e1..dd516ec42e2 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocatorHelper.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRegionLocatorHelper.java
@@ -53,12 +53,18 @@ final class AsyncRegionLocatorHelper {
       Function<HRegionLocation, HRegionLocation> cachedLocationSupplier,
       Consumer<HRegionLocation> addToCache, Consumer<HRegionLocation> removeFromCache) {
     HRegionLocation oldLoc = cachedLocationSupplier.apply(loc);
-    LOG.debug("Try updating {} , the old value is {}", loc, oldLoc, exception);
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Try updating {} , the old value is {}, error={}", loc, oldLoc,
+        exception != null ? exception.toString() : "none");
+    }
     if (!canUpdateOnError(loc, oldLoc)) {
       return;
     }
     Throwable cause = findException(exception);
-    LOG.debug("The actual exception when updating {}", loc, cause);
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("The actual exception when updating {} is {}", loc,
+        cause != null ? cause.toString() : "none");
+    }
     if (cause == null || !isMetaClearingException(cause)) {
       LOG.debug("Will not update {} because the exception is null or not the one we care about",
         loc);
diff --git a/hbase-server/pom.xml b/hbase-server/pom.xml
index bdae0fdf794..9e0e282904f 100644
--- a/hbase-server/pom.xml
+++ b/hbase-server/pom.xml
@@ -19,7 +19,6 @@
  * limitations under the License.
  */
 -->
-<!-- for testing -->
   <modelVersion>4.0.0</modelVersion>
   <parent>
     <artifactId>hbase-build-configuration</artifactId>