From 6b7ebc019c8c64a7e7a00461029f699b0f0e3772 Mon Sep 17 00:00:00 2001 From: Phil Yang 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 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 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 implements RetryingCallable * 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 implements RetryingCallable } 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 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;