From fc362f69cba3aaee3ddef832f9d06a1ae5293836 Mon Sep 17 00:00:00 2001
From: Phil Yang <yangzhe1991@apache.org>
Date: Wed, 19 Jul 2017 11:34:57 +0800
Subject: [PATCH] HBASE-18390 Sleep too long when finding region location
 failed

---
 .../hadoop/hbase/client/ConnectionUtils.java  | 14 -------------
 .../client/RegionAdminServiceCallable.java    |  8 +-------
 .../hbase/client/RegionServerCallable.java    |  8 +-------
 .../hbase/client/TestConnectionUtils.java     | 20 -------------------
 4 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
index 98ac8458db6..1f2fbb5c140 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
@@ -94,20 +94,6 @@ public final class ConnectionUtils {
     return normalPause + jitter;
   }
 
-  /**
-   * Adds / subs an up to 50% jitter to a pause time. Minimum is 1.
-   * @param pause the expected pause.
-   * @param jitter the jitter ratio, between 0 and 1, exclusive.
-   */
-  public static long addJitter(final long pause, final float jitter) {
-    float lag = pause * (ThreadLocalRandom.current().nextFloat() - 0.5f) * jitter;
-    long newPause = pause + (long) lag;
-    if (newPause <= 0) {
-      return 1;
-    }
-    return newPause;
-  }
-
   /**
    * @param conn The connection for which to replace the generator.
    * @param cnm Replaces the nonce generator used, for testing.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionAdminServiceCallable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionAdminServiceCallable.java
index 6846562805e..c9a143c4f7f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionAdminServiceCallable.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionAdminServiceCallable.java
@@ -51,7 +51,6 @@ public abstract class RegionAdminServiceCallable<T> implements RetryingCallable<
   protected final TableName tableName;
   protected final byte[] row;
   protected final int replicaId;
-  protected final static int MIN_WAIT_DEAD_SERVER = 10000;
 
   public RegionAdminServiceCallable(ClusterConnection connection,
       RpcControllerFactory rpcControllerFactory, TableName tableName, byte[] row) {
@@ -136,12 +135,7 @@ public abstract class RegionAdminServiceCallable<T> implements RetryingCallable<
 
   @Override
   public long sleep(long pause, int tries) {
-    long sleep = ConnectionUtils.getPauseTime(pause, tries);
-    if (sleep < MIN_WAIT_DEAD_SERVER
-        && (location == null || connection.isDeadServer(location.getServerName()))) {
-      sleep = ConnectionUtils.addJitter(MIN_WAIT_DEAD_SERVER, 0.10f);
-    }
-    return sleep;
+    return ConnectionUtils.getPauseTime(pause, tries);
   }
 
   public static RegionLocations getRegionLocations(
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionServerCallable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionServerCallable.java
index a8e17c6d943..fb593a3def5 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionServerCallable.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionServerCallable.java
@@ -58,7 +58,6 @@ public abstract class RegionServerCallable<T, S> implements RetryingCallable<T>
    * Some subclasses want to set their own location. Make it protected.
    */
   protected HRegionLocation location;
-  protected final static int MIN_WAIT_DEAD_SERVER = 10000;
   protected S stub;
 
   /**
@@ -185,12 +184,7 @@ public abstract class RegionServerCallable<T, S> implements RetryingCallable<T>
   }
 
   public long sleep(long pause, int tries) {
-    long sleep = ConnectionUtils.getPauseTime(pause, tries);
-    if (sleep < MIN_WAIT_DEAD_SERVER
-        && (location == null || getConnection().isDeadServer(location.getServerName()))) {
-      sleep = ConnectionUtils.addJitter(MIN_WAIT_DEAD_SERVER, 0.10f);
-    }
-    return sleep;
+    return ConnectionUtils.getPauseTime(pause, tries);
   }
 
   /**
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionUtils.java
index c3e4a2862cc..d34f9fa41c3 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionUtils.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionUtils.java
@@ -55,26 +55,6 @@ public class TestConnectionUtils {
     assertTrue(retyTimeSet.size() > (retries.length * 0.80));
   }
 
-  @Test
-  public void testAddJitter() {
-    long basePause = 10000;
-    long maxTimeExpected = (long) (basePause * 1.25f);
-    long minTimeExpected = (long) (basePause * 0.75f);
-    int testTries = 100;
-
-    Set<Long> timeSet = new TreeSet<>();
-    for (int i = 0; i < testTries; i++) {
-      long withJitter = ConnectionUtils.addJitter(basePause, 0.5f);
-      assertTrue(withJitter >= minTimeExpected);
-      assertTrue(withJitter <= maxTimeExpected);
-      // Add the long to the set
-      timeSet.add(withJitter);
-    }
-
-    //Make sure that most are unique.  some overlap will happen
-    assertTrue(timeSet.size() > (testTries * 0.90));
-  }
-
   @Test
   public void testGetPauseTime() {
     long pauseTime;