Skip to content

Commit d1d0462

Browse files
zahariashclaude
authored andcommitted
fix(connection): don't block in set_query_timeout() while a query runs
The engine applies the query timeout and the per-query thread limit under the same per-connection lock that a running query holds for its whole duration. Connection.set_query_timeout() and set_max_threads_for_exec() called the engine right away, so with a query running on the connection they blocked until it finished. On the pybind backend they also held the GIL while waiting, which froze every Python thread, including an asyncio event loop that called AsyncConnection.set_query_timeout(). Both setters now record the value and return immediately. The value is applied before the next execute(), query_as_arrow() or Arrow-table call on that connection, by one thread at a time so that the latest value wins. The pybind bindings of the two native setters release the GIL, so a thread that does wait for the lock does not stall other threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 0738739 commit d1d0462

3 files changed

Lines changed: 100 additions & 4 deletions

File tree

‎src_cpp/py_connection.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,11 @@ void PyConnection::initialize(py::handle& m) {
4747
.def("query_as_arrow", &PyConnection::queryAsArrow, py::arg("statement"),
4848
py::arg("chunk_size"))
4949
.def("set_max_threads_for_exec", &PyConnection::setMaxNumThreadForExec,
50-
py::arg("num_threads"))
50+
py::arg("num_threads"), py::call_guard<py::gil_scoped_release>())
5151
.def("prepare", &PyConnection::prepare, py::arg("query"),
5252
py::arg("parameters") = py::dict())
53-
.def("set_query_timeout", &PyConnection::setQueryTimeout, py::arg("timeout_in_ms"))
53+
.def("set_query_timeout", &PyConnection::setQueryTimeout, py::arg("timeout_in_ms"),
54+
py::call_guard<py::gil_scoped_release>())
5455
.def("interrupt", &PyConnection::interrupt)
5556
.def("get_num_nodes", &PyConnection::getNumNodes, py::arg("node_name"))
5657
.def("get_num_rels", &PyConnection::getNumRels, py::arg("rel_name"))

‎src_py/connection.py‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,10 @@ def __init__(self, database: Database, num_threads: int = 0):
411411
self._calls_in_flight = 0
412412
self._prefer_pybind = False
413413
self._query_timeout_ms = 0
414+
# Native setter name -> value, applied before the next query.
415+
self._pending_settings: dict[str, int] = {}
416+
self._pending_settings_lock = threading.Lock()
417+
self._apply_settings_lock = threading.Lock()
414418
self._query_results: WeakSet[QueryResult] = WeakSet()
415419
self._capi_scan_tables: set[str] = set()
416420
# Implicit prepared-statement cache, shared by the pybind and
@@ -464,14 +468,27 @@ def set_max_threads_for_exec(self, num_threads: int) -> None:
464468
"""
465469
Set the maximum number of threads for executing queries.
466470
471+
The setting applies from the next query on this connection; this call
472+
does not wait for a running query.
473+
467474
Parameters
468475
----------
469476
num_threads : int
470477
Maximum number of threads to use for executing queries.
471478
472479
"""
473480
self.init_connection()
474-
self._connection.set_max_threads_for_exec(num_threads)
481+
with self._pending_settings_lock:
482+
self._pending_settings["set_max_threads_for_exec"] = int(num_threads)
483+
484+
def _apply_pending_settings(self) -> None:
485+
# The engine applies settings under the lock a running query holds, so
486+
# they are applied here, before a query, rather than in the setters.
487+
with self._apply_settings_lock:
488+
with self._pending_settings_lock:
489+
pending, self._pending_settings = self._pending_settings, {}
490+
for setter, value in pending.items():
491+
getattr(self._connection, setter)(value)
475492

