Bill commented on a change in pull request #6684:
URL: https://github.com/apache/geode/pull/6684#discussion_r675219223
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
##########
@@ -519,161 +509,161 @@ protected void handleException(Op op, Throwable e,
Connection conn, int retryCou
handleException(e, conn, retryCount, finalAttempt, timeoutFatal);
}
- protected void handleException(Throwable e, Connection conn, int retryCount,
boolean finalAttempt,
- boolean timeoutFatal) throws CacheRuntimeException {
- GemFireException exToThrow = null;
- String title;
- boolean invalidateServer = true;
- boolean warn = true;
- boolean forceThrow = false;
- Throwable cause = e;
+ protected void handleException(final Throwable throwable, final Connection
connection,
+ final int retryCount, final boolean finalAttempt,
+ final boolean timeoutFatal) throws CacheRuntimeException {
- cancelCriterion.checkCancelInProgress(e);
+ cancelCriterion.checkCancelInProgress(throwable);
- if (logger.isDebugEnabled() && !(e instanceof java.io.EOFException)) {
- if (e instanceof java.net.SocketTimeoutException) {
+ if (logger.isDebugEnabled() && !(throwable instanceof
java.io.EOFException)) {
+ if (throwable instanceof java.net.SocketTimeoutException) {
logger.debug("OpExecutor.handleException on Connection to {} read
timed out",
- conn.getServer());
+ connection.getServer());
} else {
- logger.debug("OpExecutor.handleException on Connection to {}",
conn.getServer(), e);
+ logger.debug("OpExecutor.handleException on Connection to {}",
connection.getServer(),
+ throwable);
}
}
// first take care of all exceptions that should not invalidate the
// connection and do not need to be logged
- if (e instanceof MessageTooLargeException) {
+ GemFireException exceptionToThrow = null;
+ String title;
+ boolean invalidateServer = true;
+ boolean warn = true;
+ boolean forceThrow = false;
+ if (throwable instanceof MessageTooLargeException) {
title = null;
- exToThrow = new GemFireIOException("message is too large to transmit",
e);
- } else if (e instanceof NotSerializableException) {
+ exceptionToThrow = new GemFireIOException("message is too large to
transmit", throwable);
+ } else if (throwable instanceof NotSerializableException) {
title = null; // no message
- exToThrow = new SerializationException("Pool message failure", e);
- } else if (e instanceof BatchException || e instanceof BatchException70) {
+ exceptionToThrow = new SerializationException("Pool message failure",
throwable);
+ } else if (throwable instanceof BatchException || throwable instanceof
BatchException70) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof RegionDestroyedException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof RegionDestroyedException) {
invalidateServer = false;
title = null;
- exToThrow = (RegionDestroyedException) e;
- } else if (e instanceof GemFireSecurityException) {
+ exceptionToThrow = (RegionDestroyedException) throwable;
+ } else if (throwable instanceof GemFireSecurityException) {
title = null;
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof SerializationException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof SerializationException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof CopyException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof CopyException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof ClassNotFoundException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof ClassNotFoundException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof TransactionException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof TransactionException) {
title = null; // no message
- exToThrow = (TransactionException) e;
+ exceptionToThrow = (TransactionException) throwable;
invalidateServer = false;
- } else if (e instanceof SynchronizationCommitConflictException) {
+ } else if (throwable instanceof SynchronizationCommitConflictException) {
title = null;
- exToThrow = (SynchronizationCommitConflictException) e;
+ exceptionToThrow = (SynchronizationCommitConflictException) throwable;
invalidateServer = false;
- } else if (e instanceof SocketException) {
- if ("Socket closed".equals(e.getMessage()) || "Connection
reset".equals(e.getMessage())
- || "Connection refused: connect".equals(e.getMessage())
- || "Connection refused".equals(e.getMessage())) {
- title = e.getMessage();
+ } else if (throwable instanceof SocketException) {
+ if ("Socket closed".equals(throwable.getMessage())
+ || "Connection reset".equals(throwable.getMessage())
+ || "Connection refused: connect".equals(throwable.getMessage())
+ || "Connection refused".equals(throwable.getMessage())) {
+ title = throwable.getMessage();
} else {
title = "SocketException";
}
- } else if (e instanceof SocketTimeoutException) {
+ } else if (throwable instanceof SocketTimeoutException) {
invalidateServer = timeoutFatal;
title = "socket timed out on client";
- cause = null;
- } else if (e instanceof ConnectionDestroyedException) {
+ } else if (throwable instanceof ConnectionDestroyedException) {
invalidateServer = false;
title = "connection was asynchronously destroyed";
- cause = null;
- } else if (e instanceof java.io.EOFException) {
+ } else if (throwable instanceof java.io.EOFException) {
title = "closed socket on server";
- } else if (e instanceof IOException) {
+ } else if (throwable instanceof IOException) {
title = "IOException";
- } else if (e instanceof BufferUnderflowException) {
+ } else if (throwable instanceof BufferUnderflowException) {
title = "buffer underflow reading from server";
- } else if (e instanceof CancelException) {
+ } else if (throwable instanceof CancelException) {
title = "Cancelled";
warn = false;
- } else if (e instanceof InternalFunctionInvocationTargetException) {
+ } else if (throwable instanceof InternalFunctionInvocationTargetException)
{
// In this case, function will be re executed
title = null;
- exToThrow = (InternalFunctionInvocationTargetException) e;
- } else if (e instanceof FunctionInvocationTargetException) {
+ exceptionToThrow = (InternalFunctionInvocationTargetException) throwable;
+ } else if (throwable instanceof FunctionInvocationTargetException) {
// in this case function will not be re executed
title = null;
- exToThrow = (GemFireException) e;
- } else if (e instanceof PutAllPartialResultException) {
+ exceptionToThrow = (GemFireException) throwable;
+ } else if (throwable instanceof PutAllPartialResultException) {
title = null;
- exToThrow = (PutAllPartialResultException) e;
+ exceptionToThrow = (PutAllPartialResultException) throwable;
invalidateServer = false;
} else {
- Throwable t = e.getCause();
+ Throwable t = throwable.getCause();
if ((t instanceof IOException)
|| (t instanceof SerializationException) || (t instanceof
CopyException)
|| (t instanceof GemFireSecurityException) || (t instanceof
ServerOperationException)
|| (t instanceof TransactionException) || (t instanceof
CancelException)) {
- handleException(t, conn, retryCount, finalAttempt, timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt, timeoutFatal);
return;
- } else if (e instanceof ServerOperationException) {
+ } else if (throwable instanceof ServerOperationException) {
title = null; // no message
- exToThrow = (ServerOperationException) e;
+ exceptionToThrow = (ServerOperationException) throwable;
invalidateServer = false; // fix for bug #42225
- } else if (e instanceof FunctionException) {
+ } else if (throwable instanceof FunctionException) {
if (t instanceof InternalFunctionInvocationTargetException) {
// Client server to re-execute for node failure
- handleException(t, conn, retryCount, finalAttempt, timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt,
timeoutFatal);
return;
} else {
title = null; // no message
- exToThrow = (FunctionException) e;
+ exceptionToThrow = (FunctionException) throwable;
}
- } else if (e instanceof ServerConnectivityException
- && e.getMessage().equals("Connection error while authenticating
user")) {
+ } else if (throwable instanceof ServerConnectivityException
+ && throwable.getMessage().equals("Connection error while
authenticating user")) {
title = null;
if (logger.isDebugEnabled()) {
- logger.debug(e.getMessage(), e);
+ logger.debug(throwable.getMessage(), throwable);
}
} else {
- title = e.toString();
+ title = throwable.toString();
forceThrow = true;
}
}
if (title != null) {
- conn.destroy();
+ connection.destroy();
if (invalidateServer) {
- endpointManager.serverCrashed(conn.getEndpoint());
+ endpointManager.serverCrashed(connection.getEndpoint());
}
boolean logEnabled = warn ? logger.isWarnEnabled() :
logger.isDebugEnabled();
boolean msgNeeded = logEnabled || finalAttempt;
if (msgNeeded) {
- final StringBuffer sb = getExceptionMessage(title, retryCount,
finalAttempt, conn);
+ final StringBuilder sb = getExceptionMessage(title, retryCount,
finalAttempt, connection);
final String msg = sb.toString();
if (logEnabled) {
if (warn) {
logger.warn(msg);
} else {
- logger.debug(msg, e);
+ logger.debug(msg, throwable);
}
}
if (forceThrow || finalAttempt) {
- exToThrow = new ServerConnectivityException(msg, cause);
+ exceptionToThrow = new ServerConnectivityException(msg, throwable);
}
}
}
- if (exToThrow != null) {
- throw exToThrow;
+ if (exceptionToThrow != null) {
+ throw exceptionToThrow;
}
}
- private StringBuffer getExceptionMessage(String exceptionName, int
retryCount,
- boolean finalAttempt, Connection connection) {
- StringBuffer message = new StringBuffer(200);
+ private StringBuilder getExceptionMessage(final String exceptionName, final
int retryCount,
Review comment:
I confirmed what @pivotal-jbarrett said by converting the explicit
`StringBuilder` to `+` and viewing the byte code. Sure enough, the byte code
contains lots of `StringBuilder` allocations and lots of `toString()` calls.
That being said, is this a performance critical path? It is only invoked if
there is an exception.
--
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]
