bbeaudreault commented on code in PR #5629:
URL: https://github.com/apache/hbase/pull/5629#discussion_r1453443039


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/IPCUtil.java:
##########
@@ -159,8 +163,19 @@ static RemoteException createRemoteException(final 
ExceptionResponse e) {
   }
 
   /** Returns True if the exception is a fatal connection exception. */
-  static boolean isFatalConnectionException(final ExceptionResponse e) {
-    return 
e.getExceptionClassName().equals(FatalConnectionException.class.getName());
+  static boolean isFatalConnectionException(ExceptionResponse e) {
+    if 
(e.getExceptionClassName().equals(FatalConnectionException.class.getName())) {
+      return true;
+    }
+    // try our best to check for sub classes of FatalConnectionException
+    try {
+      return e.getExceptionClassName() != null && 
FatalConnectionException.class
+        .isAssignableFrom(Class.forName(e.getExceptionClassName()));

Review Comment:
   Might be good to pass initialize=false to Class.forName



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java:
##########
@@ -667,46 +685,34 @@ private void readResponse() {
         // this connection.
         int readSoFar = getTotalSizeWhenWrittenDelimited(responseHeader);
         int whatIsLeftToRead = totalSize - readSoFar;
+        LOG.debug("Unknown callId: " + id + ", skipping over this response of 
" + whatIsLeftToRead
+          + " bytes");
         IOUtils.skipFully(in, whatIsLeftToRead);
         if (call != null) {
           call.callStats.setResponseSizeBytes(totalSize);
-          call.callStats
-            .setCallTimeMs(EnvironmentEdgeManager.currentTime() - 
call.callStats.getStartTime());

Review Comment:
   was there a reason to remove this here?



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java:
##########
@@ -667,46 +685,34 @@ private void readResponse() {
         // this connection.
         int readSoFar = getTotalSizeWhenWrittenDelimited(responseHeader);
         int whatIsLeftToRead = totalSize - readSoFar;
+        LOG.debug("Unknown callId: " + id + ", skipping over this response of 
" + whatIsLeftToRead
+          + " bytes");
         IOUtils.skipFully(in, whatIsLeftToRead);
         if (call != null) {
           call.callStats.setResponseSizeBytes(totalSize);
-          call.callStats
-            .setCallTimeMs(EnvironmentEdgeManager.currentTime() - 
call.callStats.getStartTime());
         }
         return;
       }
-      if (responseHeader.hasException()) {
-        ExceptionResponse exceptionResponse = responseHeader.getException();
-        RemoteException re = createRemoteException(exceptionResponse);
-        call.setException(re);
-        call.callStats.setResponseSizeBytes(totalSize);
-        call.callStats

Review Comment:
   it seems like you missed copying over the call to setCallTimeMs. Add that 
above the if branch?



-- 
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