[NO ISSUE] Cleanup query service exception handling Change-Id: I1f2828481df055d6c96f1ae1869ef37a065bf576 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2524 Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: Murtadha Hubail <mhub...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/c4eb7b14 Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/c4eb7b14 Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/c4eb7b14 Branch: refs/heads/release-0.9.4-pre-rc Commit: c4eb7b143a09edee73e3fa5c91b94dcda5413eaa Parents: e541f04 Author: Michael Blow <mb...@apache.org> Authored: Mon Mar 26 20:07:32 2018 -0400 Committer: Michael Blow <mb...@apache.org> Committed: Mon Mar 26 20:21:44 2018 -0700 ---------------------------------------------------------------------- .../api/http/server/NCQueryServiceServlet.java | 8 ++-- .../api/http/server/QueryServiceServlet.java | 48 ++++++++++++++------ .../message/ExecuteStatementRequestMessage.java | 14 +++--- .../asterix/common/exceptions/ErrorCode.java | 2 + .../main/resources/asx_errormsg/en.properties | 2 + .../common/exceptions/AlgebricksException.java | 4 ++ .../api/exceptions/HyracksException.java | 4 +- 7 files changed, 54 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java index 3564736..a420efc 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java @@ -154,13 +154,13 @@ public class NCQueryServiceServlet extends QueryServiceServlet { } @Override - protected void handleExecuteStatementException(Throwable t, RequestExecutionState execution) { - if (t instanceof TimeoutException + protected void handleExecuteStatementException(Throwable t, RequestExecutionState state, RequestParameters param) { + if (t instanceof TimeoutException // TODO(mblow): I don't think t can ever been an instance of TimeoutException || ExceptionUtils.matchingCause(t, candidate -> candidate instanceof IPCException)) { GlobalConfig.ASTERIX_LOGGER.log(Level.WARN, t.toString(), t); - execution.setStatus(ResultStatus.FAILED, HttpResponseStatus.SERVICE_UNAVAILABLE); + state.setStatus(ResultStatus.FAILED, HttpResponseStatus.SERVICE_UNAVAILABLE); } else { - super.handleExecuteStatementException(t, execution); + super.handleExecuteStatementException(t, state, param); } } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java index 1057a73..56359e3 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java @@ -18,6 +18,11 @@ */ package org.apache.asterix.api.http.server; +import static org.apache.asterix.common.exceptions.ErrorCode.ASTERIX; +import static org.apache.asterix.common.exceptions.ErrorCode.QUERY_TIMEOUT; +import static org.apache.asterix.common.exceptions.ErrorCode.REJECT_BAD_CLUSTER_STATE; +import static org.apache.asterix.common.exceptions.ErrorCode.REJECT_NODE_UNREGISTERED; + import java.io.IOException; import java.io.PrintWriter; import java.net.InetAddress; @@ -35,7 +40,6 @@ import org.apache.asterix.common.config.GlobalConfig; import org.apache.asterix.common.context.IStorageComponentProvider; import org.apache.asterix.common.dataflow.ICcApplicationContext; import org.apache.asterix.common.exceptions.AsterixException; -import org.apache.asterix.common.exceptions.ErrorCode; import org.apache.asterix.compiler.provider.ILangCompilationProvider; import org.apache.asterix.lang.aql.parser.TokenMgrError; import org.apache.asterix.lang.common.base.IParser; @@ -69,7 +73,6 @@ import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; - import io.netty.handler.codec.http.HttpResponseStatus; public class QueryServiceServlet extends AbstractQueryApiServlet { @@ -212,7 +215,8 @@ public class QueryServiceServlet extends AbstractQueryApiServlet { on.put("maxResultReads", maxResultReads); return om.writer(new MinimalPrettyPrinter()).writeValueAsString(on); } catch (JsonProcessingException e) { // NOSONAR - return e.getMessage(); + LOGGER.debug("unexpected exception marshalling {} instance to json", getClass(), e); + return e.toString(); } } } @@ -447,7 +451,7 @@ public class QueryServiceServlet extends AbstractQueryApiServlet { private void handleRequest(IServletRequest request, IServletResponse response) throws IOException { RequestParameters param = getRequestParameters(request); - LOGGER.info(param.toString()); + LOGGER.info("handleRequest: {}", param); long elapsedStart = System.nanoTime(); final PrintWriter httpWriter = response.writer(); @@ -492,7 +496,7 @@ public class QueryServiceServlet extends AbstractQueryApiServlet { } errorCount = 0; } catch (Exception | TokenMgrError | org.apache.asterix.aqlplus.parser.TokenMgrError e) { - handleExecuteStatementException(e, execution); + handleExecuteStatementException(e, execution, param); response.setStatus(execution.getHttpStatus()); ResultUtil.printError(sessionOutput.out(), e); ResultUtil.printStatus(sessionOutput, execution.getResultStatus()); @@ -531,21 +535,35 @@ public class QueryServiceServlet extends AbstractQueryApiServlet { execution.end(); } - protected void handleExecuteStatementException(Throwable t, RequestExecutionState execution) { + protected void handleExecuteStatementException(Throwable t, RequestExecutionState state, RequestParameters param) { if (t instanceof org.apache.asterix.aqlplus.parser.TokenMgrError || t instanceof TokenMgrError || t instanceof AlgebricksException) { - GlobalConfig.ASTERIX_LOGGER.log(Level.INFO, t.getMessage(), t); - execution.setStatus(ResultStatus.FATAL, HttpResponseStatus.BAD_REQUEST); - } else if (t instanceof HyracksException) { - GlobalConfig.ASTERIX_LOGGER.log(Level.WARN, t.getMessage(), t); - if (((HyracksException) t).getErrorCode() == ErrorCode.QUERY_TIMEOUT) { - execution.setStatus(ResultStatus.TIMEOUT, HttpResponseStatus.OK); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("handleException: {}: {}", t.getMessage(), param, t); } else { - execution.setStatus(ResultStatus.FATAL, HttpResponseStatus.INTERNAL_SERVER_ERROR); + LOGGER.info("handleException: {}: {}", t.getMessage(), param); + } + state.setStatus(ResultStatus.FATAL, HttpResponseStatus.BAD_REQUEST); + } else if (t instanceof HyracksException) { + HyracksException he = (HyracksException) t; + switch (he.getComponent() + he.getErrorCode()) { + case ASTERIX + QUERY_TIMEOUT: + LOGGER.info("handleException: query execution timed out: {}", param); + state.setStatus(ResultStatus.TIMEOUT, HttpResponseStatus.OK); + break; + case ASTERIX + REJECT_BAD_CLUSTER_STATE: + case ASTERIX + REJECT_NODE_UNREGISTERED: + LOGGER.warn("handleException: {}: {}", he.getMessage(), param); + state.setStatus(ResultStatus.FATAL, HttpResponseStatus.SERVICE_UNAVAILABLE); + break; + default: + LOGGER.warn("handleException: unexpected exception {}: {}", he.getMessage(), param, he); + state.setStatus(ResultStatus.FATAL, HttpResponseStatus.INTERNAL_SERVER_ERROR); + break; } } else { - GlobalConfig.ASTERIX_LOGGER.log(Level.WARN, "Unexpected exception", t); - execution.setStatus(ResultStatus.FATAL, HttpResponseStatus.INTERNAL_SERVER_ERROR); + LOGGER.warn("handleException: unexpected exception: {}", param, t); + state.setStatus(ResultStatus.FATAL, HttpResponseStatus.INTERNAL_SERVER_ERROR); } } } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java index 31b213e..53d4f3f 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java @@ -34,6 +34,8 @@ import org.apache.asterix.common.cluster.IClusterStateManager; import org.apache.asterix.common.config.GlobalConfig; import org.apache.asterix.common.context.IStorageComponentProvider; import org.apache.asterix.common.dataflow.ICcApplicationContext; +import org.apache.asterix.common.exceptions.ErrorCode; +import org.apache.asterix.common.exceptions.RuntimeDataException; import org.apache.asterix.common.messaging.api.ICcAddressedMessage; import org.apache.asterix.compiler.provider.ILangCompilationProvider; import org.apache.asterix.hyracks.bootstrap.CCApplication; @@ -95,7 +97,7 @@ public final class ExecuteStatementRequestMessage implements ICcAddressedMessage ClusterControllerService ccSrv = (ClusterControllerService) ccSrvContext.getControllerService(); CCApplication ccApp = (CCApplication) ccSrv.getApplication(); CCMessageBroker messageBroker = (CCMessageBroker) ccSrvContext.getMessageBroker(); - final String rejectionReason = getRejectionReason(ccSrv); + final RuntimeDataException rejectionReason = getRejectionReason(ccSrv); if (rejectionReason != null) { sendRejection(rejectionReason, messageBroker); return; @@ -145,22 +147,22 @@ public final class ExecuteStatementRequestMessage implements ICcAddressedMessage } } - private String getRejectionReason(ClusterControllerService ccSrv) { + private RuntimeDataException getRejectionReason(ClusterControllerService ccSrv) { if (ccSrv.getNodeManager().getNodeControllerState(requestNodeId) == null) { - return "Node is not registerted with the CC"; + return new RuntimeDataException(ErrorCode.REJECT_NODE_UNREGISTERED); } ICcApplicationContext appCtx = (ICcApplicationContext) ccSrv.getApplicationContext(); IClusterStateManager csm = appCtx.getClusterStateManager(); final IClusterManagementWork.ClusterState clusterState = csm.getState(); if (clusterState != IClusterManagementWork.ClusterState.ACTIVE) { - return "Cannot execute request, cluster is " + clusterState; + return new RuntimeDataException(ErrorCode.REJECT_BAD_CLUSTER_STATE, clusterState); } return null; } - private void sendRejection(String reason, CCMessageBroker messageBroker) { + private void sendRejection(RuntimeDataException reason, CCMessageBroker messageBroker) { ExecuteStatementResponseMessage responseMsg = new ExecuteStatementResponseMessage(requestMessageId); - responseMsg.setError(new Exception(reason)); + responseMsg.setError(reason); try { messageBroker.sendApplicationMessageToNC(responseMsg, requestNodeId); } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java index 04054f1..8146797 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java @@ -72,6 +72,8 @@ public class ErrorCode { public static final int UNKNOWN_DURATION_UNIT = 29; public static final int QUERY_TIMEOUT = 30; public static final int INVALID_TYPE_CASTING_MATH_FUNCTION = 31; + public static final int REJECT_BAD_CLUSTER_STATE = 32; + public static final int REJECT_NODE_UNREGISTERED = 33; public static final int INSTANTIATION_ERROR = 100; http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties ---------------------------------------------------------------------- diff --git a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties index 1fb8480..f9188b8 100644 --- a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties +++ b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties @@ -65,6 +65,8 @@ 29 = Unknown duration unit %1$s 30 = Query timed out and will be cancelled 31 = Invalid type-casting math function: %1$s for converting %2$s to %3$s +32 = Cannot execute request, cluster is %1$s +33 = Node is not registered with the CC 100 = Unable to instantiate class %1$s http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/AlgebricksException.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/AlgebricksException.java b/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/AlgebricksException.java index d1feb08..adb6c79 100644 --- a/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/AlgebricksException.java +++ b/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/exceptions/AlgebricksException.java @@ -97,6 +97,10 @@ public class AlgebricksException extends Exception { return component; } + public int getErrorCode() { + return errorCode; + } + public Object[] getParams() { return params; } http://git-wip-us.apache.org/repos/asf/asterixdb/blob/c4eb7b14/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java index 3aa95b3..c85717d 100644 --- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java +++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/HyracksException.java @@ -136,9 +136,7 @@ public class HyracksException extends IOException { @Override public String getMessage() { if (msgCache == null) { - synchronized (this) { - msgCache = ErrorMessageUtil.formatMessage(component, errorCode, super.getMessage(), params); - } + msgCache = ErrorMessageUtil.formatMessage(component, errorCode, super.getMessage(), params); } return msgCache; }
