DartSqlResource: Return HTTP 202 on cancellation even if no such query. (#17278)
Return HTTP 202 (Accepted) on cancellation, even if the requested query
ID was not found.
The main reason for this is that when the Router broadcasts DELETE requests
to all Brokers, it returns the response from one of them randomly. If we
return 404 when a query ID isn't found, then the Router randomly returns 404s
even when the query really was found and canceled.
This is also arguably still correct behavior. The cancellation request
*was* accepted, it just won't do anything because the query was not in
fact running.
diff --git a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java
index ef6725d..65d770a 100644
--- a/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java
+++ b/extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/dart/controller/http/DartSqlResource.java
@@ -239,7 +239,10 @@
List<SqlLifecycleManager.Cancelable> cancelables = sqlLifecycleManager.getAll(sqlQueryId);
if (cancelables.isEmpty()) {
- return Response.status(Response.Status.NOT_FOUND).build();
+ // Return ACCEPTED even if the query wasn't found. When the Router broadcasts cancellation requests to all
+ // Brokers, this ensures the user sees a successful request.
+ AuthorizationUtils.setRequestAuthorizationAttributeIfNeeded(req);
+ return Response.status(Response.Status.ACCEPTED).build();
}
final Access access = authorizeCancellation(req, cancelables);
@@ -249,14 +252,12 @@
// Don't call cancel() on the cancelables. That just cancels native queries, which is useless here. Instead,
// get the controller and stop it.
- boolean found = false;
for (SqlLifecycleManager.Cancelable cancelable : cancelables) {
final HttpStatement stmt = (HttpStatement) cancelable;
final Object dartQueryId = stmt.context().get(DartSqlEngine.CTX_DART_QUERY_ID);
if (dartQueryId instanceof String) {
final ControllerHolder holder = controllerRegistry.get((String) dartQueryId);
if (holder != null) {
- found = true;
holder.cancel();
}
} else {
@@ -269,7 +270,9 @@
}
}
- return Response.status(found ? Response.Status.ACCEPTED : Response.Status.NOT_FOUND).build();
+ // Return ACCEPTED even if the query wasn't found. When the Router broadcasts cancellation requests to all
+ // Brokers, this ensures the user sees a successful request.
+ return Response.status(Response.Status.ACCEPTED).build();
} else {
return Response.status(Response.Status.FORBIDDEN).build();
}
diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/dart/controller/http/DartSqlResourceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/dart/controller/http/DartSqlResourceTest.java
index 8a8ce5f..51e1723 100644
--- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/dart/controller/http/DartSqlResourceTest.java
+++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/dart/controller/http/DartSqlResourceTest.java
@@ -727,7 +727,7 @@
.thenReturn(makeAuthenticationResult(REGULAR_USER_NAME));
final Response cancellationResponse = sqlResource.cancelQuery("nonexistent", httpServletRequest);
- Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), cancellationResponse.getStatus());
+ Assertions.assertEquals(Response.Status.ACCEPTED.getStatusCode(), cancellationResponse.getStatus());
}
/**
