HBASE-7756 Strange code in ServerCallable#shouldRetry

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1442560 13f79535-47bb-0310-9956-ffa450edef68

hbase-server/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java

@@ -35,6 +35,7 @@ import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.ipc.HBaseClientRPC;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.ipc.RemoteException;
 
 import com.google.protobuf.ServiceException;
@@ -59,7 +60,9 @@ public abstract class ServerCallable<T> implements Callable<T> {
   protected HRegionLocation location;
   protected ClientProtocol server;
   protected int callTimeout;
+  protected long globalStartTime;
   protected long startTime, endTime;
+  protected final static int MIN_RPC_TIMEOUT = 2000;
 
   /**
    * @param connection Connection to use.
@@ -112,27 +115,20 @@ public abstract class ServerCallable<T> implements Callable<T> {
   }
 
   public void beforeCall() {
-    HBaseClientRPC.setRpcTimeout(this.callTimeout);
+    this.startTime = EnvironmentEdgeManager.currentTimeMillis();
-    this.startTime = System.currentTimeMillis();
+    int remaining = (int)(callTimeout - (this.startTime - this.globalStartTime));
+    if (remaining < MIN_RPC_TIMEOUT) {
+      // If there is no time left, we're trying anyway. It's too late.
+      // 0 means no timeout, and it's not the intent here. So we secure both cases by
+      // resetting to the minimum.
+      remaining = MIN_RPC_TIMEOUT;
+    }
+    HBaseClientRPC.setRpcTimeout(remaining);
   }
 
   public void afterCall() {
     HBaseClientRPC.resetRpcTimeout();
-    this.endTime = System.currentTimeMillis();
+    this.endTime = EnvironmentEdgeManager.currentTimeMillis();
-  }
-
-  public void shouldRetry(Throwable throwable) throws IOException {
-    if (this.callTimeout != HConstants.DEFAULT_HBASE_CLIENT_OPERATION_TIMEOUT)
-      if (throwable instanceof SocketTimeoutException
-          || (this.endTime - this.startTime > this.callTimeout)) {
-        throw (SocketTimeoutException) (SocketTimeoutException) new SocketTimeoutException(
-            "Call to access row '" + Bytes.toString(row) + "' on table '"
-                + Bytes.toString(tableName)
-                + "' failed on socket timeout exception: " + throwable)
-            .initCause(throwable);
-      } else {
-        this.callTimeout = ((int) (this.endTime - this.startTime));
-      }
   }
 
   /**
@@ -159,13 +155,13 @@ public abstract class ServerCallable<T> implements Callable<T> {
       HConstants.DEFAULT_HBASE_CLIENT_RETRIES_NUMBER);
     List<RetriesExhaustedException.ThrowableWithExtraContext> exceptions =
       new ArrayList<RetriesExhaustedException.ThrowableWithExtraContext>();
+    this.globalStartTime = EnvironmentEdgeManager.currentTimeMillis();
     for (int tries = 0; tries < numRetries; tries++) {
       try {
         beforeCall();
         connect(tries != 0);
         return call();
       } catch (Throwable t) {
-        shouldRetry(t);
         t = translateException(t);
         if (t instanceof SocketTimeoutException ||
             t instanceof ConnectException ||
@@ -180,11 +176,21 @@ public abstract class ServerCallable<T> implements Callable<T> {
         }
         RetriesExhaustedException.ThrowableWithExtraContext qt =
           new RetriesExhaustedException.ThrowableWithExtraContext(t,
-            System.currentTimeMillis(), toString());
+              EnvironmentEdgeManager.currentTimeMillis(), toString());
         exceptions.add(qt);
         if (tries == numRetries - 1) {
           throw new RetriesExhaustedException(tries, exceptions);
         }
+        long expectedSleep = ConnectionUtils.getPauseTime(pause, tries);
+        // If, after the planned sleep, there won't be enough time left, we stop now.
+        if (((this.endTime - this.globalStartTime) + MIN_RPC_TIMEOUT + expectedSleep) >
+            this.callTimeout) {
+          throw (SocketTimeoutException) new SocketTimeoutException(
+              "Call to access row '" + Bytes.toString(row) + "' on table '"
+                  + Bytes.toString(tableName)
+                  + "' failed on timeout. " + " callTimeout=" + this.callTimeout +
+                  ", time=" + (this.endTime - this.startTime)).initCause(t);
+        }
       } finally {
         afterCall();
       }
@@ -192,7 +198,7 @@ public abstract class ServerCallable<T> implements Callable<T> {
         Thread.sleep(ConnectionUtils.getPauseTime(pause, tries));
       } catch (InterruptedException e) {
         Thread.currentThread().interrupt();
-        throw new IOException("Giving up after tries=" + tries, e);
+        throw new IOException("Interrupted after tries=" + tries, e);
       }
     }
     return null;
@@ -206,6 +212,7 @@ public abstract class ServerCallable<T> implements Callable<T> {
    */
   public T withoutRetries()
   throws IOException, RuntimeException {
+    this.globalStartTime = EnvironmentEdgeManager.currentTimeMillis();
     try {
       beforeCall();
       connect(false);