Repository: asterixdb
Updated Branches:
  refs/heads/master 2b057010d -> cd00eaaca


[ASTERIXDB-2190][API] fix timeout behavior

- user model changes: no
- storage format changes: no
- interface changes: no

The timeout parameter is now handled for json and urlencoded requests,
the result status for a request that has timed out is "timeout", and
invalid timeout values result in an error.

Change-Id: Ide0515dc8ef9f8c295e1dc2ffde297100634060a
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2199
Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Contrib: 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/cd00eaac
Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/cd00eaac
Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/cd00eaac

Branch: refs/heads/master
Commit: cd00eaacaac70a0a7b6fbabf3e4c225c5505b27c
Parents: 2b05701
Author: Till Westmann <ti...@apache.org>
Authored: Fri Dec 8 14:21:13 2017 -0800
Committer: Till Westmann <ti...@apache.org>
Committed: Sat Dec 9 14:26:51 2017 -0800

----------------------------------------------------------------------
 .../api/http/server/NCQueryServiceServlet.java  | 26 +++---
 .../api/http/server/QueryServiceServlet.java    | 88 +++++++++++++++-----
 .../asterix/api/http/server/ResultUtil.java     |  3 +-
 .../main/resources/asx_errormsg/en.properties   |  2 +-
 .../src/main/resources/errormsg/en.properties   |  2 +-
 5 files changed, 83 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cd00eaac/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 64ea73d..cd1064b 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
@@ -68,12 +68,13 @@ public class NCQueryServiceServlet extends 
QueryServiceServlet {
     @Override
     protected void executeStatement(String statementsText, SessionOutput 
sessionOutput,
             IStatementExecutor.ResultDelivery delivery, 
IStatementExecutor.Stats stats, RequestParameters param,
-            long[] outExecStartEnd, Map<String, String> optionalParameters) 
throws Exception {
+            RequestExecutionState execution, Map<String, String> 
optionalParameters) throws Exception {
         // Running on NC -> send 'execute' message to CC
         INCServiceContext ncCtx = (INCServiceContext) serviceCtx;
         INCMessageBroker ncMb = (INCMessageBroker) ncCtx.getMessageBroker();
-        IStatementExecutor.ResultDelivery ccDelivery = delivery == 
IStatementExecutor.ResultDelivery.IMMEDIATE
-                ? IStatementExecutor.ResultDelivery.DEFERRED : delivery;
+        IStatementExecutor.ResultDelivery ccDelivery =
+                delivery == IStatementExecutor.ResultDelivery.IMMEDIATE ? 
IStatementExecutor.ResultDelivery.DEFERRED
+                        : delivery;
         ExecuteStatementResponseMessage responseMsg;
         MessageFuture responseFuture = ncMb.registerMessageFuture();
         final String handleUrl = getHandleUrl(param.host, param.path, 
delivery);
@@ -82,13 +83,13 @@ public class NCQueryServiceServlet extends 
QueryServiceServlet {
                 param.clientContextID = UUID.randomUUID().toString();
             }
             long timeout = 
ExecuteStatementRequestMessage.DEFAULT_NC_TIMEOUT_MILLIS;
-            if (param.timeout != null) {
+            if (param.timeout != null && !param.timeout.trim().isEmpty()) {
                 timeout = 
TimeUnit.NANOSECONDS.toMillis(Duration.parseDurationStringToNanos(param.timeout));
             }
             ExecuteStatementRequestMessage requestMsg = new 
ExecuteStatementRequestMessage(ncCtx.getNodeId(),
                     responseFuture.getFutureId(), queryLanguage, 
statementsText, sessionOutput.config(), ccDelivery,
                     param.clientContextID, handleUrl, optionalParameters);
-            outExecStartEnd[0] = System.nanoTime();
+            execution.start();
             ncMb.sendMessageToCC(requestMsg);
             try {
                 responseMsg = (ExecuteStatementResponseMessage) 
responseFuture.get(timeout, TimeUnit.MILLISECONDS);
@@ -96,12 +97,13 @@ public class NCQueryServiceServlet extends 
QueryServiceServlet {
                 cancelQuery(ncMb, ncCtx.getNodeId(), param.clientContextID, e, 
false);
                 throw e;
             } catch (TimeoutException exception) {
-                RuntimeDataException hde = new 
RuntimeDataException(ErrorCode.QUERY_TIMEOUT, exception);
+                RuntimeDataException hde = new 
RuntimeDataException(ErrorCode.QUERY_TIMEOUT);
+                hde.addSuppressed(exception);
                 // cancel query
                 cancelQuery(ncMb, ncCtx.getNodeId(), param.clientContextID, 
hde, true);
                 throw hde;
             }
-            outExecStartEnd[1] = System.nanoTime();
+            execution.end();
         } finally {
             ncMb.deregisterMessageFuture(responseFuture.getFutureId());
         }
@@ -131,8 +133,8 @@ public class NCQueryServiceServlet extends 
QueryServiceServlet {
         }
     }
 
-    private void cancelQuery(INCMessageBroker messageBroker, String nodeId, 
String clientContextID,
-            Exception exception, boolean wait) {
+    private void cancelQuery(INCMessageBroker messageBroker, String nodeId, 
String clientContextID, Exception exception,
+            boolean wait) {
         MessageFuture cancelQueryFuture = 
messageBroker.registerMessageFuture();
         try {
             CancelQueryRequest cancelQueryMessage =
@@ -150,13 +152,13 @@ public class NCQueryServiceServlet extends 
QueryServiceServlet {
     }
 
     @Override
-    protected HttpResponseStatus handleExecuteStatementException(Throwable t) {
+    protected void handleExecuteStatementException(Throwable t, 
RequestExecutionState execution) {
         if (t instanceof TimeoutException
                 || (t instanceof HyracksDataException && 
ExceptionUtils.getRootCause(t) instanceof IPCException)) {
             GlobalConfig.ASTERIX_LOGGER.log(Level.WARNING, t.toString(), t);
-            return HttpResponseStatus.SERVICE_UNAVAILABLE;
+            execution.setStatus(ResultStatus.FAILED, 
HttpResponseStatus.SERVICE_UNAVAILABLE);
         } else {
-            return super.handleExecuteStatementException(t);
+            super.handleExecuteStatementException(t, execution);
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cd00eaac/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 42c9edd..0b6151f 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
@@ -20,7 +20,6 @@ package org.apache.asterix.api.http.server;
 
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.io.StringWriter;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentMap;
@@ -36,6 +35,7 @@ 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;
@@ -68,7 +68,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
 import io.netty.handler.codec.http.HttpResponseStatus;
 
 public class QueryServiceServlet extends AbstractQueryApiServlet {
-    private static final Logger LOGGER = 
Logger.getLogger(QueryServiceServlet.class.getName());
+    protected static final Logger LOGGER = 
Logger.getLogger(QueryServiceServlet.class.getName());
     protected final ILangExtension.Language queryLanguage;
     private final ILangCompilationProvider compilationProvider;
     private final IStatementExecutorFactory statementExecutorFactory;
@@ -184,6 +184,7 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
                 on.put("mode", mode);
                 on.put("clientContextID", clientContextID);
                 on.put("format", format);
+                on.put("timeout", timeout);
                 return om.writer(new 
MinimalPrettyPrinter()).writeValueAsString(on);
             } catch (JsonProcessingException e) { // NOSONAR
                 return e.getMessage();
@@ -191,6 +192,46 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
         }
     }
 
+    static final class RequestExecutionState {
+        private long execStart = -1;
+        private long execEnd = -1;
+        private ResultStatus resultStatus = ResultStatus.SUCCESS;
+        private HttpResponseStatus httpResponseStatus = HttpResponseStatus.OK;
+
+        void setStatus(ResultStatus resultStatus, HttpResponseStatus 
httpResponseStatus) {
+            this.resultStatus = resultStatus;
+            this.httpResponseStatus = httpResponseStatus;
+        }
+
+        ResultStatus getResultStatus() {
+            return resultStatus;
+        }
+
+        HttpResponseStatus getHttpStatus() {
+            return httpResponseStatus;
+        }
+
+        void start() {
+            execStart = System.nanoTime();
+        }
+
+        void end() {
+            execEnd = System.nanoTime();
+        }
+
+        void finish() {
+            if (execStart == -1) {
+                execEnd = -1;
+            } else if (execEnd == -1) {
+                execEnd = System.nanoTime();
+            }
+        }
+
+        long duration() {
+            return execEnd - execStart;
+        }
+    }
+
     private static String getParameterValue(String content, String attribute) {
         if (content == null || attribute == null) {
             return null;
@@ -289,7 +330,6 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
         ResultUtil.printField(pw, Metrics.RESULT_SIZE.str(), resultSize, true);
         pw.print("\t");
         ResultUtil.printField(pw, Metrics.PROCESSED_OBJECTS_COUNT.str(), 
processedObjects, hasErrors);
-        pw.print("\t");
         if (hasErrors) {
             pw.print("\t");
             ResultUtil.printField(pw, Metrics.ERROR_COUNT.str(), errorCount, 
false);
@@ -334,6 +374,7 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
             param.pretty = 
Boolean.parseBoolean(request.getParameter(Parameter.PRETTY.str()));
             param.mode = toLower(request.getParameter(Parameter.MODE.str()));
             param.clientContextID = 
request.getParameter(Parameter.CLIENT_ID.str());
+            param.timeout = request.getParameter(Parameter.TIMEOUT.str());
         }
         return param;
     }
@@ -391,7 +432,7 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
         HttpUtil.setContentType(response, 
HttpUtil.ContentType.APPLICATION_JSON, HttpUtil.Encoding.UTF8);
 
         Stats stats = new Stats();
-        long[] execStartEnd = new long[] { -1, -1 };
+        RequestExecutionState execution = new RequestExecutionState();
 
         // buffer the output until we are ready to set the status of the 
response message correctly
         sessionOutput.hold();
@@ -410,27 +451,24 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
             if (optionalParamProvider != null) {
                 optionalParams = optionalParamProvider.apply(request);
             }
-            response.setStatus(HttpResponseStatus.OK);
-            executeStatement(statementsText, sessionOutput, delivery, stats, 
param, execStartEnd, optionalParams);
+            response.setStatus(execution.getHttpStatus());
+            executeStatement(statementsText, sessionOutput, delivery, stats, 
param, execution, optionalParams);
             if (ResultDelivery.IMMEDIATE == delivery || 
ResultDelivery.DEFERRED == delivery) {
-                ResultUtil.printStatus(sessionOutput, ResultStatus.SUCCESS);
+                ResultUtil.printStatus(sessionOutput, 
execution.getResultStatus());
             }
             errorCount = 0;
         } catch (Exception | TokenMgrError | 
org.apache.asterix.aqlplus.parser.TokenMgrError e) {
-            response.setStatus(handleExecuteStatementException(e));
+            handleExecuteStatementException(e, execution);
+            response.setStatus(execution.getHttpStatus());
             ResultUtil.printError(sessionOutput.out(), e);
-            ResultUtil.printStatus(sessionOutput, ResultStatus.FATAL);
+            ResultUtil.printStatus(sessionOutput, execution.getResultStatus());
         } finally {
             // make sure that we stop buffering and return the result to the 
http response
             sessionOutput.release();
-            if (execStartEnd[0] == -1) {
-                execStartEnd[1] = -1;
-            } else if (execStartEnd[1] == -1) {
-                execStartEnd[1] = System.nanoTime();
-            }
+            execution.finish();
         }
-        printMetrics(sessionOutput.out(), System.nanoTime() - elapsedStart, 
execStartEnd[1] - execStartEnd[0],
-                stats.getCount(), stats.getSize(), 
stats.getProcessedObjects(), errorCount);
+        printMetrics(sessionOutput.out(), System.nanoTime() - elapsedStart, 
execution.duration(), stats.getCount(),
+                stats.getSize(), stats.getProcessedObjects(), errorCount);
         sessionOutput.out().print("}\n");
         sessionOutput.out().flush();
         if (sessionOutput.out().checkError()) {
@@ -439,7 +477,7 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
     }
 
     protected void executeStatement(String statementsText, SessionOutput 
sessionOutput, ResultDelivery delivery,
-            IStatementExecutor.Stats stats, RequestParameters param, long[] 
outExecStartEnd,
+            IStatementExecutor.Stats stats, RequestParameters param, 
RequestExecutionState execution,
             Map<String, String> optionalParameters) throws Exception {
         IClusterManagementWork.ClusterState clusterState =
                 ((ICcApplicationContext) 
appCtx).getClusterStateManager().getState();
@@ -452,25 +490,29 @@ public class QueryServiceServlet extends 
AbstractQueryApiServlet {
         MetadataManager.INSTANCE.init();
         IStatementExecutor translator = 
statementExecutorFactory.create((ICcApplicationContext) appCtx, statements,
                 sessionOutput, compilationProvider, componentProvider);
-        outExecStartEnd[0] = System.nanoTime();
+        execution.start();
         final IRequestParameters requestParameters =
                 new 
org.apache.asterix.app.translator.RequestParameters(getHyracksDataset(), 
delivery, stats, null,
                         param.clientContextID, optionalParameters);
         translator.compileAndExecute(getHyracksClientConnection(), queryCtx, 
requestParameters);
-        outExecStartEnd[1] = System.nanoTime();
+        execution.end();
     }
 
-    protected HttpResponseStatus handleExecuteStatementException(Throwable t) {
+    protected void handleExecuteStatementException(Throwable t, 
RequestExecutionState execution) {
         if (t instanceof org.apache.asterix.aqlplus.parser.TokenMgrError || t 
instanceof TokenMgrError
                 || t instanceof AlgebricksException) {
             GlobalConfig.ASTERIX_LOGGER.log(Level.INFO, t.getMessage(), t);
-            return HttpResponseStatus.BAD_REQUEST;
+            execution.setStatus(ResultStatus.FATAL, 
HttpResponseStatus.BAD_REQUEST);
         } else if (t instanceof HyracksException) {
             GlobalConfig.ASTERIX_LOGGER.log(Level.WARNING, t.getMessage(), t);
-            return HttpResponseStatus.INTERNAL_SERVER_ERROR;
+            if (((HyracksException) t).getErrorCode() == 
ErrorCode.QUERY_TIMEOUT) {
+                execution.setStatus(ResultStatus.TIMEOUT, 
HttpResponseStatus.OK);
+            } else {
+                execution.setStatus(ResultStatus.FATAL, 
HttpResponseStatus.INTERNAL_SERVER_ERROR);
+            }
         } else {
             GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, "Unexpected 
exception", t);
-            return HttpResponseStatus.INTERNAL_SERVER_ERROR;
+            execution.setStatus(ResultStatus.FATAL, 
HttpResponseStatus.INTERNAL_SERVER_ERROR);
         }
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cd00eaac/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java
----------------------------------------------------------------------
diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java
index 72d82e0..ccbf68d 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java
@@ -130,8 +130,9 @@ public class ResultUtil {
     public static void printError(PrintWriter pw, String msg, int code, 
boolean comma) {
         pw.print("\t\"");
         pw.print(AbstractQueryApiServlet.ResultFields.ERRORS.str());
-        pw.print("\": [{ \n");
+        pw.print("\": [{ \n\t");
         printField(pw, QueryServiceServlet.ErrorField.CODE.str(), code);
+        pw.print("\t");
         printField(pw, QueryServiceServlet.ErrorField.MSG.str(), 
JSONUtil.escape(msg), false);
         pw.print(comma ? "\t}],\n" : "\t}]\n");
     }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cd00eaac/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 61d8291..5a7cbc3 100644
--- a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties
+++ b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties
@@ -61,7 +61,7 @@
 25 = Polygon must have at least 3 points
 26 = %1$s can not be an instance of polygon
 27 = Operation not supported
-28 = Invalid duration %1$s
+28 = Invalid duration \"%1$s\"
 29 = Unknown duration unit %1$s
 30 = Query timed out and will be cancelled
 

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/cd00eaac/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
----------------------------------------------------------------------
diff --git 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
index 330cf1f..1f82a17 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
+++ 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
@@ -41,7 +41,7 @@
 22 = The distributed job %1$s already exists
 23 = The distributed work failed for %1$s at %2$s
 24 = No result set for job %1$s
-25 = Job %1$s has been cancelled by a user
+25 = Job %1$s has been cancelled
 26 = Node %1$s failed
 27 = File %1$s is not a directory
 28 = User doesn't have read permissions on the file %1$s

Reply via email to