Skip to content

fix(sessions): use read-only transactions for get/list operations#4810

Open
giulio-leone wants to merge 1 commit intogoogle:mainfrom
giulio-leone:fix/session-service-readonly-transactions
Open

fix(sessions): use read-only transactions for get/list operations#4810
giulio-leone wants to merge 1 commit intogoogle:mainfrom
giulio-leone:fix/session-service-readonly-transactions

Conversation

@giulio-leone
Copy link

Summary

Fixes #4771

DatabaseSessionService.get_session() and list_sessions() are pure-read operations but currently open regular read-write transactions via _rollback_on_exception_session(). On Cloud Spanner this causes RetryAborted errors when a concurrent write commits during the read, because Spanner's OCC (Optimistic Concurrency Control) layer sees the read-write transaction as conflicting.

Root Cause

Spanner uses OCC for read-write transactions. Even a SELECT-only transaction opened in read-write mode will be aborted if a concurrent write touches the same data. Since reads never mutate data, they should use read-only transactions which are not subject to OCC aborts.

Fix

Added a _readonly_session() context manager that:

  • Acquires the underlying connection and sets execution_options(postgresql_readonly=True)
  • This option is also respected by the sqlalchemy-spanner dialect to open read-only transactions
  • Never commits (reads don't need commits), only rolls back on exception
  • Reuses the existing database_session_factory and connection pool

Switched get_session() and list_sessions() to use _readonly_session().

Write operations (create_session, delete_session, append_event) continue to use _rollback_on_exception_session() with explicit commits — unchanged.

Testing

All 128 session tests pass. Full suite: 4725 passed, 0 failures, 0 regressions.

DatabaseSessionService.get_session() and list_sessions() are pure-read
operations but currently open regular read-write transactions via
_rollback_on_exception_session(). On Cloud Spanner this causes
RetryAborted errors when a concurrent write commits during the read,
because Spanner's OCC layer sees the read-write transaction as
conflicting.

Add _readonly_session() context manager that:
 - Marks the connection as read-only (postgresql_readonly=True) which
   also benefits Cloud Spanner's sqlalchemy-spanner dialect
 - Never commits, avoiding unnecessary write-path overhead
 - Still rolls back on exception to release the connection cleanly

Switch get_session() and list_sessions() to use _readonly_session().
Write operations (create_session, delete_session, append_event) continue
to use _rollback_on_exception_session() with explicit commits.

Fixes google#4771

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@google-cla
Copy link

google-cla bot commented Mar 13, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Mar 13, 2026
@rohityan rohityan self-assigned this Mar 13, 2026
@rohityan rohityan added the request clarification [Status] The maintainer need clarification or more information from the author label Mar 13, 2026
@rohityan
Copy link
Collaborator

Hi @giulio-leone , Thank you for your contribution! It appears you haven't yet signed the Contributor License Agreement (CLA). Please visit https://cla.developers.google.com/ to complete the signing process. Once the CLA is signed, we'll be able to proceed with the review of your PR. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DatabaseSessionService: get_session / list_sessions use read-write transactions unnecessarily, causing RetryAborted on Spanner

3 participants