Skip to content

fix(sessions): Prevent MissingGreenlet after append_event with asyncpg#5814

Open
kkj333 wants to merge 1 commit into
google:mainfrom
kkj333:fix/database-session-post-commit-orm-read-5761
Open

fix(sessions): Prevent MissingGreenlet after append_event with asyncpg#5814
kkj333 wants to merge 1 commit into
google:mainfrom
kkj333:fix/database-session-post-commit-orm-read-5761

Conversation

@kkj333
Copy link
Copy Markdown

@kkj333 kkj333 commented May 22, 2026

Summary

  • Capture last_update_time and _storage_update_marker in DatabaseSessionService.append_event before commit(), instead of reading storage_session.update_time afterward
  • Add regression test ensuring storage revision fields are not read after commit completes

Fixes #5761

Problem

With postgresql+asyncpg:// and default pool_pre_ping=True, post-commit ORM access to storage_session.update_time can lazy-load an expired column (due to onupdate=func.now()), triggering sqlalchemy.exc.MissingGreenlet during asyncpg pool pre-ping.

Fix approach

Follows maintainer guidance on #5761: read get_update_timestamp() / get_update_marker() before commit, then assign locals to the in-memory session.

Test plan

  • uv run pytest tests/unittests/sessions/test_session_service.py::test_append_event_reads_storage_revision_before_commit -q
  • uv run pytest tests/unittests/sessions/test_session_service.py -k "append_event or last_update_time" -q (26 passed)

Manual verification (asyncpg + Postgres)

  • Env: postgresql+asyncpg://..., default pool_pre_ping=True, Postgres 16 (local docker compose)
  • Fix branch: append_event × 100 → PASS (0 MissingGreenlet)
  • main repro: post-commit ORM read of update_time after session.expire(..., ["update_time"]) → reproduces MissingGreenlet (lazy load → pool_pre_pingasyncpg.ping())
  • Fix pattern: reading revision fields before commit survives forced attribute expiry
  • Note: a simple append_event loop alone did not naturally trigger attribute expiry on main; the repro requires the post-commit lazy-load path described above

Post-commit reads of storage_session.update_time lazy-load an expired
column and trigger asyncpg pool_pre_ping outside SQLAlchemy's greenlet
bridge. Capture revision fields before commit instead.

Fixes google#5761
@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label May 22, 2026
@kkj333
Copy link
Copy Markdown
Author

kkj333 commented May 23, 2026

Manual asyncpg verification added to the PR test plan.

Tested locally against Postgres 16 with postgresql+asyncpg:// and default pool_pre_ping=True:

  • On this branch, append_event × 100 completed with 0 MissingGreenlet.
  • On main, I could reproduce MissingGreenlet by exercising the post-commit lazy-load path (expire update_time after commit, then read via ORM). The stack matches the issue: lazy load → pool pre-ping → asyncpg.ping().
  • The pre-commit read pattern in this PR avoids that IO after commit.

A plain append loop did not naturally expire update_time on main, so the forced lazy-load probe was needed to confirm the root cause in asyncpg. Unit tests cover the structural fix via _CommitOrderSpySession.

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

Labels

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.append_event raises MissingGreenlet with asyncpg after client disconnect (regression from da73e718ef)

2 participants