aminghadersohi commented on PR #45845: URL: https://github.com/apache/superset/pull/45845#issuecomment-6511590497
Product acceptance at `85ef2d19eb32be18d0531806302f5312320ce037` against real Impala 4.5.2 (one admission slot), MySQL 8.0 metadata DB, Celery prefork and gunicorn, compared with the merge-base `5735bdb991ba501c3883c312b1264760407be78f`. **Works as described** for the phases the PR covers (a limit that lands while the operation is running or admission-queued): | Case | This PR | merge-base | |---|---|---| | Async, Celery soft limit 12 s (running / queued) | 6/6 `failed` at 12.2–12.4 s, Impala `Cancelled` | 4/4 same timing on this fixture | | Sync main-thread, `SQLLAB_TIMEOUT` 10 s (running / queued) | 6/6 `failed` at 10.2 s, "The query exceeded the 10 seconds timeout.", Impala `Cancelled` | 4/4 `failed` at 20.2 s, "Timed out waiting for the Impala operation" | | Limit lands inside `GetLog()` (3 s delay injected into `get_log`) | async 3/3 and sync 3/3 end at the limit, Impala `Cancelled` | async 2/2 killed at the hard limit with the row left `running`; sync 1/1 no response at 60 s | | Fast queries, query under the limit, analysis / parse / runtime errors | same status and messages as merge-base | — | | Stop (async, sync, admission-queued, gthread) | 12/12 `stopped`, Impala `Cancelled` | 8/8 same | **Gap in `ImpalaEngineSpec.fetch_data` (pre-existing, same at the merge-base):** when the limit lands while rows are being fetched (Impala already reports FINISHED and keeps producing rows), the run ends `failed` on time but the Impala query stays open: 5/5 here, 2/2 at the merge-base. It was still in flight 2 min 21 s later and held the admission slot, so the next SQL Lab query queued behind it and timed out too. `return super().fetch_data(cursor, limit)` sits outside the new `except TIME_LIMIT_EXCEPTIONS`, so nothing cancels the operation, and closing the impyla connection only closes the transport (Impala keeps the session until `disconnected_session_timeout`, 900 s by default). Wrapping that call in the same cancel-and-re-raise handler would cover it. Unchanged and pre-existing, as the description says: a threaded sync request (gthread) still has no time limit (no response at 45 s, here and at the merge-base). Not covered: the Hive change was not run against a live Hive server. Testing note: Impala's `sleep()` is not interruptible, so after a cancel the `CloseOperation` call blocks until the sleep ends. A `sleep()`-based query makes a timeout look like a hang both here and at the merge-base, so these runs use a cancellable cross-join query instead. -- 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]
