This is an automated email from the ASF dual-hosted git repository. mblow pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/asterixdb.git
commit 461415c605fd9520e0c5934c47be6c48c608eae5 Author: Ali Alsuliman <[email protected]> AuthorDate: Wed Oct 22 19:25:30 2025 -0700 [ASTERIXDB-3485][OTH] improvements to client request cancellation - user model changes: no - storage format changes: no - interface changes: yes Ext-ref: MB-63174 Change-Id: Ic94f4a87e93636ed113ffc1ed3ce2ed742bb0276 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20509 Reviewed-by: Ali Alsuliman <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Murtadha Hubail <[email protected]> --- .../apache/asterix/translator/BaseClientRequest.java | 16 ++++++++++++++-- .../api/http/server/AbstractQueryApiServlet.java | 3 +++ .../asterix/api/http/server/ActiveRequestsServlet.java | 9 ++++++--- .../apache/asterix/app/message/CancelQueryRequest.java | 7 +++++-- .../org/apache/asterix/common/api/IClientRequest.java | 9 ++++++++- .../org/apache/asterix/common/api/IRequestTracker.java | 2 +- .../src/main/resources/asx_errormsg/en.properties | 2 +- .../apache/asterix/runtime/utils/RequestTracker.java | 18 +++++++++--------- 8 files changed, 47 insertions(+), 19 deletions(-) diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/BaseClientRequest.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/BaseClientRequest.java index e03e33037a..f49f5859fc 100644 --- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/BaseClientRequest.java +++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/BaseClientRequest.java @@ -50,15 +50,18 @@ public abstract class BaseClientRequest implements IClientRequest { } @Override - public synchronized void cancel(ICcApplicationContext appCtx) throws HyracksDataException { + public synchronized boolean cancel(ICcApplicationContext appCtx) throws HyracksDataException { if (complete) { - return; + // it should also be true that the request has already been untracked by the RequestTracker + return false; } if (cancellable) { complete(); state = State.CANCELLED; doCancel(appCtx); + return true; } + return false; } @Override @@ -66,6 +69,15 @@ public abstract class BaseClientRequest implements IClientRequest { cancellable = true; } + @Override + public synchronized boolean markUncancellable() { + if (state == State.CANCELLED) { + return false; + } + cancellable = false; + return true; + } + @Override public synchronized boolean isCancelled() { return state == State.CANCELLED; diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/AbstractQueryApiServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/AbstractQueryApiServlet.java index 06c83cd8f9..e8c8a611c7 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/AbstractQueryApiServlet.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/AbstractQueryApiServlet.java @@ -161,6 +161,9 @@ public class AbstractQueryApiServlet extends AbstractServlet { logException(Level.INFO, "request execution timed out", requestId, clientContextId); executionState.setStatus(ResultStatus.TIMEOUT, HttpResponseStatus.OK); return true; + case REQUEST_CANCELLED: + executionState.setStatus(ResultStatus.FAILED, HttpResponseStatus.INTERNAL_SERVER_ERROR); + return true; case REJECT_NODE_UNREGISTERED: case REJECT_BAD_CLUSTER_STATE: logException(Level.WARN, ex.getMessage(), requestId, clientContextId); diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ActiveRequestsServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ActiveRequestsServlet.java index 3f1845e5b3..2b5d2cc787 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ActiveRequestsServlet.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ActiveRequestsServlet.java @@ -72,9 +72,12 @@ public class ActiveRequestsServlet extends AbstractRequestsServlet { } try { // Cancels the on-going job. - requestTracker.cancel(req.getId()); - // response: OK - response.setStatus(HttpResponseStatus.OK); + if (requestTracker.cancel(req.getId())) { + // response: OK + response.setStatus(HttpResponseStatus.OK); + } else { + response.setStatus(HttpResponseStatus.FORBIDDEN); + } } catch (Exception e) { LOGGER.log(Level.WARN, "unexpected exception thrown from cancel", e); // response: INTERNAL SERVER ERROR diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/CancelQueryRequest.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/CancelQueryRequest.java index 65700413b1..26277ca21e 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/CancelQueryRequest.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/CancelQueryRequest.java @@ -61,8 +61,11 @@ public class CancelQueryRequest implements ICcAddressedMessage { } else { try { requestId = req.getId(); - requestTracker.cancel(requestId); - status = RequestStatus.SUCCESS; + if (requestTracker.cancel(requestId)) { + status = RequestStatus.SUCCESS; + } else { + status = RequestStatus.REJECTED; + } } catch (Exception e) { LOGGER.log(Level.WARN, "unexpected exception thrown from cancel", e); status = RequestStatus.FAILED; diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IClientRequest.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IClientRequest.java index ee518dd664..2d239c1265 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IClientRequest.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IClientRequest.java @@ -80,6 +80,13 @@ public interface IClientRequest { */ void markCancellable(); + /** + * Mark the request as not cancellable if it has not been cancelled yet. + * + * @return true if the request was marked as not cancellable. Otherwise, false without changing anything. + */ + boolean markUncancellable(); + /** * @return true if the request can be cancelled. Otherwise false. */ @@ -91,7 +98,7 @@ public interface IClientRequest { * @param appCtx * @throws HyracksDataException */ - void cancel(ICcApplicationContext appCtx) throws HyracksDataException; + boolean cancel(ICcApplicationContext appCtx) throws HyracksDataException; /** * @return A json string representation of this request diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IRequestTracker.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IRequestTracker.java index dbae70e144..b27a4e0b6e 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IRequestTracker.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/IRequestTracker.java @@ -55,7 +55,7 @@ public interface IRequestTracker extends IJobLifecycleListener { * @param requestId * @throws HyracksDataException */ - void cancel(String requestId) throws HyracksDataException; + boolean cancel(String requestId) throws HyracksDataException; /** * Completes the request with id {@code requestId} 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 9c875d0e20..5cff16cb52 100644 --- a/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties +++ b/asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties @@ -75,7 +75,7 @@ 38 = Input contains different list types 39 = Expected integer value, got %1$s 40 = No statement provided -41 = Request %1$s has been cancelled +41 = Request has been cancelled %1$s 42 = %1$s: '%2$s' is not a TPC-DS table name 43 = Value out of range, function %1$s expects its %2$s input parameter value to be between %3$s and %4$s, received %5$s 44 = %1$s statement is not supported in read-only mode diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/RequestTracker.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/RequestTracker.java index 24bbd0611c..1dbb6b2f60 100644 --- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/RequestTracker.java +++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/RequestTracker.java @@ -101,15 +101,12 @@ public class RequestTracker implements IRequestTracker { } @Override - public void cancel(String requestId) throws HyracksDataException { + public boolean cancel(String requestId) throws HyracksDataException { final IClientRequest request = runningRequests.get(requestId); if (request == null) { - return; + return false; } - if (!request.isCancellable()) { - throw new IllegalStateException("Request " + request.getId() + " cannot be cancelled"); - } - cancel(request); + return cancel(request); } @Override @@ -131,9 +128,12 @@ public class RequestTracker implements IRequestTracker { return Collections.unmodifiableCollection(new ArrayList<>(completedRequests.values())); } - private void cancel(IClientRequest request) throws HyracksDataException { - request.cancel(ccAppCtx); - untrack(request); + private boolean cancel(IClientRequest request) throws HyracksDataException { + boolean cancelled = request.cancel(ccAppCtx); + if (cancelled) { + untrack(request); + } + return cancelled; } private void untrack(IClientRequest request) {