476493
def _register_query_result(self, query_result: QueryResult) -> None:
477494
self._query_results.add(query_result)
@@ -975,6 +992,7 @@ def execute(
975992
parameters = {}
976993

977994
self.init_connection()
995+
self._apply_pending_settings()
978996
if not isinstance(parameters, dict):
979997
msg = f"Parameters must be a dict; found {type(parameters)}."
980998
raise RuntimeError(msg) # noqa: TRY004
@@ -1068,6 +1086,7 @@ def query_as_arrow(self, query: str, chunk_size: int) -> ArrowQueryResult:
10681086
This is the efficient path for CSR-aware Arrow export.
10691087
"""
10701088
self.init_connection()
1089+
self._apply_pending_settings()
10711090
if not self._using_pybind_backend():
10721091
query_result_internal = self._connection.query(query)
10731092
else:
@@ -1186,6 +1205,9 @@ def set_query_timeout(self, timeout_in_ms: int) -> None:
11861205
"""
11871206
Set the query timeout value in ms for executing queries.
11881207
1208+
The timeout applies from the next query on this connection; this call
1209+
does not wait for a running query.
1210+
11891211
Parameters
11901212
----------
11911213
timeout_in_ms : int
@@ -1194,7 +1216,8 @@ def set_query_timeout(self, timeout_in_ms: int) -> None:
11941216
"""
11951217
self.init_connection()
11961218
self._query_timeout_ms = int(timeout_in_ms)
1197-
self._connection.set_query_timeout(timeout_in_ms)
1219+
with self._pending_settings_lock:
1220+
self._pending_settings["set_query_timeout"] = self._query_timeout_ms
11981221

11991222
def interrupt(self) -> None:
12001223
"""
@@ -1317,6 +1340,7 @@ def create_arrow_table(
13171340
13181341
"""
13191342
self.init_connection()
1343+
self._apply_pending_settings()
13201344
try:
13211345
query_result_internal = self._connection.create_arrow_table(
13221346
table_name, dataframe
@@ -1350,6 +1374,7 @@ def drop_arrow_table(self, table_name: str) -> QueryResult:
13501374
13511375
"""
13521376
self.init_connection()
1377+
self._apply_pending_settings()
13531378
try:
13541379
query_result_internal = self._connection.drop_arrow_table(table_name)
13551380
except NotImplementedError:
@@ -1411,6 +1436,7 @@ def create_arrow_rel_table(
14111436
14121437
"""
14131438
self.init_connection()
1439+
self._apply_pending_settings()
14141440
layout_value = (
14151441
layout.value if isinstance(layout, ArrowRelTableLayout) else str(layout)
14161442
).upper()

‎test/test_timeout.py‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
11
from __future__ import annotations
22

3+
import contextlib
4+
import threading
5+
import time
6+
from typing import TYPE_CHECKING
7+
8+
import ladybug as lb
39
import pytest
410
from type_aliases import ConnDB
511

12+
if TYPE_CHECKING:
13+
from collections.abc import Iterator
14+
615

716
def test_timeout(conn_db_readonly: ConnDB) -> None:
817
conn, _ = conn_db_readonly
@@ -11,3 +20,63 @@ def test_timeout(conn_db_readonly: ConnDB) -> None:
1120
conn.execute(
1221
"UNWIND RANGE(1,100000) AS x UNWIND RANGE(1, 100000) AS y RETURN COUNT(x + y);"
1322
)
23+
24+
25+
# Runs for seconds without materializing large lists.
26+
LONG_QUERY = "WITH range(1, 30000) AS r UNWIND r AS x UNWIND r AS y RETURN sum(x * y)"
27+
28+
29+
def _execute_until_interrupted(conn: lb.Connection, query: str) -> None:
30+
with contextlib.suppress(RuntimeError):
31+
conn.execute(query)
32+
33+
34+
def _interrupt_until_done(conn: lb.Connection, *threads: threading.Thread) -> None:
35+
# An interrupt sent before execution starts is cleared, so repeat it.
36+
for thread in threads:
37+
while thread.is_alive():
38+
conn.interrupt()
39+
thread.join(0.05)
40+
41+
42+
@pytest.fixture
43+
def conn() -> Iterator[lb.Connection]:
44+
# A fresh connection: the shared read-only one carries other tests' settings.
45+
db = lb.Database(":memory:")
46+
conn = lb.Connection(db)
47+
yield conn
48+
conn.close()
49+
db.close()
50+
51+
52+
def test_settings_do_not_wait_for_running_query(conn: lb.Connection) -> None:
53+
worker = threading.Thread(
54+
target=_execute_until_interrupted, args=(conn, LONG_QUERY)
55+
)
56+
worker.start()
57+
time.sleep(0.3)
58+
started = time.monotonic()
59+
conn.set_query_timeout(60_000)
60+
conn.set_max_threads_for_exec(2)
61+
elapsed = time.monotonic() - started
62+
_interrupt_until_done(conn, worker)
63+
assert elapsed < 1, elapsed
64+
65+
66+
def test_applying_settings_lets_other_threads_run(conn: lb.Connection) -> None:
67+
running = threading.Thread(
68+
target=_execute_until_interrupted, args=(conn, LONG_QUERY)
69+
)
70+
running.start()
71+
time.sleep(0.3)
72+
conn.set_query_timeout(60_000)
73+
# This call applies the new timeout, which waits for the running query.
74+
waiting = threading.Thread(
75+
target=_execute_until_interrupted, args=(conn, "RETURN 1")
76+
)
77+
started = time.monotonic()
78+
waiting.start()
79+
time.sleep(0.4)
80+
elapsed = time.monotonic() - started
81+
_interrupt_until_done(conn, running, waiting)
82+
assert elapsed < 1, elapsed

0 commit comments

Comments
 (0)