support to show running queries and cancel query by id #9171

klsince · 2022-08-05T06:40:36Z

This PR adds the support to list running queries and cancel a query as needed.

As context, Pinot is typically used for low latency + high QPS scenario, so query cancellation hasn't been really needed. But as more and more data gets kept in Pinot, users might start to run exploratory queries over long time window, which may take minutes to finish. Query cancel support makes life easier to Iterate those kinda queries. Timeout is less handy here, as setting it too low aborts the query too soon; setting it too high might let query run off, if the query is not written properly (which often happens while exploring the data).

The key change is the extension in QueryScheduler to track map<queryId, queryFuture>. Essentially, the running query is cancelled by calling queryFuture.cancel(), which firstly interrupts task running on pqr thread, which then interrupts tasks running on pqw threads.

The other changes are for adding GET /queries and DELETE /query/<queryId> rest APIs on broker, which calls DELETE /query/<queryId> on servers via the MultiHttpMethod util to cancel the query that may be running on multiple servers.

In this PR, each broker tracks the running queries submitted to it, and not knowing the queries tracked by other brokers. So as next step, may also add the two rest APIs on controller, so that users can have cluster wide view of the running queries and cancel them from controller.

Release Notes

A new config to turn on and off the query cancellation feature on server and broker side

pinot.query.scheduler.enable.query.cancellation, false by default
pinot.broker.enable.query.cancellation, false by default

codecov-commenter · 2022-08-05T21:59:58Z

Codecov Report

Merging #9171 (9279568) into master (2aa2165) will decrease coverage by 1.29%.
The diff coverage is 43.38%.

❗ Current head 9279568 differs from pull request most recent head 637ad89. Consider uploading reports for the commit 637ad89 to get more accurate results

@@             Coverage Diff              @@
##             master    #9171      +/-   ##
============================================
- Coverage     70.01%   68.72%   -1.30%     
- Complexity     4762     4999     +237     
============================================
  Files          1856     1857       +1     
  Lines         98923    99079     +156     
  Branches      15042    15066      +24     
============================================
- Hits          69263    68089    -1174     
- Misses        24754    26116    +1362     
+ Partials       4906     4874      -32

