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