Skip to content

fix(spanner): release transaction lock if inline begin fails - #18409

Merged
olavloite merged 1 commit into
mainfrom
spanner-release-tx-lock-on-fail
Sep 17, 2026
Merged

fix(spanner): release transaction lock if inline begin fails#18409
olavloite merged 1 commit into
mainfrom
spanner-release-tx-lock-on-fail

Conversation

@olavloite

@olavloite olavloite commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

When execute_update() or batch_update() starts an inline begin (transaction_id is None), it acquires self._lock while waiting for the transaction ID from the server. If that request failed with an error, the lock was never released.

This was a latent bug in _async/transaction.py that became an active deadlock when async/sync parity was recently restored in snapshot.py: reintroducing _wait_for_transaction_begin() meant subsequent queries in the same transaction now try to acquire self._lock and hang.

Wrap the request execution in try...finally in _async/transaction.py and regenerate sync transaction.py so self._lock is always released.

Also:

  • Add unit tests for failed inline begin in both async and sync clients.
  • Add a GitHub Actions workflow to run Spanner mock server tests in CI.
  • Include the mockserver session in default local Nox runs.

@olavloite
olavloite requested review from a team as code owners September 17, 2026 11:41
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Sep 17, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces try-finally blocks in both the synchronous and asynchronous implementations of execute_update and batch_update within the Google Cloud Spanner transaction modules. This ensures that the transaction lock is safely released if an error occurs during an inline begin. Unit tests have been added to verify that the lock is released upon failure, and mockserver has been added to the noxfile. There are no review comments, so I have no feedback to provide.

@olavloite
fix(spanner): release transaction lock if inline begin fails
16d474c 
When execute_update() or batch_update() starts an inline begin
(transaction_id is None), it acquires self._lock while waiting for
the transaction ID from the server. If that request failed with an
error, the lock was never released.

This was a latent bug in _async/transaction.py that became an active
deadlock when async/sync parity was recently restored in snapshot.py:
reintroducing _wait_for_transaction_begin() meant subsequent queries
in the same transaction now try to acquire self._lock and hang.

Wrap the request execution in try...finally in _async/transaction.py
and regenerate sync transaction.py so self._lock is always released.

Also:
- Add unit tests for failed inline begin in both async and sync clients.
- Add a GitHub Actions workflow to run Spanner mock server tests in CI.
- Include the mockserver session in default local Nox runs.
@olavloite
olavloite force-pushed the spanner-release-tx-lock-on-fail branch from 5769984 to 16d474c Compare September 17, 2026 12:02
@olavloite
olavloite enabled auto-merge (squash) September 17, 2026 12:33
@olavloite
olavloite merged commit dd24029 into main Sep 17, 2026
130 of 179 checks passed
@olavloite
olavloite deleted the spanner-release-tx-lock-on-fail branch September 17, 2026 12:36
@release-please release-please Bot mentioned this pull request Sep 16, 2026
Merged
parthea pushed a commit that referenced this pull request Nov 12, 2026
@release-please
chore(main): release google-cloud-spanner 3.72.0 (#18340)
fe40421 
🤖 I have created a release *beep* *boop*
---


##
[3.72.0](google-cloud-spanner-v3.71.0...google-cloud-spanner-v3.72.0)
(2026-11-12)


### Features

* **dbapi:** wire timeout parameter through Connection to execute_sql
([#18484](#18484))
([98fc304](98fc304))
* declare Python3.15 support
([8775794](8775794))
* **spanner:** auth login support for Spanner Omni endpoints
([#18273](#18273))
([9e0548d](9e0548d))
* update API sources and regenerate
([#18797](#18797))
([be4d954](be4d954))


### Bug Fixes

* **google-cloud-spanner:** clear buffered DDL statements on rollback
and flush on autocommit
([#18636](#18636))
([15d6a2c](15d6a2c))
* **google-cloud-spanner:** do not replay DDL statements on transaction
retry
([#18625](#18625))
([78b9da2](78b9da2))
* **spanner:** correctly wrap async streaming calls in metrics
interceptor
([#18581](#18581))
([2d19b0a](2d19b0a))
* **spanner:** eliminate duplicate metrics capture and measure full
streaming latency
([#18605](#18605))
([a7678fd](a7678fd))
* **spanner:** record streaming operation completion when stream
finishes
([#18704](#18704))
([ddeae47](ddeae47))
* **spanner:** release transaction lock if inline begin fails
([#18409](#18409))
([dd24029](dd24029))
* **spanner:** stop deleting multiplexed sessions on rotation and close
([#18332](#18332))
([c38e960](c38e960))


### Performance Improvements

* **spanner:** add fast-path for multiplexed session acquisition
([#18317](#18317))
([9569b39](9569b39))
* **spanner:** avoid allocating empty RequestOptions on read and query
paths
([#18422](#18422))
([3743ff0](3743ff0))
* **spanner:** build ExecuteSqlRequest on the raw protobuf message
([#18379](#18379))
([6d91fec](6d91fec))
* **spanner:** eliminate redundant tracing span in run_in_transaction
([#18357](#18357))
([181e26a](181e26a))
* **spanner:** optimize built-in metrics hot path and harden concurrency
([#18329](#18329))
([f2a791f](f2a791f))
* **spanner:** optimize query option merging and prevent in-place
mutation
([#18358](#18358))
([8cb3c6a](8cb3c6a))
* **spanner:** optimize query parameter encoding with direct type
dispatch
([#18359](#18359))
([f06497c](f06497c))
* **spanner:** optimize query result decoding and type resolution
([#18330](#18330))
([ce72986](ce72986))
* **spanner:** optimize request ID header generation and retry closures
([#18408](#18408))
([ad03b95](ad03b95))
* **spanner:** optimize row popping in StreamedResultSet
([#18316](#18316))
([e66103d](e66103d))
* **spanner:** optimize single-chunk results and single-row lookups
([#18602](#18602))
([05b3775](05b3775))
* **spanner:** prune lock and begin event allocations on single-use
snapshots
([#18420](#18420))
([ab56473](ab56473))
* **spanner:** short-circuit trace_call when tracing is inactive
([#18356](#18356))
([58031be](58031be))
* **spanner:** support PartialResultSet.last with background stream
draining
([#18320](#18320))
([80efd63](80efd63))


### Documentation

* fix typos
([#18979](#18979))
([5c8baeb](5c8baeb))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sakthivelmanii sakthivelmanii sakthivelmanii approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@olavloite @sakthivelmanii