aminghadersohi commented on code in PR #45845:
URL: https://github.com/apache/superset/pull/45845#discussion_r4429313614
##########
superset/db_engine_specs/impala.py:
##########
@@ -136,30 +139,43 @@ def execute(
@classmethod
def fetch_data(cls, cursor: Any, limit: int | None = None) ->
list[tuple[Any, ...]]:
"""Wait for asynchronous operations using the public cursor API."""
- if callable(getattr(cursor, "execute_async", None)):
- from impala.error import Error
-
- deadline = time.monotonic() + app.config["SQLLAB_TIMEOUT"]
+ try:
+ if callable(getattr(cursor, "execute_async", None)):
+ from impala.error import Error
+
+ deadline = time.monotonic() + app.config["SQLLAB_TIMEOUT"]
+ try:
+ while cursor.is_executing():
+ if time.monotonic() >= deadline:
+ cls._cancel_operation(cursor)
+ raise TimeoutError(
+ "Timed out waiting for the Impala operation"
+ )
+ time.sleep(0.1)
+ if cursor.execution_failed():
+ # Fetching surfaces the driver's detailed asynchronous
error,
+ # including errors on statements that produce no
result set.
+ cursor.fetchall()
+ except Error as ex:
+ raise cls.get_dbapi_mapped_exception(ex) from ex
+ return super().fetch_data(cursor, limit)
+ except TIME_LIMIT_EXCEPTIONS:
+ # The operation is released before the time limit propagates so
+ # it does not keep running on the coordinator.
try:
- while cursor.is_executing():
- if time.monotonic() >= deadline:
- cursor.cancel_operation()
- raise TimeoutError("Timed out waiting for the Impala
operation")
- time.sleep(0.1)
- if cursor.execution_failed():
- # Fetching surfaces the driver's detailed asynchronous
error,
- # including errors on statements that produce no result
set.
- cursor.fetchall()
- except Error as ex:
- raise cls.get_dbapi_mapped_exception(ex) from ex
- return super().fetch_data(cursor, limit)
+ cursor.cancel_operation()
+ except TIME_LIMIT_EXCEPTIONS:
+ raise
+ except Exception: # pylint: disable=broad-except
+ logger.warning("cancel_operation() failed after a time limit")
+ raise
Review Comment:
I addressed this in bde3fa4522a0590de2e3030760aa05a2d4064345: the time-limit
handler reconnects before cancellation and uses `_cancel_operation` to release
the operation and cursor even when cancellation fails.
Regression test:
`tests/unit_tests/db_engine_specs/test_impala.py::test_fetch_data_reconnects_before_cancelling_an_interrupted_fetch`
(cancel success and failure). Before the change, both cases failed at `assert
calls.mock_calls[:2] == [call.reconnect(), call.cancel_operation()]`: `At index
0 diff: call.cancel_operation() != call.reconnect()`. After the change, the
Impala, Impala pending-operation, and Hive unit suites passed: 93 passed.
Pre-commit passed, including mypy. Real Impala not run; I have not
independently remeasured the live hang or coordinator resource release.
Siblings checked:
- superset/db_engine_specs/impala.py:140 — fixed in
bde3fa4522a0590de2e3030760aa05a2d4064345
- superset/db_engine_specs/impala.py:178 — not affected: shared cleanup
releases handles after an ordinary cancel failure; the interrupted-fetch caller
reconnects before entering it.
- superset/db_engine_specs/impala.py:203 — not affected: polling and
explicit Stop occur before row fetching, not after an interrupted FetchResults
response.
- superset/db_engine_specs/impala.py:321 — not affected: cancel_query uses a
separate HTTP request, not the cursor transport.
- superset/db_engine_specs/base.py:1624 — not affected: generic fetching
propagates exceptions without issuing a cancellation RPC.
- superset/db_engine_specs/hive.py:200 — not affected: Hive fetching does
not cancel after a time limit; its log-poll handler rethrows the limit without
a cancellation RPC.
- superset/db_engine_specs/spark.py — not affected: no fetch override or
cancel_operation call.
- superset/sql_lab.py:369 — not affected: dispatches fetching through the
engine spec and handles timeouts without a cursor cancellation RPC.
--
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]
