Skip to content

Commit 2736b2d

Browse files
fix: reuse terminal async Thrift status
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
1 parent ac1ada2 commit 2736b2d

2 files changed

Lines changed: 19 additions & 7 deletions

File tree

‎src/databricks/sql/backend/thrift_backend.py‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import re
77
import time
88
import threading
9+
import weakref
910
from typing import Dict, List, Optional, Union, Any, TYPE_CHECKING
1011
from uuid import UUID
1112

@@ -267,6 +268,7 @@ def __init__(
267268
raise
268269

269270
self._request_lock = threading.RLock()
271+
self._terminal_operation_statuses = weakref.WeakKeyDictionary()
270272
self._session_id_hex = None
271273

272274
@property
@@ -929,17 +931,21 @@ def get_execution_result(
929931
) -> "ResultSet":
930932
"""Fetch a completed async result and its latest operation metadata.
931933
932-
FetchResults does not carry ``numModifiedRows``, so Thrift refreshes
933-
operation status before constructing the result set.
934+
FetchResults does not carry ``numModifiedRows``, so reuse a terminal
935+
status observed by ``get_query_state`` or refresh it when the caller
936+
resumes directly by command ID.
934937
"""
935938
thrift_handle = command_id.to_thrift_handle()
936939
if not thrift_handle:
937940
raise ValueError("Not a valid Thrift command ID")
938941

939-
operation_status = self._poll_for_status(thrift_handle)
940-
self._check_command_not_in_error_or_closed_state(
941-
thrift_handle, operation_status
942-
)
942+
with self._request_lock:
943+
operation_status = self._terminal_operation_statuses.pop(command_id, None)
944+
if operation_status is None:
945+
operation_status = self._poll_for_status(thrift_handle)
946+
self._check_command_not_in_error_or_closed_state(
947+
thrift_handle, operation_status
948+
)
943949

944950
req = ttypes.TFetchResultsReq(
945951
operationHandle=ttypes.TOperationHandle(
@@ -1040,6 +1046,9 @@ def get_query_state(self, command_id: CommandId) -> CommandState:
10401046
state = CommandState.from_thrift_state(operation_state)
10411047
if state is None:
10421048
raise ValueError(f"Unknown command state: {operation_state}")
1049+
if state == CommandState.SUCCEEDED:
1050+
with self._request_lock:
1051+
self._terminal_operation_statuses[command_id] = poll_resp
10431052
return state
10441053

10451054
@staticmethod

‎tests/unit/test_thrift_backend.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ def test_metadata_catalog_escaping(
136136

137137
@pytest.mark.skipif(pyarrow is None, reason="PyArrow is not installed")
138138
@pytest.mark.parametrize("count", [7, 0, None])
139-
def test_async_result_reads_status_count_and_preserves_rows(count):
139+
@pytest.mark.parametrize("cache_status", [True, False])
140+
def test_async_result_reads_status_count_and_preserves_rows(count, cache_status):
140141
with (
141142
patch(
142143
"databricks.sql.backend.thrift_backend.TCLIService.Client", autospec=True
@@ -171,6 +172,8 @@ def test_async_result_reads_status_count_and_preserves_rows(count):
171172
)
172173
)
173174
cid = CommandId.from_thrift_handle(ThriftBackendTestSuite.operation_handle)
175+
if cache_status:
176+
assert backend.get_query_state(cid) == CommandState.SUCCEEDED
174177
backend.get_execution_result(cid, Mock())
175178

176179
backend._poll_for_status.assert_called_once_with(cid.to_thrift_handle())

0 commit comments

Comments
 (0)