aminghadersohi commented on PR #45232:
URL: https://github.com/apache/superset/pull/45232#issuecomment-6161699002
Product acceptance at `ce10e4f068174d2009d7beefa740c2cdc339d12c`
(regressions checked against merge-base
`1cedab01e9e2513e1c7548590a451c6434987b52`).
**Verdict: ship.** No defects found in the diff; no commits pushed.
Environment: real Impala 4.5.2 (one impalad, one admission slot, HS2 binary
and HTTP transport). The metadata DB was MySQL 8.0.46 with `REPEATABLE READ`
(verified via `@@transaction_isolation`), and a Postgres-metadata subset was
also run. Celery prefork and gunicorn ran this branch. I drove the SQL Lab REST
API, and real SQL Lab in Chromium. The Impala web UI and instrumented
`cancel_operation` / `ImpalaEngineSpec.cancel_query` counted every cancel.
| Case | This PR | master |
|---|---|---|
| Stop a running query (sync, async, HTTP transport, browser) | 15/15 + 3/3
browser `stopped`; Impala cancelled in ~1.8s; 1 cursor cancel, 0 HTTP | 6/6
`failed`, sync Run 500, 1 HTTP cancel |
| Stop an admission-queued query | 26/26 + 2/2 browser `stopped`, Run
returns in ~1.4s | 6/6 `failed` |
| Two concurrent Stops / Stop pressed twice | 6/6 and 4/4: both 200, still
one cursor cancel | – |
| Stop after completion, plus a 60-run race sweep near completion | 8/8 keep
`success`; 60/60 Run response matches the final row; stop on a `failed` query
keeps `failed` 4/4 | – |
| `execute_async` + `handle.cancel()` (running / queued) | 3/3 + 3/3
`stopped`, cancelled through the cursor | running ran 30s to `success`; queued
`failed` with unknown query handle |
| Chart-style HTTP cancel (`cancel_query` with the chart-task stub) | 3/3:
one HTTP cancel, `prepare_cancel_query` not called | – |
| Postgres / MySQL datasources | Postgres 18/18; MySQL complete/error 5/5 |
same |
| Postgres metadata DB subset | 24/24 | – |
Things I found that are **pre-existing** (they reproduce identically at the
merge-base and are not caused by this PR):
- **Timeouts on Impala.** A sync `SQLLAB_TIMEOUT` raised while
`handle_cursor` is polling is swallowed by its broad `except Exception`, so the
Run waits for the whole query and then fails from `fetch_data`'s deadline. With
an async soft time limit, the cursor is cancelled but the task hangs until the
hard limit and the row stays `running`. Separately, `get_sql_results` hardcodes
`soft_time_limit=21600`, so `SQLLAB_ASYNC_TIME_LIMIT_SEC` is not applied.
- **A Stop during Impala's fetch phase is not sent to Impala.** On a
streaming select, Impala reports `FINISHED` (rows available) almost at once and
does the work during fetch. With this PR the UI and the row say `stopped` right
away, but Impala finishes the query (15s) and a sync Run blocks until then. On
master, Stop returns 422 and the query ends `failed`, and the Run still blocks
~15s. So this is not worse, and the user-facing result is better. A possible
follow-up is a fetch loop that checks for the stop.
- A stopped Impala CTAS leaves its target table behind (24/24 here, 6/6
master).
- Stop on a MySQL datasource ends `failed` with "Lost connection to MySQL
server during query" (6/6 here, 4/4 master).
- `SQLExecutor` handle `get_result()` cannot decode stored results ("'utf-8'
codec can't decode byte 0x85") for Impala and Postgres (10/10 here, 3/3 master).
Not verified:
- Ctrl+E and window-close (`cancel_query_on_windows_unload`): headless
Chromium sent no stop request, and the frontend is unchanged by this PR.
- MCP deadline cancellation end-to-end.
- Hive, Presto and Trino.
- The frontend assets came from the `apache/superset:master` image built
2026-10-09.
--
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]