Flag	Coverage Δ
integration1	`26.32% <22.75%> (+0.11%)`	⬆️
integration2	`?`
unittests1	`67.08% <53.16%> (-0.02%)`	⬇️
unittests2	`15.39% <21.96%> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pinot/broker/api/resources/PinotClientRequest.java	`43.28% <0.00%> (-15.91%)`	⬇️
...ot/broker/requesthandler/BrokerRequestHandler.java	`100.00% <ø> (ø)`
...r/requesthandler/BrokerRequestHandlerDelegate.java	`52.50% <0.00%> (-2.77%)`	⬇️
...ata/manager/realtime/RealtimeTableDataManager.java	`44.06% <0.00%> (-25.08%)`	⬇️
...e/pinot/core/query/request/ServerQueryRequest.java	`45.94% <0.00%> (-48.50%)`	⬇️
...t/ConcurrentMapPartitionUpsertMetadataManager.java	`66.66% <0.00%> (-1.00%)`	⬇️
...psert/ConcurrentMapTableUpsertMetadataManager.java	`50.00% <0.00%> (-50.00%)`	⬇️
...ache/pinot/server/api/resources/QueryResource.java	`0.00% <0.00%> (ø)`
...rg/apache/pinot/server/starter/ServerInstance.java	`74.43% <0.00%> (-12.69%)`	⬇️
...va/org/apache/pinot/spi/utils/CommonConstants.java	`27.69% <ø> (ø)`
... and 154 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java

pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java

npawar · 2022-08-09T23:31:23Z

pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java

In a query that is long running, this cancel may not have much effect? The check for isInterrupted only happens in the base class when we go to a new operator. So if we're already past that check, and doing a long running op, we'll still have to wait?

Good question. I think operators either finish their compute quickly or wait for sth (like the blockingQueue.poll). The waits can be interrupted.

A compute operator might miss the isInterrupted flag at the beginning but should end itself quickly. Or better refactor it to check the flag more often during the compute.

Hey, we plan to use the same cancellation mechanism for cpu time/memory usage based query pre-emption. Do you think the current frequency of checking on if (Thread.interrupted()) at each nextBlock() is insufficient to preempt a query in timely manner @npawar ?

jasperjiaguo · 2022-08-11T01:35:31Z

LGTM. Don't see any obvious conflict with the in progress query killing work.

pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java

pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BrokerRequestHandler.java

pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java

Jackie-Jiang

We should also check the code path on the server side when an interrupt is sent. I believe the query can be cancelled, but the exception message will be quite confusing because we didn't expect getting interruption. This can be done with a separate PR.

...roker/src/main/java/org/apache/pinot/broker/requesthandler/BrokerRequestHandlerDelegate.java

pinot-common/src/main/java/org/apache/pinot/common/utils/config/InstanceUtils.java

pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java

pinot-server/src/main/java/org/apache/pinot/server/api/resources/QueryResource.java

pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java

Jackie-Jiang · 2022-08-17T18:30:10Z

Let's add this new feature into the documentation

klsince force-pushed the support_query_cancel branch 3 times, most recently from d3dc5b2 to 3192574 Compare August 5, 2022 21:18

klsince force-pushed the support_query_cancel branch from 3192574 to 2d73d8e Compare August 5, 2022 23:07

jackjlli reviewed Aug 9, 2022

View reviewed changes

npawar added the feature label Aug 9, 2022

npawar reviewed Aug 9, 2022

View reviewed changes

klsince force-pushed the support_query_cancel branch 3 times, most recently from 36bc407 to 5bb9111 Compare August 10, 2022 20:18

klsince force-pushed the support_query_cancel branch 2 times, most recently from a606273 to 4d660eb Compare August 11, 2022 21:55

Jackie-Jiang reviewed Aug 12, 2022

View reviewed changes

klsince force-pushed the support_query_cancel branch from b232015 to 2a4aeed Compare August 15, 2022 22:39

Jackie-Jiang reviewed Aug 16, 2022

View reviewed changes

Jackie-Jiang mentioned this pull request Aug 16, 2022

Initial commit to support cancelling a long running query (WIP) #9141

Closed

klsince force-pushed the support_query_cancel branch from 2a4aeed to 1405535 Compare August 16, 2022 22:55

support to show running queries and cancel query by id

5a41365

klsince force-pushed the support_query_cancel branch 3 times, most recently from acd3e33 to de4c666 Compare August 17, 2022 16:34

rebase and cr

637ad89

klsince force-pushed the support_query_cancel branch from de4c666 to 637ad89 Compare August 17, 2022 17:22

Jackie-Jiang approved these changes Aug 17, 2022

View reviewed changes

Jackie-Jiang added the release-notes Referenced by PRs that need attention when compiling the next release notes label Aug 17, 2022

Jackie-Jiang merged commit 3a655d2 into apache:master Aug 17, 2022

walterddr mentioned this pull request Aug 18, 2022

[multistage] query cancel for multi-stage engine #9241

Open

4 tasks

Jackie-Jiang mentioned this pull request Aug 23, 2022

Make broker short-circuit for point queries w/ a limit clause #9137

Open

klsince deleted the support_query_cancel branch September 13, 2024 05:00

support to show running queries and cancel query by id #9171

support to show running queries and cancel query by id #9171

Uh oh!

Conversation

klsince commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

Uh oh!

codecov-commenter commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

npawar Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

klsince Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

jasperjiaguo Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

jasperjiaguo commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jackie-Jiang commented Aug 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

klsince commented Aug 5, 2022 •

edited

Loading

codecov-commenter commented Aug 5, 2022 •

edited

Loading

jasperjiaguo commented Aug 11, 2022 •

edited

Loading