Skip to content

Commit 14f2710

Browse files
ai: apply changes for #923 (2 review threads)
Addresses: - #3832976351 at src/databricks/sql/backend/kernel/client.py:793 - #3836811357 at src/databricks/sql/backend/kernel/client.py:715 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent cb7c68e commit 14f2710

1 file changed

Lines changed: 20 additions & 0 deletions

File tree

src/databricks/sql/backend/kernel/client.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,15 @@ def get_query_state(self, command_id: CommandId) -> CommandState:
704704
# state=CLOSED until the result TTL elapses.
705705
if self._kernel_session is None:
706706
raise InterfaceError("get_query_state requires an open session.")
707+
# Concurrency note: the lock guards the _async_handles / _async_result_stream_started
708+
# bookkeeping only. The retained owning handle it returns is a shared object, and
709+
# handle.status() below runs OUTSIDE the lock. Concurrent in-process polling of a
710+
# single async id from two cursors (before result streaming is claimed) therefore
711+
# invokes status() on the same underlying kernel handle concurrently; the connector
712+
# does not serialise that and does not assume the kernel handle is safe for it. Such
713+
# concurrent polling of one async id is unsupported — the supported cross-cursor
714+
# resume path re-attaches by id (the attach-by-id fallback below) once result
715+
# streaming has been claimed.
707716
with self._async_handles_lock:
708717
handle = (
709718
None
@@ -794,6 +803,17 @@ def get_execution_result(
794803
raise _wrap_kernel_exception("get_execution_result", exc) from exc
795804
# ``KernelResultSet.__init__`` calls ``arrow_schema()`` which
796805
# can raise — map that to PEP 249 too.
806+
#
807+
# Unlike the ``await_result()`` failure above, we deliberately do
808+
# NOT discard the ``_async_result_stream_started`` marker here.
809+
# By this point ``await_result()`` has already succeeded, so the
810+
# owning handle's result stream has been started (and may be
811+
# partially consumed); re-awaiting that same handle on a retry is
812+
# not safe. Leaving the marker set routes any retry through the
813+
# attach-by-id fallback, which re-materialises a fresh stream.
814+
# The trade-off is that such a retry loses the async-statement
815+
# telemetry — an accepted, narrow gap limited to the case where
816+
# result-set construction fails after a successful await.
797817
try:
798818
return self._make_result_set(stream, cursor, command_id)
799819
except Exception as exc:

0 commit comments

Comments
 (0)