>From Peeyush Gupta <[email protected]>: Peeyush Gupta has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20768?usp=email )
Change subject: [ASTERIXDB-3686][API] Incorrect response for failed async request ...................................................................... [ASTERIXDB-3686][API] Incorrect response for failed async request - user model changes: no - storage format changes: no - interface changes: no Details: This change ensures that for compilation errors in async queries are properly reported with correct status and error codes. Also, async query api returns 202 Accepted on success for new API format. Ext-ref: MB-69760, MB-69761 Change-Id: I9ca684c1a90403ba2813cc858be76be69b8a4a6d Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20768 Reviewed-by: Ian Maxon <[email protected]> Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Peeyush Gupta <[email protected]> --- M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java 3 files changed, 58 insertions(+), 30 deletions(-) Approvals: Jenkins: Verified; Verified Anon. E. Moose #1000171: Ian Maxon: Looks good to me, approved Peeyush Gupta: Looks good to me, but someone else must approve 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 5ad2d1c..818205d 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 @@ -130,7 +130,11 @@ } } // if the was no error, we can set the result status to success - executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.OK); + if (delivery == IStatementExecutor.ResultDelivery.ASYNC && !isOldApi(request)) { + executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.ACCEPTED); + } else { + executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.OK); + } updateStatsFromCC(stats, responseMsg); if (param.isSignature() && delivery != IStatementExecutor.ResultDelivery.ASYNC && !param.isParseOnly()) { responsePrinter.addResultPrinter(SignaturePrinter.newInstance(responseMsg.getExecutionPlans())); 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 c4956bd..d4b6239 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 @@ -258,7 +258,12 @@ executeStatement(request, requestRef, statement, sessionOutput, resultProperties, statementProperties, stats, param, executionState, param.getOptionalParams(), statementParams, responsePrinter, warnings); - executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.OK); + if (delivery == ResultDelivery.ASYNC && !isOldApi(request)) { + executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.ACCEPTED); + } else { + executionState.setStatus(ResultStatus.SUCCESS, HttpResponseStatus.OK); + } + response.setStatus(executionState.getHttpStatus()); } errorCount = 0; } catch (Exception | org.apache.asterix.lang.sqlpp.parser.TokenMgrError e) { @@ -313,10 +318,13 @@ stats.getBufferCacheHitRatio(), stats.getBufferCachePageReadCount(), stats.getCloudReadRequestsCount(), stats.getCloudPagesReadCount(), stats.getCloudPagesPersistedCount()); if (ResultDelivery.ASYNC != delivery) { - // in case of ASYNC delivery, the status is printed by query translator responsePrinter.addFooterPrinter(new StatusPrinter(executionState.getResultStatus())); responsePrinter.addFooterPrinter(new MetricsPrinter(metrics, resultCharset)); } else { + // in case of ASYNC mode and compilation/parsing error, we need to print the statust + if (executionState.getResultStatus() == ResultStatus.FATAL) { + responsePrinter.addFooterPrinter(new StatusPrinter(ResultStatus.FATAL)); + } // Only print selected metrics for async requests responsePrinter.addFooterPrinter(new MetricsPrinter(metrics, resultCharset, Set.of(MetricsPrinter.Metrics.ELAPSED_TIME, MetricsPrinter.Metrics.QUEUE_WAIT_TIME, diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java index c4f5b3d..9d01995 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java @@ -38,7 +38,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Properties; import java.util.Set; @@ -65,10 +64,8 @@ import org.apache.asterix.app.active.ActiveNotificationHandler; import org.apache.asterix.app.active.FeedEventsListener; import org.apache.asterix.app.external.ExternalLibraryJobUtils; -import org.apache.asterix.app.result.ExecutionError; import org.apache.asterix.app.result.ResultHandle; import org.apache.asterix.app.result.ResultReader; -import org.apache.asterix.app.result.fields.ErrorsPrinter; import org.apache.asterix.app.result.fields.ResultHandlePrinter; import org.apache.asterix.app.result.fields.ResultsPrinter; import org.apache.asterix.app.result.fields.StatusPrinter; @@ -87,7 +84,6 @@ import org.apache.asterix.common.config.DatasetConfig.DatasetType; import org.apache.asterix.common.config.DatasetConfig.IndexType; import org.apache.asterix.common.config.DatasetConfig.TransactionState; -import org.apache.asterix.common.config.GlobalConfig; import org.apache.asterix.common.config.StorageProperties; import org.apache.asterix.common.dataflow.ICcApplicationContext; import org.apache.asterix.common.exceptions.ACIDException; @@ -5604,13 +5600,38 @@ switch (resultDelivery) { case ASYNC: MutableBoolean printed = new MutableBoolean(false); - executorService.submit(() -> asyncCreateAndRunJob(hcc, compiler, locker, resultDelivery, - requestParameters, cancellable, resultSetId, printed, metadataProvider, atomicStmt, jobKind)); + MutableBoolean exceptionThrown = new MutableBoolean(false); + Future<?> f = executorService.submit(() -> { + try { + asyncCreateAndRunJob(hcc, compiler, locker, resultDelivery, requestParameters, cancellable, + resultSetId, printed, metadataProvider, atomicStmt, jobKind, exceptionThrown); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); synchronized (printed) { while (!printed.booleanValue()) { printed.wait(); } } + if (exceptionThrown.booleanValue()) { + try { + f.get(); + } catch (Exception e) { + Throwable cause = e.getCause(); + // Unwrap RuntimeException wrapper if present + if (cause instanceof RuntimeException && cause.getCause() != null) { + cause = cause.getCause(); + } + if (cause instanceof Exception) { + throw (Exception) cause; + } else if (cause instanceof Error) { + throw (Error) cause; + } else { + throw HyracksDataException.create(e); + } + } + } break; case IMMEDIATE: createAndRunJob(hcc, jobFlags, null, compiler, locker, resultDelivery, id -> { @@ -5669,7 +5690,7 @@ private void asyncCreateAndRunJob(IHyracksClientConnection hcc, IStatementCompiler compiler, IMetadataLocker locker, ResultDelivery resultDelivery, IRequestParameters requestParameters, boolean cancellable, ResultSetId resultSetId, MutableBoolean printed, MetadataProvider metadataProvider, Statement atomicStmt, - JobKind jobKind) { + JobKind jobKind, MutableBoolean exceptionThrown) throws Exception { Mutable<JobId> jobId = new MutableObject<>(JobId.INVALID); final CompletableFuture<JobId> jobIdFuture = new CompletableFuture<>(); Future<?> jobSubmitFuture = executorService.submit(() -> { @@ -5688,6 +5709,9 @@ }, requestParameters, cancellable, appCtx, metadataProvider, atomicStmt, jobKind); } catch (Exception e) { jobIdFuture.completeExceptionally(e); + synchronized (printed) { + exceptionThrown.setTrue(); + } throw new RuntimeException(e); } }); @@ -5700,10 +5724,18 @@ cancelIfStarted(hcc, jobIdFuture); jobSubmitFuture.cancel(true); } catch (ExecutionException e) { - Throwable cause = e.getCause() != null ? e.getCause() : e; - handleAsyncJobException(cause, jobId.get(), resultDelivery); - } catch (Exception e) { - handleAsyncJobException(e, jobId.get(), resultDelivery); + Throwable cause = e.getCause(); + // Unwrap RuntimeException wrapper if present + if (cause instanceof RuntimeException && cause.getCause() != null) { + cause = cause.getCause(); + } + if (cause instanceof Exception) { + throw (Exception) cause; + } else if (cause instanceof Error) { + throw (Error) cause; + } else { + throw HyracksDataException.create(e); + } } finally { synchronized (printed) { if (printed.isFalse()) { @@ -5723,22 +5755,6 @@ controllerService.getResultDirectoryService().reportJobTimeout(jobId); } - private void handleAsyncJobException(Throwable e, JobId jobId, ResultDelivery resultDelivery) { - if (Objects.equals(JobId.INVALID, jobId)) { - // compilation failed - responsePrinter.addResultPrinter(new StatusPrinter(AbstractQueryApiServlet.ResultStatus.FAILED)); - responsePrinter.addResultPrinter(new ErrorsPrinter(Collections.singletonList(ExecutionError.of(e)))); - try { - responsePrinter.printResults(); - } catch (HyracksDataException ex) { - LOGGER.error("failed to print result", ex); - } - } else { - GlobalConfig.ASTERIX_LOGGER.log(Level.ERROR, - resultDelivery.name() + " job with id " + jobId + " " + "failed", e); - } - } - private void cancelIfStarted(IHyracksClientConnection hcc, CompletableFuture<JobId> jobIdFuture) { if (jobIdFuture.isDone() && !jobIdFuture.isCompletedExceptionally()) { try { -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20768?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I9ca684c1a90403ba2813cc858be76be69b8a4a6d Gerrit-Change-Number: 20768 Gerrit-PatchSet: 4 Gerrit-Owner: Peeyush Gupta <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]>
