Apache9 commented on code in PR #6462:
URL: https://github.com/apache/hbase/pull/6462#discussion_r1894924647
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java:
##########
@@ -300,13 +318,27 @@ private boolean scheduleForRetry(IOException e) {
if (numberOfAttemptsSoFar == 0 && unableToConnectToServer(e)) {
return false;
}
+
+ // Check if the num of attempts have crossed the retry limit, and if the
error type can
+ // fail-fast.
+ if (numberOfAttemptsSoFar >= failFastRetryLimit - 1 &&
isErrorTypeFailFast(e)) {
+ LOG
+ .warn("Number of retries {} exceeded limit {} for the given error
type. Scheduling server"
+ + " crash for {}", numberOfAttemptsSoFar + 1, failFastRetryLimit,
serverName, e);
+ // Expiring the server will schedule SCP and also reject the
regionserver report from the
+ // regionserver if regionserver is somehow able to send the
regionserver report to master.
+ // The master rejects the report by throwing YouAreDeadException,
which would eventually
+ // result in the regionserver abort.
+ // This will also remove "serverName" from the ServerManager's
onlineServers map.
+ master.getServerManager().expireServer(serverName);
Review Comment:
But it will log another line to say that because server is not online, which
is confusing... We should always return earlier if possible.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java:
##########
@@ -371,6 +404,39 @@ private boolean isSaslError(IOException e) {
}
}
+ /**
+ * Returns true if the error or its cause is of type
ConnectionClosedException.
+ * @param e IOException thrown by the underlying rpc framework.
+ * @return True if the error or its cause is of type
ConnectionClosedException.
+ */
+ private boolean isConnectionClosedError(IOException e) {
+ if (e instanceof ConnectionClosedException) {
+ return true;
+ }
+ Throwable cause = e;
+ while (true) {
+ if (cause instanceof IOException) {
Review Comment:
OK, I saw you have already used this style in isSaslError, I should stop it
at the first place...
Let's file new issues to polish it. UnwrapException is not designed to be
used here, it just want to put the instanceof test inside the method so we do
not need to write it everywhere but here we already have the instanceof, so
calling this method again does not make sense...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
