You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2016/06/07 23:06:50 UTC
[2/8] hbase git commit: HBASE-15615 Wrong sleep time when
RegionServerCallable need retry (Guanghao Zhang)
HBASE-15615 Wrong sleep time when RegionServerCallable need retry (Guanghao Zhang)
Conflicts:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionAdminServiceCallable.java
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
Amending-Author: Andrew Purtell <ap...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/2d3ef97b
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/2d3ef97b
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/2d3ef97b
Branch: refs/heads/0.98
Commit: 2d3ef97bcd9837223f6866928d336e54e993726a
Parents: fb1995f
Author: Mikhail Antonov <an...@apache.org>
Authored: Sun May 15 20:49:00 2016 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Jun 7 14:18:04 2016 -0700
----------------------------------------------------------------------
.../hadoop/hbase/client/ConnectionUtils.java | 3 ++
.../hbase/client/RegionServerCallable.java | 3 +-
.../hadoop/hbase/client/RpcRetryingCaller.java | 4 +-
.../hbase/client/TestConnectionUtils.java | 20 ++++++++++
.../org/apache/hadoop/hbase/client/TestHCM.java | 40 +++++---------------
5 files changed, 35 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/2d3ef97b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
----------------------------------------------------------------------
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 0823bbb..5eb1047 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
@@ -41,6 +41,9 @@ public class ConnectionUtils {
if (ntries >= HConstants.RETRY_BACKOFF.length) {
ntries = HConstants.RETRY_BACKOFF.length - 1;
}
+ if (ntries < 0) {
+ ntries = 0;
+ }
long normalPause = pause * HConstants.RETRY_BACKOFF[ntries];
long jitter = (long)(normalPause * RANDOM.nextFloat() * 0.01f); // 1% possible jitter
http://git-wip-us.apache.org/repos/asf/hbase/blob/2d3ef97b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionServerCallable.java
----------------------------------------------------------------------
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 65cd2f3..d0a399a 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
@@ -136,8 +136,7 @@ public abstract class RegionServerCallable<T> implements RetryingCallable<T> {
@Override
public long sleep(long pause, int tries) {
- // Tries hasn't been bumped up yet so we use "tries + 1" to get right pause time
- long sleep = ConnectionUtils.getPauseTime(pause, tries + 1);
+ 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);
http://git-wip-us.apache.org/repos/asf/hbase/blob/2d3ef97b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCaller.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCaller.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCaller.java
index 95fbb92..8b1713f 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCaller.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RpcRetryingCaller.java
@@ -132,8 +132,8 @@ public class RpcRetryingCaller<T> {
}
// If the server is dead, we need to wait a little before retrying, to give
// a chance to the regions to be
- // tries hasn't been bumped up yet so we use "tries + 1" to get right pause time
- expectedSleep = callable.sleep(pause, tries + 1);
+ // get right pause time, start by RETRY_BACKOFF[0] * pause
+ expectedSleep = callable.sleep(pause, tries);
// If, after the planned sleep, there won't be enough time left, we stop now.
long duration = singleCallDuration(expectedSleep);
http://git-wip-us.apache.org/repos/asf/hbase/blob/2d3ef97b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionUtils.java
----------------------------------------------------------------------
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 649d674..3d449ae 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
@@ -19,6 +19,7 @@
*/
package org.apache.hadoop.hbase.client;
+import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -53,4 +54,23 @@ public class TestConnectionUtils {
assertTrue(retyTimeSet.size() > (retries.length * 0.80));
}
+ @Test
+ public void testGetPauseTime() {
+ long pauseTime;
+ long baseTime = 100;
+ pauseTime = ConnectionUtils.getPauseTime(baseTime, -1);
+ assertTrue(pauseTime >= (baseTime * HConstants.RETRY_BACKOFF[0]));
+ assertTrue(pauseTime <= (baseTime * HConstants.RETRY_BACKOFF[0] * 1.01f));
+
+ for (int i = 0; i < HConstants.RETRY_BACKOFF.length; i++) {
+ pauseTime = ConnectionUtils.getPauseTime(baseTime, i);
+ assertTrue(pauseTime >= (baseTime * HConstants.RETRY_BACKOFF[i]));
+ assertTrue(pauseTime <= (baseTime * HConstants.RETRY_BACKOFF[i] * 1.01f));
+ }
+
+ int length = HConstants.RETRY_BACKOFF.length;
+ pauseTime = ConnectionUtils.getPauseTime(baseTime, length);
+ assertTrue(pauseTime >= (baseTime * HConstants.RETRY_BACKOFF[length - 1]));
+ assertTrue(pauseTime <= (baseTime * HConstants.RETRY_BACKOFF[length - 1] * 1.01f));
+ }
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/2d3ef97b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
index 0ab204d..1f1c9e4 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java
@@ -866,7 +866,7 @@ public class TestHCM {
conn.close();
}
- @Ignore ("Test presumes RETRY_BACKOFF will never change; it has") @Test
+ @Test
public void testErrorBackoffTimeCalculation() throws Exception {
// TODO: This test would seem to presume hardcoded RETRY_BACKOFF which it should not.
final long ANY_PAUSE = 100;
@@ -887,46 +887,24 @@ public class TestHCM {
// Check some backoff values from HConstants sequence.
tracker.reportServerError(location);
- assertEqualsWithJitter(ANY_PAUSE, tracker.calculateBackoffTime(location, ANY_PAUSE));
+ assertEqualsWithJitter(ANY_PAUSE * HConstants.RETRY_BACKOFF[0],
+ tracker.calculateBackoffTime(location, ANY_PAUSE));
tracker.reportServerError(location);
tracker.reportServerError(location);
tracker.reportServerError(location);
- assertEqualsWithJitter(ANY_PAUSE * 5, tracker.calculateBackoffTime(location, ANY_PAUSE));
+ assertEqualsWithJitter(ANY_PAUSE * HConstants.RETRY_BACKOFF[3],
+ tracker.calculateBackoffTime(location, ANY_PAUSE));
// All of this shouldn't affect backoff for different location.
assertEquals(0, tracker.calculateBackoffTime(diffLocation, ANY_PAUSE));
tracker.reportServerError(diffLocation);
- assertEqualsWithJitter(ANY_PAUSE, tracker.calculateBackoffTime(diffLocation, ANY_PAUSE));
-
- // But should still work for a different region in the same location.
- HRegionInfo ri2 = new HRegionInfo(TABLE_NAME2);
- HRegionLocation diffRegion = new HRegionLocation(ri2, location.getServerName());
- assertEqualsWithJitter(ANY_PAUSE * 5, tracker.calculateBackoffTime(diffRegion, ANY_PAUSE));
+ assertEqualsWithJitter(ANY_PAUSE * HConstants.RETRY_BACKOFF[0],
+ tracker.calculateBackoffTime(diffLocation, ANY_PAUSE));
// Check with different base.
- assertEqualsWithJitter(ANY_PAUSE * 10,
- tracker.calculateBackoffTime(location, ANY_PAUSE * 2));
-
- // See that time from last error is taken into account. Time shift is applied after jitter,
- // so pass the original expected backoff as the base for jitter.
- long timeShift = (long)(ANY_PAUSE * 0.5);
- timeMachine.setValue(timeBase + timeShift);
- assertEqualsWithJitter((ANY_PAUSE * 5) - timeShift,
- tracker.calculateBackoffTime(location, ANY_PAUSE), ANY_PAUSE * 2);
-
- // However we should not go into negative.
- timeMachine.setValue(timeBase + ANY_PAUSE * 100);
- assertEquals(0, tracker.calculateBackoffTime(location, ANY_PAUSE));
-
- // We also should not go over the boundary; last retry would be on it.
- long timeLeft = (long)(ANY_PAUSE * 0.5);
- timeMachine.setValue(timeBase + largeAmountOfTime - timeLeft);
- assertTrue(tracker.canRetryMore(1));
- tracker.reportServerError(location);
- assertEquals(timeLeft, tracker.calculateBackoffTime(location, ANY_PAUSE));
- timeMachine.setValue(timeBase + largeAmountOfTime);
- assertFalse(tracker.canRetryMore(1));
+ assertEqualsWithJitter(ANY_PAUSE * 2 * HConstants.RETRY_BACKOFF[3],
+ tracker.calculateBackoffTime(location, ANY_PAUSE * 2));
} finally {
EnvironmentEdgeManager.reset();
}