aminghadersohi opened a new pull request, #45232: URL: https://github.com/apache/superset/pull/45232
### SUMMARY Stopping a running Impala query in SQL Lab could fail the query instead of stopping it. Once the operation's cancel handle was published, `sql_lab.cancel_query` cancelled the query through the coordinator's HTTP `/cancel_query` endpoint. The SQL Lab worker was still polling that same operation in `ImpalaEngineSpec.handle_cursor`, which https://github.com/apache/superset/pull/44737 extended to cancel the live cursor when the query is STOPPED. The HTTP cancel invalidates the handle under the worker, so `status()` raises, `handle_cursor` returns, and `fetch_data` fails with "Invalid or unknown query handle" before the Stop request has committed STOPPED. On Impala 4.5.2 this shows as: - Stop on a running query: Stop returns 200, but the Run request returns HTTP 500 with the handle error. - Stop on an admission-queued query whose handle is already published: Stop returns 422 and the query ends `failed`. Fix: `ImpalaEngineSpec.prepare_cancel_query` always records the early-cancel flag, not only when no handle is published yet. The stop succeeds without the HTTP call, and `handle_cursor` cancels and closes the operation with `cancel_operation()` on its next poll. This works for pending/initialized operations as well as running ones. Why not `has_implicit_cancel() -> True`? That flag is also read outside SQL Lab: - The MCP worker would switch to `cursor.cancel()`, which impyla's `HiveServer2Cursor` does not have, so MCP deadline cancellation would silently stop working. - `SQLExecutor._cancel_query` would return True without persisting anything the async worker can see, so an async executor stop would no longer reach Impala. Routing through `prepare_cancel_query` changes only the two SQL Lab stop paths (`sql_lab.cancel_query` and `SQLExecutor._cancel_query`), both of which run `handle_cursor`. The HTTP `cancel_query` stays in place for chart and MCP cancellation, which have no polling loop. Trade-off: a SQL Lab stop no longer cancels on the coordinator when no worker is polling the cursor at all (for example, a worker that has died). That matches how Hive and Presto stop SQL Lab queries. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend only). ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/db_engine_specs/test_impala.py tests/unit_tests/db_engine_specs/test_impala_pending.py tests/unit_tests/db_engine_specs/test_cancel_query_validation.py tests/unit_tests/sql_lab_test.py tests/unit_tests/sql/execution ``` - `test_stop_running_query_uses_live_cursor` (SQL Lab and executor paths): with a published cancel handle, Stop makes no HTTP call, and `handle_cursor` on a RUNNING operation cancels and closes it through the cursor. It fails on master, where the HTTP cancel is sent. - `test_prepare_cancel_query_flags_stop`: the flag is recorded with or without a published handle. - Existing pending-state tests (`test_stop_before_cancel_id_is_published_uses_live_cursor`, `test_cancel_unfinished_operation`, `test_stopped_status_cancels_pending_operation`) still pass, so Stop on an admission-queued query is still cancelled through the live cursor. Manual: on an Impala-backed database, run a long query in SQL Lab (for example a CTAS with `sleep()`), press Stop while it is running, and confirm the query ends as stopped with no error on the Run request. Repeat while the query is waiting in admission control. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
