Skip to content

fix(spanner): stop deleting multiplexed sessions on rotation and close - #18332

Merged
olavloite merged 2 commits into
mainfrom
spanner-do-not-delete-mux-session
Oct 1, 2026
Merged

olavloite merged 2 commits into
mainfrom
spanner-do-not-delete-mux-session

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Multiplexed sessions in Cloud Spanner cannot and should not be deleted by client applications. Calling DeleteSession on a multiplexed session causes Cloud Spanner to return an INVALID_ARGUMENT error.

  • Remove old_session.delete() from _rotate_multiplexed_session() in both _async/database_sessions_manager.py and database_sessions_manager.py.
  • Remove session_to_delete.delete() from close() in both managers, simply clearing the session reference (self._multiplexed_session = None) and relying on Spanner's server-side session eviction once idle.
  • Update docstrings to reflect that multiplexed sessions are rotated rather than deleted and recreated.

@olavloite
olavloite requested a review from a team as a code owner September 10, 2026 16:34

@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 updates the database session managers (both sync and async) to stop deleting old multiplexed sessions during rotation and closure, instead simply rotating or clearing the reference. Docstrings and unit tests have been updated accordingly to reflect this change, including removing tests that handled deletion failures. The review feedback points out that two test names (test_maintain_multiplexed_session_swaps_before_deleting_old_session) are now misleading since deletion no longer occurs, and suggests renaming them to better match the new behavior.

Multiplexed sessions in Cloud Spanner cannot and should not be deleted
by client applications. Calling DeleteSession on a multiplexed session
causes Cloud Spanner to return an INVALID_ARGUMENT error. In addition,
calling delete() upon session rotation risks aborting in-flight queries
that are still actively using the previous session reference.

- Remove `old_session.delete()` from `_rotate_multiplexed_session()` in
  both `_async/database_sessions_manager.py` and
  `database_sessions_manager.py`.
- Remove `session_to_delete.delete()` from `close()` in both managers,
  simply clearing the session reference (`self._multiplexed_session = None`)
  and relying on Spanner's server-side session eviction once idle.
- Update docstrings to reflect that multiplexed sessions are rotated
  rather than deleted and recreated.
@olavloite
olavloite force-pushed the spanner-do-not-delete-mux-session branch from 8c0c1e1 to da435da Compare September 10, 2026 16:47
@olavloite

Copy link
Copy Markdown
Contributor Author

/gemini review

@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 updates the synchronous and asynchronous DatabaseSessionsManager implementations to stop explicitly deleting old multiplexed sessions during session rotation and manager closure. Instead, the manager now simply rotates the session reference, leaving the cleanup of old sessions to garbage collection or other mechanisms. Corresponding unit tests have been updated to verify that delete() is no longer called on these sessions. There are no review comments, and I have no additional feedback to provide.

@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 16, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 16, 2026
@olavloite
olavloite merged commit c38e960 into main Oct 1, 2026
48 checks passed
@olavloite
olavloite deleted the spanner-do-not-delete-mux-session branch October 1, 2026 07:56
parthea pushed a commit that referenced this pull request Nov 12, 2026
🤖 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants