-
Notifications
You must be signed in to change notification settings - Fork 114
Re-raise and stop interrupted queries in the SQL cursors, sharing the Spark handling #853
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
base: master
Are you sure you want to change the base?
Changes from all commits
168c45b
da57283
ef17dec
3338a1e
d66c699
10f7a83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -501,6 +501,39 @@ The `on_start_query_execution` callback is supported by the following cursor typ | |
| Note: `AsyncCursor` and its variants do not support this callback as they already | ||
| return the query ID immediately through their different execution model. | ||
|
|
||
| ## Query cancellation on interrupt | ||
|
|
||
| With `kill_on_interrupt` enabled, which is the default, a `KeyboardInterrupt` while `execute()` waits for the query | ||
| requests cancellation, waits until the query reaches a terminal state, and then propagates. | ||
| Cancellation is a best-effort request, so the query can still end as `SUCCEEDED` or `FAILED`. | ||
| The `query_id` property keeps the ID of the interrupted query. | ||
| If the cancellation request fails, the `KeyboardInterrupt` propagates with the error as its cause. | ||
|
|
||
| A `KeyboardInterrupt` while `execute()` is still starting the query first waits for the | ||
| [StartQueryExecution](https://docs.aws.amazon.com/athena/latest/APIReference/API_StartQueryExecution.html) | ||
| request to finish, and then cancels the query it started in the same way. | ||
| The `query_id` property returns that query's ID. | ||
| If the request has not been sent yet when the interrupt is handled, it is never sent. | ||
| `AsyncCursor` and its variants also stop a query whose start is interrupted in `execute()`. | ||
| They wait for queries on worker threads, which do not receive `KeyboardInterrupt`. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round 2 (expanded scope): claims, callers, and operations. Result: CLEAN Scope: Claims checked:
No corrections were needed beyond the round 1 repair. |
||
|
|
||
| A second `KeyboardInterrupt` during the cancellation request or these waits propagates immediately, and the query can keep running. | ||
| With `kill_on_interrupt=False`, the `KeyboardInterrupt` propagates immediately and the query keeps running. | ||
|
|
||
| ```python | ||
| from pyathena import connect | ||
|
|
||
| cursor = connect(s3_staging_dir="s3://YOUR_S3_BUCKET/path/to/", | ||
| region_name="us-west-2").cursor() | ||
| try: | ||
| cursor.execute("SELECT * FROM many_rows") | ||
| except KeyboardInterrupt: | ||
| print(f"Query {cursor.query_id} was interrupted") | ||
| raise | ||
| ``` | ||
|
|
||
| For the native asyncio cursors, see {ref}`aio-task-cancellation`. | ||
|
|
||
| ## Query polling callback | ||
|
|
||
| PyAthena provides an `on_poll` callback that is invoked once per poll iteration with the | ||
|
|
||
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.
Self-review round 2: claims, callers, and operations. Result: FINDINGS (PR description only, repaired)
Scope:
c01c56f73c7dbf973fe73b52b6093f0a32952321..2de91e768196dc061251f7609ae0d9b9be2905cd, all claims in the PR description, commit message,_poll()docstrings,docs/usage.md, anddocs/aio.md.Claims checked:
SUCCEEDED: measured with oneSELECT 1on the CI account.StopQueryExecutionon an alreadySUCCEEDEDquery returns HTTP 200 and the state staysSUCCEEDED. So the race re-raises the interrupt without a cause, and this sentence and the_poll()docstrings hold.failure[cancel]tests on both bases.kill_on_interrupt=Falsekeeps the query running: no stop request is made (test_execute_without_kill_on_interrupt).asyncio.wait_for()surfacesOperationalError(the new timeout test fails this way), and CPython'sTimeout.__aexit__converts onlyCancelledErrorintoTimeoutError._poll()(dbt-athena/src/dbt/adapters/athena/connections_legacy.py:167), so it is unaffected. Thekill_on_interruptparameter descriptions inpyathena/connection.py:228and the cursor docstrings remain accurate.Findings, repaired in the PR description:
TaskGroupcould not handle the cancellation. ATaskGroupstill raises itsExceptionGroupwhen a sibling fails, so this is narrowed to the verifiedasyncio.wait_for()effect.executemany()consequence: it now stops at the interrupted execution, where an interrupted execution that endedSUCCEEDEDused to let the loop continue. This is added, along with the dbt-athena compatibility note and the live stop measurement.Deferred (pre-existing, out of scope): the thread-pool
Async*cursor docstrings (e.g.pyathena/arrow/async_cursor.py:91) saykill_on_interruptcancels on keyboard interrupt, but their polling runs in executor threads, which never receiveKeyboardInterrupt. This PR does not change that path.