-
Notifications
You must be signed in to change notification settings - Fork 13.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(trino): support early cancellation of queries #22498
feat(trino): support early cancellation of queries #22498
Conversation
022f1c7
to
34b9148
Compare
Codecov Report
@@ Coverage Diff @@
## master #22498 +/- ##
==========================================
- Coverage 66.90% 66.89% -0.02%
==========================================
Files 1851 1851
Lines 70696 70715 +19
Branches 7764 7766 +2
==========================================
+ Hits 47299 47303 +4
- Misses 21375 21390 +15
Partials 2022 2022
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
bb0dfb0
to
ed2f711
Compare
ef32ab2
to
5f7b62d
Compare
5f7b62d
to
65f612c
Compare
ef4483d
to
7e3977a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not tested this PR in the production Trino cluster, but I think it works as expected and saves time for the slow queries.
(cherry picked from commit b6d39d1)
SUMMARY
Currently cancelling queries on Trino fails if the query cancellation request is submitted before the query id has been retrieved. This typically happens when executing using async query execution, as the query first gets placed on the Celery queue prior to being executed. To work around this, we add a new method
prepare_cancel_query
onBaseEngineSpec
which is called when calling the query cancellation endpoint. On Trino this will set an "early query cancellation" flag toTrue
, which will trigger the query to cancel whenBaseEngineSpec.handle_cursor
is called during query execution. Appropriate tests are also added.AFTER
Now stopping a query immediately shows the correct alert for both sync and async modes:
query-stop.mp4
BEFORE
sqlllab_before.mp4
In addition, when running queries in sync mode, cancelling a query would sometimes flash the following error:
This was due to the result handler not considering the possibility of the query being returned in a stopped state. To fix this, the query result handling logic is changed to only trigger if the result is successful. Some general type cleanup is also done.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION