docs: delete / cancel query #11708

techdocsmith · 2021-09-14T19:33:13Z

Adds documentation for the DELETE method to cancel queries.

techdocsmith · 2021-09-14T19:33:55Z

docs/querying/sql.md

@@ -956,6 +955,24 @@ trailer they all include: one blank line at the end of the result set. If you de
 through a JSON parsing error or through a missing trailing newline, you should assume the response was not fully
 delivered due to an error.

+### HTTP DELETE
+You can use the HTTP `DELETE` method to cancel a SQL query using the following syntax:
+`DELETE https://BROKER:8082/druid/v2/sql/{sqlQueryId}`. The DELETE method requires the `sqlQueryId`, therefore to predict the query id you must set it in the query context.


@jihoonson do we want to say something about what happens behind the scenes with this query cancellation?

docs/querying/sql.md

jihoonson · 2021-09-14T20:54:14Z

docs/querying/sql.md

+DELETE https://BROKER:8082/druid/v2/sql/{sqlQueryId}
+```
+
+The DELETE method requires the `sqlQueryId`, therefore to predict the query id you must set it in the query context.


Queries are allowed to have the same sqlQueryId even when they are different. It would be nice to mention that all queries will be cancelled if you have multiple sql queries running with the same sqlQueryId.

I was wondering about that actually. Thank you!

jihoonson · 2021-09-14T21:01:05Z

docs/querying/sql.md

+
+HTTP DELETE method uses the following syntax:
+```
+DELETE https://BROKER:8082/druid/v2/sql/{sqlQueryId}


I'm wondering whether we should promote using router instead of broker since the router is now sort of mandatory to use the web console. Or, should we mention that you can use either router or broker address here?

The POST examples used BROKER, but I can. change to ROUTER and mention both are options.

jihoonson · 2021-09-14T21:07:41Z

docs/querying/sql.md

+curl --request DELETE 'https://BROKER:8082/druid/v2/sql/myQuery01' \
+```
+
+Druid returns an HTTP 202 response for successful deletion requests. If the the query id is incorrect or if the query completes before your cancellation request, Druid returns an HTTP 404 response. 


The cancellation request requires the read permission on all resources (currently datasource and view) in the sql query. It returns 404 as well when authorization fails.

Hi @jihoonson Why is status code 404 returned for autorization failure? In compliance with the HTTP status code definition, code 403 is the appropriate one. Actually, the native query endpoint returns 403 for such case

druid/server/src/main/java/org/apache/druid/server/QueryResource.java

Lines 164 to 166 in 7220d04

if (!authResult.isAllowed()) {

throw new ForbiddenException(authResult.toString());

}

Hey @FrankChen021, per the RFC for HTTP, you can return either 403 or 404 (https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.3, https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.4). From security perspective, 404 is preferred to 403 since 403 leaks the existence of resource.

But in such case, this endpoint is already public to everyone, I think there's no concern of security problem so that we need to hide its existence. 404 might be a confusion for users when authorization fails. And it's better that we keep it consistent with the implementation of native query.

The SQL authorization checks the permission on the resources used in the SQL query, such as datasources or views. Returning 404 is to hide the existence of those datasources or views, not the endpoint. For user confusion, what is the use case where the user should know that he doesn't have permission to cancel the query? The query is usually cancelled by the user who submitted the query or the cluster admin. Either case, they will have the right permission already. For the difference from native query behavior, I'm pretty sure that most clients should handle native queries and sql queries separately anyway since their result format is different.

BUT, I think authorization failure handling should be consistent with the query posting endpoint which returns 403. I will create a PR to fix this. Thank you for bringing this up!

Raised #11710.

Thank you @jihoonson

vtlim

Looks good! I made a few phrasing edits and suggested one change in context of the HTTP POST section.

vtlim · 2021-09-14T21:01:20Z

docs/querying/sql.md

@@ -956,6 +955,30 @@ trailer they all include: one blank line at the end of the result set. If you de
 through a JSON parsing error or through a missing trailing newline, you should assume the response was not fully
 delivered due to an error.

+### HTTP DELETE
+You can use the HTTP `DELETE` method to cancel a SQL query. When you cancel a query canceled, Druid marks its `SqlLifecycle` canceled immediately. After that, any operations in `SqlLifecycle` become non-operational to avoid unnecessary execution of expensive operations.


Suggested change

You can use the HTTP `DELETE` method to cancel a SQL query. When you cancel a query canceled, Druid marks its `SqlLifecycle` canceled immediately. After that, any operations in `SqlLifecycle` become non-operational to avoid unnecessary execution of expensive operations.

You can use the HTTP `DELETE` method to cancel a SQL query. When you cancel a query, Druid immediately marks its `SqlLifecycle` as canceled. After that, any operations in `SqlLifecycle` become non-operational to avoid unnecessary execution of expensive operations.

docs/querying/sql.md

vtlim · 2021-09-14T21:08:07Z

docs/querying/sql.md

+
+HTTP DELETE method uses the following syntax:
+```
+DELETE https://BROKER:8082/druid/v2/sql/{sqlQueryId}


I like that this is clearly laid out in a code block, but it's a different format than is used to describe HTTP POST, which says: "You can make Druid SQL queries using HTTP via POST to the endpoint /druid/v2/sql/."

Perhaps have a DELETE endpoint description similar to HTTP POST, or edit HTTP POST to have a code block like done here?

docs/querying/sql.md

Co-authored-by: Jihoon Son <[email protected]>

Co-authored-by: Victoria Lim <[email protected]>

jihoonson · 2021-09-15T00:46:31Z

docs/querying/sql.md

-be a JSON object with a "query" field, like `{"query" : "SELECT COUNT(*) FROM data_source WHERE foo = 'bar'"}`.
+To use the SQL API to make Druid SQL queries, POST your query to the following endpoint on either the Router or Broker:
+```
+POST https://ROUTER:8082/druid/v2/sql/`. 


Router uses the port 8888 by default while broker uses 8082.

docs/querying/sql.md

Co-authored-by: Jihoon Son <[email protected]>

fix port for router

jihoonson · 2021-09-15T21:37:36Z

docs/querying/sql.md

@@ -990,7 +990,7 @@ Druid returns an HTTP 202 response for successful deletion requests.

 Druid returns an HTTP 404 response in the following cases:
  - Authorization fails.


Sorry @techdocsmith, now it return 403. Can you update the doc one last time?

remove authorization until it is 403

add 403 message

jihoonson

LGTM. Thanks @techdocsmith!

FrankChen021

👍

techdocsmith requested a review from jihoonson September 14, 2021 19:33

techdocsmith commented Sep 14, 2021

View reviewed changes

techdocsmith added the Area - Documentation label Sep 14, 2021

techdocsmith force-pushed the cancelsqldoc branch 2 times, most recently from 662b0e1 to 31d15a6 Compare September 14, 2021 19:42

draft delete query

a01e289

techdocsmith force-pushed the cancelsqldoc branch from 31d15a6 to a01e289 Compare September 14, 2021 19:43

techdocsmith changed the title ~~draft delete query~~ docs: delete / cancel query Sep 14, 2021

jihoonson reviewed Sep 14, 2021

View reviewed changes

vtlim reviewed Sep 14, 2021

View reviewed changes

techdocsmith and others added 4 commits September 14, 2021 14:16

Update docs/querying/sql.md

336fe0e

Co-authored-by: Jihoon Son <[email protected]>

Update docs/querying/sql.md

b8a43d1

Co-authored-by: Victoria Lim <[email protected]>

Update docs/querying/sql.md

212fbed

Co-authored-by: Victoria Lim <[email protected]>

address comments

1b53481

jihoonson reviewed Sep 15, 2021

View reviewed changes

techdocsmith and others added 3 commits September 15, 2021 08:21

Update docs/querying/sql.md

a7c0094

Co-authored-by: Jihoon Son <[email protected]>

Update docs/querying/sql.md

ee41c11

Co-authored-by: Jihoon Son <[email protected]>

Update sql.md

390b00a

fix port for router

jihoonson reviewed Sep 15, 2021

View reviewed changes

techdocsmith added 2 commits September 15, 2021 16:17

Update sql.md

bd03875

remove authorization until it is 403

Update sql.md

6a83d7b

add 403 message

jihoonson approved these changes Sep 15, 2021

View reviewed changes

FrankChen021 approved these changes Sep 16, 2021

View reviewed changes

suneet-s merged commit 1ae1bbf into apache:master Sep 16, 2021

abhishekagarwal87 added this to the 0.23.0 milestone May 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: delete / cancel query #11708

docs: delete / cancel query #11708

techdocsmith commented Sep 14, 2021

techdocsmith Sep 14, 2021

jihoonson Sep 14, 2021

techdocsmith Sep 14, 2021

jihoonson Sep 14, 2021

techdocsmith Sep 14, 2021

jihoonson Sep 14, 2021

FrankChen021 Sep 15, 2021

jihoonson Sep 15, 2021 •

edited

Loading

FrankChen021 Sep 15, 2021

jihoonson Sep 15, 2021

jihoonson Sep 15, 2021

FrankChen021 Sep 15, 2021

vtlim left a comment

vtlim Sep 14, 2021

vtlim Sep 14, 2021

jihoonson Sep 15, 2021

jihoonson Sep 15, 2021

jihoonson left a comment

FrankChen021 left a comment

	if (!authResult.isAllowed()) {
	throw new ForbiddenException(authResult.toString());
	}

	You can use the HTTP `DELETE` method to cancel a SQL query. When you cancel a query canceled, Druid marks its `SqlLifecycle` canceled immediately. After that, any operations in `SqlLifecycle` become non-operational to avoid unnecessary execution of expensive operations.
	You can use the HTTP `DELETE` method to cancel a SQL query. When you cancel a query, Druid immediately marks its `SqlLifecycle` as canceled. After that, any operations in `SqlLifecycle` become non-operational to avoid unnecessary execution of expensive operations.

		@@ -990,7 +990,7 @@ Druid returns an HTTP 202 response for successful deletion requests.

		Druid returns an HTTP 404 response in the following cases:
		- Authorization fails.

docs: delete / cancel query #11708

docs: delete / cancel query #11708

Conversation

techdocsmith commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihoonson Sep 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtlim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihoonson left a comment

Choose a reason for hiding this comment

FrankChen021 left a comment

Choose a reason for hiding this comment

jihoonson Sep 15, 2021 •

edited

Loading