Skip to content

Comments

fix(python): commit read-only sessions to avoid spurious rollbacks#408

Merged
davideme merged 1 commit intomainfrom
claude/infallible-lumiere
Feb 22, 2026
Merged

fix(python): commit read-only sessions to avoid spurious rollbacks#408
davideme merged 1 commit intomainfrom
claude/infallible-lumiere

Conversation

@davideme
Copy link
Owner

Summary

  • Every GET request opened an implicit PostgreSQL transaction that was rolled back when the SQLAlchemy session closed (via async with session_factory() as sessionsession.close() → implicit rollback)
  • During benchmarks this appeared as ~200/s rollbacks vs ~20/s commits, even though no data was corrupted
  • Fix wraps the session yield in try/commit/except/rollback so read-only transactions are committed rather than rolled back

How it works

Operation Before After
GET /v1/lamps rollback (no commit) commit (read-only tx, no data change)
GET /v1/lamps/{id} rollback (no commit) commit (read-only tx, no data change)
POST /v1/lamps commit (already done in repo) commit (outer commit is no-op)
PUT /v1/lamps/{id} commit (already done in repo) commit (outer commit is no-op)
DELETE /v1/lamps/{id} commit (already done in repo) commit (outer commit is no-op)

Note: the create() missing-commit bug was separately fixed in #407.

Test plan

  • Run existing Python test suite: cd src/python && poetry run pytest
  • Run a benchmark and verify the rollback/commit ratio normalises

🤖 Generated with Claude Code

Every GET request opened an implicit PostgreSQL transaction that was
rolled back when the session closed, generating ~200/s rollbacks visible
in transaction count metrics during benchmarks.

Wrapping the session yield in try/commit/except/rollback ensures:
- Read-only transactions are committed (no-op for data, counts as commit
  in pg metrics instead of rollback)
- Write operations that already committed inside the repository have no
  active transaction left, so the outer commit is a no-op
- Exceptions still trigger an explicit rollback and re-raise

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 22, 2026 20:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes an issue where read-only database operations (GET requests) were being rolled back instead of committed, causing spurious rollback metrics during benchmarking (~200/s rollbacks vs ~20/s commits). The fix wraps the session yield in a try/commit/except/rollback pattern so that all successful transactions, including read-only ones, are properly committed.

Changes:

  • Modified the get_session method in DatabaseManager to explicitly commit successful transactions and rollback on exceptions
  • This change affects all database operations routed through FastAPI's dependency injection system

@davideme davideme merged commit a1b59e5 into main Feb 22, 2026
7 checks passed
@davideme davideme deleted the claude/infallible-lumiere branch February 22, 2026 21:11
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.

1 participant