Skip to content

Commit db68994

Browse files
committed
fix(client): serialize statements with the same in-flight guard as commits
The mirror image of the commit race: while a statement awaited its RPC the handle still read "open", so a concurrent commit could pass its check, land on the server without the statement's write, and the statement's late "unknown transaction" failure would overwrite the recorded outcome. A statement now transitions the handle to an in-flight state before awaiting, returning to open on success, so a concurrent commit or second statement is refused with a clear error before sending anything. Together with the committing state this makes every operation on one handle strictly serialized. Regression tests (seen failing before the fix): a commit during a slow statement and a second concurrent statement are both refused, the in-flight statement completes, and the handle works again afterwards.
1 parent 20f445e commit db68994

2 files changed

Lines changed: 64 additions & 4 deletions

File tree

coordinode/coordinode/client.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,12 @@ def is_open(self) -> bool:
314314
def _require_open(self, action: str) -> None:
315315
if self._state == "open":
316316
return
317-
if self._state == "committing":
317+
if self._state in ("committing", "executing"):
318+
in_flight = "commit" if self._state == "committing" else "statement"
318319
raise RuntimeError(
319-
f"Cannot {action} this transaction: its commit is already in flight. "
320-
"Concurrent operations on one transaction handle would race the "
321-
"commit's outcome; await it instead."
320+
f"Cannot {action} this transaction: a {in_flight} on it is already in "
321+
"flight. Concurrent operations on one transaction handle would race "
322+
"its outcome; await the in-flight operation instead."
322323
)
323324
if self._state == "aborted":
324325
raise RuntimeError(
@@ -403,6 +404,11 @@ async def cypher(
403404
)
404405

405406
self._require_open("run a statement in")
407+
# Transition BEFORE the await, mirroring commit(): a concurrent
408+
# commit slipping in while this statement is in flight could land
409+
# without the statement's write, and the statement's late "unknown
410+
# transaction" failure would then overwrite the real outcome.
411+
self._state = "executing"
406412
req = ExecuteCypherRequest(
407413
query=query,
408414
parameters=dict_to_props(params or {}),
@@ -447,6 +453,7 @@ async def cypher(
447453
self._state = "aborted"
448454
self._spawn_cleanup()
449455
raise
456+
self._state = "open"
450457
return _rows_to_dicts(resp)
451458

452459
async def commit(self) -> int:

tests/unit/test_transactions.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,3 +1254,56 @@ async def slow_commit(req, timeout=None):
12541254
assert client._cypher_stub.ExecuteCypher.await_count == 0
12551255

12561256
asyncio.run(_inner())
1257+
1258+
1259+
class TestConcurrentStatementSerialization:
1260+
"""The mirror image of the commit race: while a statement awaits its RPC
1261+
the handle must not accept a concurrent commit (the commit could land
1262+
without the statement's write, then the statement's failure would
1263+
overwrite `committed` with `aborted`) nor a second statement."""
1264+
1265+
def test_a_commit_during_a_statement_is_refused(self):
1266+
from unittest.mock import AsyncMock
1267+
1268+
async def _inner() -> None:
1269+
started = asyncio.Event()
1270+
1271+
async def slow_execute(req, timeout=None):
1272+
started.set()
1273+
await asyncio.sleep(0.05)
1274+
return _execute_response()
1275+
1276+
client = _async_client(ExecuteCypher=AsyncMock(side_effect=slow_execute))
1277+
tx = await client.begin_transaction()
1278+
stmt = asyncio.create_task(tx.cypher("CREATE (:A)"))
1279+
await started.wait()
1280+
with pytest.raises(RuntimeError, match="in flight"):
1281+
await tx.commit()
1282+
assert client._cypher_stub.CommitTransaction.await_count == 0
1283+
await stmt
1284+
# The handle is usable again once the statement resolved.
1285+
assert await tx.commit() == 7
1286+
1287+
asyncio.run(_inner())
1288+
1289+
def test_a_second_concurrent_statement_is_refused(self):
1290+
from unittest.mock import AsyncMock
1291+
1292+
async def _inner() -> None:
1293+
started = asyncio.Event()
1294+
1295+
async def slow_execute(req, timeout=None):
1296+
started.set()
1297+
await asyncio.sleep(0.05)
1298+
return _execute_response()
1299+
1300+
client = _async_client(ExecuteCypher=AsyncMock(side_effect=slow_execute))
1301+
tx = await client.begin_transaction()
1302+
stmt = asyncio.create_task(tx.cypher("CREATE (:A)"))
1303+
await started.wait()
1304+
with pytest.raises(RuntimeError, match="in flight"):
1305+
await tx.cypher("CREATE (:B)")
1306+
await stmt
1307+
assert client._cypher_stub.ExecuteCypher.await_count == 1
1308+
1309+
asyncio.run(_inner())

0 commit comments

Comments
 (0)