From 78de46e5fea488381104b4d9b20895018a9a4315 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 15:36:35 +0300 Subject: [PATCH] perf: run the single-statement deck writes at AUTOCOMMIT POST /decks and PUT /decks/{id} each send one statement, so a BEGIN and COMMIT around it only add two round trips. They now use an AUTOCOMMIT Transaction, the same pattern chat-app uses for its single-statement reads. --- AGENTS.md | 1 + app/ioc.py | 16 +++++++- app/resources/db.py | 5 +++ tests/conftest.py | 2 + tests/test_autocommit_writes.py | 71 +++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 tests/test_autocommit_writes.py diff --git a/AGENTS.md b/AGENTS.md index d8c05b0..719f8a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,6 +47,7 @@ CI (`.github/workflows/main.yml`) runs `ruff format --check`, `ruff check --no-f - `dynamic_engine` — `Scope.REQUEST`; `choose_sa_engine` returns the read engine for `GET`/`HEAD` requests, the primary otherwise (including when there is no request). GET handlers cannot write, and a read right after a write may see replica lag. - `session` — `Scope.REQUEST`, bound to `dynamic_engine`, finalized by `close_session`. - `transaction` — `Scope.REQUEST`, a `db_retry.Transaction` over `session`. Leaving its block rolls back anything uncommitted and closes the session. +- `autocommit_transaction` — the same `Transaction`, opened at `AUTOCOMMIT` (`autocommit_isolation_level`). `CreateDeckUseCase` and `UpdateDeckUseCase` use it because each sends exactly one statement, which Postgres runs atomically on its own. The asyncpg adapter then sends no BEGIN or COMMIT, so those writes cost one round trip instead of three. A use case with two or more statements, or one that relies on rolling back after a check, as `UpdateCardsUseCase` does, keeps `transaction`. The `db_session` fixture overrides `autocommit_isolation_level` to `None`, because switching the shared test connection to `AUTOCOMMIT` would roll back its outer transaction. `tests/test_autocommit_writes.py` runs both use cases against a real engine instead. `Repositories` (`Scope.REQUEST`): `decks_repository` and `cards_repository`, on `Database.session` with `auto_commit=False`. The use case commits through `transaction.commit()`. diff --git a/app/ioc.py b/app/ioc.py index 290e5a7..6449a0e 100644 --- a/app/ioc.py +++ b/app/ioc.py @@ -5,6 +5,7 @@ from app.repositories import CardsRepository, DecksRepository from app.resources.db import ( + autocommit_isolation_level, choose_sa_engine, close_sa_engine, close_session, @@ -49,6 +50,13 @@ class Database(Group): creator=Transaction, kwargs={"session": session}, ) + autocommit_isolation_level = providers.Factory(creator=autocommit_isolation_level, bound_type=None) + autocommit_transaction = providers.Factory( + scope=Scope.REQUEST, + creator=Transaction, + bound_type=None, + kwargs={"session": session, "isolation_level": autocommit_isolation_level}, + ) class Repositories(Group, scope=Scope.REQUEST): @@ -65,8 +73,12 @@ class Repositories(Group, scope=Scope.REQUEST): class UseCases(Group, scope=Scope.REQUEST): list_decks_use_case = providers.Factory(creator=ListDecksUseCase) get_deck_use_case = providers.Factory(creator=GetDeckUseCase) - update_deck_use_case = providers.Factory(creator=UpdateDeckUseCase) - create_deck_use_case = providers.Factory(creator=CreateDeckUseCase) + update_deck_use_case = providers.Factory( + creator=UpdateDeckUseCase, kwargs={"transaction": Database.autocommit_transaction} + ) + create_deck_use_case = providers.Factory( + creator=CreateDeckUseCase, kwargs={"transaction": Database.autocommit_transaction} + ) list_cards_use_case = providers.Factory(creator=ListCardsUseCase) get_card_use_case = providers.Factory(creator=GetCardUseCase) create_cards_use_case = providers.Factory(creator=CreateCardsUseCase) diff --git a/app/resources/db.py b/app/resources/db.py index ab56ace..8747e32 100644 --- a/app/resources/db.py +++ b/app/resources/db.py @@ -3,6 +3,7 @@ import typing import fastapi +from sqlalchemy.engine.interfaces import IsolationLevel from sqlalchemy.engine.url import URL, make_url from sqlalchemy.ext import asyncio as sa @@ -39,6 +40,10 @@ def create_read_sa_engine() -> sa.AsyncEngine: ) +def autocommit_isolation_level() -> IsolationLevel | None: + return "AUTOCOMMIT" + + async def close_sa_engine(engine: sa.AsyncEngine) -> None: await engine.dispose() diff --git a/tests/conftest.py b/tests/conftest.py index 5c38fae..979dc7b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -53,6 +53,8 @@ async def db_session(di_container: modern_di.Container) -> typing.AsyncIterator[ connection = await engine.connect() transaction = await connection.begin() di_container.override(ioc.Database.dynamic_engine, connection) + # Switching the shared connection to AUTOCOMMIT would roll back the outer transaction. + di_container.override(ioc.Database.autocommit_isolation_level, None) try: yield create_session(connection) diff --git a/tests/test_autocommit_writes.py b/tests/test_autocommit_writes.py new file mode 100644 index 0000000..ee2ded5 --- /dev/null +++ b/tests/test_autocommit_writes.py @@ -0,0 +1,71 @@ +import contextlib +import typing +import uuid + +import modern_di +import pytest +import sqlalchemy as sa +from advanced_alchemy.exceptions import NotFoundError +from sqlalchemy import event + +from app import ioc, models, schemas +from app.resources.db import create_primary_sa_engine + + +if typing.TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncEngine + + +@pytest.fixture +async def primary_engine(di_container: modern_di.Container) -> typing.AsyncIterator[AsyncEngine]: + engine = create_primary_sa_engine() + di_container.override(ioc.Database.dynamic_engine, engine) + try: + yield engine + finally: + di_container.reset_override(ioc.Database.dynamic_engine) + await engine.dispose() + + +@contextlib.contextmanager +def record_in_transaction(engine: AsyncEngine) -> typing.Iterator[list[bool]]: + in_transaction: typing.Final[list[bool]] = [] + + def record(connection: sa.Connection, *_: object) -> None: + driver_connection = connection.connection.driver_connection + assert driver_connection is not None + in_transaction.append(driver_connection.is_in_transaction()) + + event.listen(engine.sync_engine, "after_cursor_execute", record) + try: + yield in_transaction + finally: + event.remove(engine.sync_engine, "after_cursor_execute", record) + + +async def test_create_deck_sends_no_begin_or_commit( + di_container: modern_di.Container, primary_engine: AsyncEngine +) -> None: + key: typing.Final = uuid.uuid4() + try: + with record_in_transaction(primary_engine) as in_transaction: + async with di_container.build_child_container(scope=modern_di.Scope.REQUEST) as container: + use_case = container.resolve_provider(ioc.UseCases.create_deck_use_case) + await use_case(data=schemas.DeckCreate(name="autocommit", idempotency_key=key)) + finally: + async with primary_engine.begin() as connection: + await connection.execute(sa.delete(models.Deck).where(models.Deck.idempotency_key == key)) + + assert in_transaction == [False] + + +async def test_update_deck_sends_no_begin_or_commit( + di_container: modern_di.Container, primary_engine: AsyncEngine +) -> None: + with record_in_transaction(primary_engine) as in_transaction: + async with di_container.build_child_container(scope=modern_di.Scope.REQUEST) as container: + use_case = container.resolve_provider(ioc.UseCases.update_deck_use_case) + with pytest.raises(NotFoundError): + await use_case(deck_id=-1, data=schemas.DeckUpdate(name="autocommit")) + + assert in_transaction == [False]