[NO ISSUE] Cleanup query service exception handling
Change-Id: I1f2828481df055d6c96f1ae1869ef37a065bf576
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2524
Sonar-Qube: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Murtadha Hubail <mhubail@apache.org>
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 @@
}
@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);
}
}
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.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.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 @@
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 @@
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 @@
}
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 @@
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);
}
}
}
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.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 @@
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 @@
}
}
- 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) {
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 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;
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
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 @@
return component;
}
+ public int getErrorCode() {
+ return errorCode;
+ }
+
public Object[] getParams() {
return params;
}
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 @@
@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;
}