d-c-manning commented on code in PR #5391:
URL: https://github.com/apache/hbase/pull/5391#discussion_r1327886285


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java:
##########
@@ -330,6 +330,56 @@ private boolean scheduleForRetry(IOException e) {
       return true;
     }
 
+    /**
+     * The category of exceptions where we can ensure that the request has not 
yet been received
+     * and/or processed by the target regionserver yet and hence we can 
determine whether it is safe
+     * to choose different regionserver as the target.
+     * @param e IOException thrown by the underlying rpc framework.
+     * @return true if the exception belongs to the category where the 
regionserver has not yet
+     *         received the request yet.
+     */
+    private boolean unableToConnectToServer(IOException e) {
+      if (e instanceof CallQueueTooBigException) {
+        LOG.warn("request to {} failed due to {}, try={}, this usually because"
+          + " server is overloaded, give up", serverName, e, 
numberOfAttemptsSoFar);
+        return true;
+      }
+      if (isSaslError(e)) {
+        LOG.warn("{} is not reachable; give up after first attempt", 
serverName, e);
+        return true;
+      }
+      return false;
+    }
+
+    private boolean isSaslError(IOException e) {
+      if (
+        e instanceof SaslException || (e.getMessage() != null
+          && 
e.getMessage().contains(RpcConnectionConstants.RELOGIN_IS_IN_PROGRESS))
+      ) {
+        return true;
+      }
+      Throwable cause = e;
+      while (true) {
+        cause = cause.getCause();
+        if (cause == null) {
+          return false;
+        }
+        if (isThrowableOfTypeSasl(cause)) {
+          return true;
+        }
+      }
+    }
+
+    private boolean isThrowableOfTypeSasl(Throwable cause) {
+      if (cause instanceof IOException) {
+        IOException unwrappedException = unwrapException((IOException) cause);
+        return unwrappedException instanceof SaslException
+          || (unwrappedException.getMessage() != null && 
unwrappedException.getMessage()
+            .contains(RpcConnectionConstants.RELOGIN_IS_IN_PROGRESS));
+      }
+      return false;
+    }

Review Comment:
   Can the unwrapping be simplified, since `unwrapException` won't do anything 
unless it's a `RemoteException`?
   
   ```suggestion
       private boolean isSaslError(IOException e) {
         Throwable cause = e;
         while (true) {
           if (cause instanceof IOException) {
             IOException unwrappedCause = unwrapException((IOException) cause);
             if (
               unwrappedCause instanceof SaslException || 
(unwrappedCause.getMessage() != null
                 && 
unwrappedCause.getMessage().contains(RpcConnectionConstants.RELOGIN_IS_IN_PROGRESS))
             ) {
             return true;
             }
           }
           cause = cause.getCause();
           if (cause == null) {
             return false;
           }
         }
       }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to