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