(medium) Geocode cache get/put делает блокирующий DB-IO на event loop, хотя тот же файл использует to_thread для других DB-вызовов #1349

Closed
opened 2026-06-14 18:08:53 +00:00 by bot-backend · 1 comment
Collaborator

Описание

geocode()async def, await-ится из async-эндпоинтов (api/v1/geocode.py:31, admin.py:234). Внутри он вызывает _cache_get(db,...) и _cache_put(db,...) напрямую — оба выполняют синхронный psycopg db.execute_cache_put ещё и db.commit()) на event loop, блокируя его. Тот же дефект в reverse_geocode.

Корень

FastAPI НЕ выполняет код внутри тела async def-эндпоинта в threadpool — только sync endpoint-функции получают такой треатмент. Cache-путь блокирует loop на каждом geocode-запросе (cache-miss для _cache_get и после любого успешного lookup для _cache_put с его db.commit()). Непоследовательность доказана контрастом: в той же функции cadastral FDW-вызовы корректно обёрнуты в await asyncio.to_thread(_cadastral_forward_sync, ...) — автор знал про offload, но пропустил cache-хелперы. core/db.py использует синхронный create_engine + sessionmaker → plain Session, инъектируемый в async-хендлеры.

Evidence

geocoder.py:626,642,659,671,684:

async def geocode(address: str, db: Session):
    ...
    cached = _cache_get(db, addr_norm)        # блокирующий, на loop
    ...
    cad_suggestions = await asyncio.to_thread(_cadastral_forward_sync, db, ...)  # корректно offloaded
    ...
    _cache_put(db, addr_norm, result)         # блокирующий + db.commit(), на loop

Suggested fix

Обернуть _cache_get/_cache_put в asyncio.to_thread(...) как cadastral-хелперы, либо сделать geocode синхронной и диспатчить целиком через run_in_executor на call-site.

Обоснование severity

medium: индексированный single-row lookup быстр в норме, но db.commit() (возможный fsync) и конкурентные запросы вызывают stall-ы event loop и latency-спайки. Не crash-уровень, но реальный async-дефект, непоследовательный с дизайном окружающего кода. confidence 0.93.


week ревью 1 · multi-agent аудит 2026-06-14 · severity=medium · category=async-concurrency

Затронутые файлы:

  • /Users/anton/Птица/gendesign/tradein-mvp/backend/app/services/geocoder.py
## Описание `geocode()` — `async def`, await-ится из async-эндпоинтов (api/v1/geocode.py:31, admin.py:234). Внутри он вызывает `_cache_get(db,...)` и `_cache_put(db,...)` напрямую — оба выполняют синхронный psycopg `db.execute` (а `_cache_put` ещё и `db.commit()`) на event loop, блокируя его. Тот же дефект в `reverse_geocode`. ## Корень FastAPI НЕ выполняет код внутри тела `async def`-эндпоинта в threadpool — только sync *endpoint-функции* получают такой треатмент. Cache-путь блокирует loop на каждом geocode-запросе (cache-miss для `_cache_get` и после любого успешного lookup для `_cache_put` с его `db.commit()`). Непоследовательность доказана контрастом: в той же функции cadastral FDW-вызовы корректно обёрнуты в `await asyncio.to_thread(_cadastral_forward_sync, ...)` — автор знал про offload, но пропустил cache-хелперы. core/db.py использует синхронный `create_engine` + `sessionmaker` → plain `Session`, инъектируемый в async-хендлеры. ## Evidence `geocoder.py:626,642,659,671,684`: ```python async def geocode(address: str, db: Session): ... cached = _cache_get(db, addr_norm) # блокирующий, на loop ... cad_suggestions = await asyncio.to_thread(_cadastral_forward_sync, db, ...) # корректно offloaded ... _cache_put(db, addr_norm, result) # блокирующий + db.commit(), на loop ``` ## Suggested fix Обернуть `_cache_get`/`_cache_put` в `asyncio.to_thread(...)` как cadastral-хелперы, либо сделать `geocode` синхронной и диспатчить целиком через `run_in_executor` на call-site. ## Обоснование severity medium: индексированный single-row lookup быстр в норме, но `db.commit()` (возможный fsync) и конкурентные запросы вызывают stall-ы event loop и latency-спайки. Не crash-уровень, но реальный async-дефект, непоследовательный с дизайном окружающего кода. confidence 0.93. --- _week ревью 1 · multi-agent аудит 2026-06-14 · severity=**medium** · category=`async-concurrency`_ Затронутые файлы: - `/Users/anton/Птица/gendesign/tradein-mvp/backend/app/services/geocoder.py`
bot-backend added the
week ревью 1
label 2026-06-14 18:08:53 +00:00
bot-backend added the
scope/backend
label 2026-06-15 11:01:50 +00:00
Owner

🔍 week ревью 1 — верификация (ПОДТВЕРЖДЕНО, с уточнением по reverse_geocode)

Текущее состояние кода: tradein-mvp/backend/app/services/geocoder.py:610 (async def geocode(address, db: Session)), вызовы _cache_get на :626, _cache_put на :642, 659, 671, 684. Хелперы синхронны: _cache_get (83) делает db.execute (84); _cache_put (106) — db.execute (107) + db.commit() (132).

Корень: FastAPI НЕ оффлоудит тело async def-эндпоинта в threadpool (это делается только для sync endpoint-функций). geocode await-ится из async-эндпоинтов, а cache-хелперы выполняют блокирующий psycopg db.execute + db.commit() (возможный fsync) прямо на event loop — stall на каждом cache-miss (_cache_get) и после любого успешного lookup (_cache_put). Непоследовательность доказана контрастом: в той же функции cadastral-вызовы корректно обёрнуты — await asyncio.to_thread(_cadastral_forward_sync, db, ...) (:632), и в reverse_geocode тоже (asyncio.to_thread(_cadastral_reverse_sync_full, ...) на :952). Автор знал про offload, но пропустил cache-хелперы.

Уточнение к телу issue: «тот же дефект в reverse_geocode» — НЕ подтверждается буквально: reverse_geocode (:935) в текущем коде НЕ вызывает _cache_get/_cache_put вовсе (нет cache-пути), так что блокирующего cache-IO там нет. Дефект локализован в geocode.

Что править:

  • Обернуть оба cache-хелпера в threadpool: cached = await asyncio.to_thread(_cache_get, db, addr_norm); await asyncio.to_thread(_cache_put, db, addr_norm, result) на всех четырёх call-site (642, 659, 671, 684) — единообразно с cadastral-хелперами.
  • Альтернатива: сделать geocode синхронной и диспатчить целиком через run_in_executor на call-site эндпоинта.

Что посмотреть:

  • Код: geocoder.py:610 (geocode), :83-104 (_cache_get), :106-132 (_cache_put + commit), :626, 642, 659, 671, 684 (call-sites), :632, 952 (корректный to_thread-паттерн). Call-sites эндпоинтов: api/v1/geocode.py, api/v1/admin.py. core/db.py — sync create_engine/sessionmaker → plain Session инъектируется в async-хендлеры.
  • Vault: релевантных заметок не найдено (geocoder cache async to_thread event loop blocking — пусто).
**🔍 week ревью 1 — верификация (ПОДТВЕРЖДЕНО, с уточнением по reverse_geocode)** **Текущее состояние кода:** `tradein-mvp/backend/app/services/geocoder.py:610` (`async def geocode(address, db: Session)`), вызовы `_cache_get` на `:626`, `_cache_put` на `:642, 659, 671, 684`. Хелперы синхронны: `_cache_get` (83) делает `db.execute` (84); `_cache_put` (106) — `db.execute` (107) + `db.commit()` (132). **Корень:** FastAPI НЕ оффлоудит тело `async def`-эндпоинта в threadpool (это делается только для sync endpoint-функций). `geocode` await-ится из async-эндпоинтов, а cache-хелперы выполняют блокирующий psycopg `db.execute` + `db.commit()` (возможный fsync) прямо на event loop — stall на каждом cache-miss (`_cache_get`) и после любого успешного lookup (`_cache_put`). Непоследовательность доказана контрастом: в той же функции cadastral-вызовы корректно обёрнуты — `await asyncio.to_thread(_cadastral_forward_sync, db, ...)` (:632), и в `reverse_geocode` тоже (`asyncio.to_thread(_cadastral_reverse_sync_full, ...)` на :952). Автор знал про offload, но пропустил cache-хелперы. **Уточнение к телу issue:** «тот же дефект в reverse_geocode» — НЕ подтверждается буквально: `reverse_geocode` (:935) в текущем коде НЕ вызывает `_cache_get`/`_cache_put` вовсе (нет cache-пути), так что блокирующего cache-IO там нет. Дефект локализован в `geocode`. **Что править:** - Обернуть оба cache-хелпера в threadpool: `cached = await asyncio.to_thread(_cache_get, db, addr_norm)`; `await asyncio.to_thread(_cache_put, db, addr_norm, result)` на всех четырёх call-site (642, 659, 671, 684) — единообразно с cadastral-хелперами. - Альтернатива: сделать `geocode` синхронной и диспатчить целиком через `run_in_executor` на call-site эндпоинта. **Что посмотреть:** - Код: `geocoder.py:610` (`geocode`), `:83-104` (`_cache_get`), `:106-132` (`_cache_put` + commit), `:626, 642, 659, 671, 684` (call-sites), `:632, 952` (корректный `to_thread`-паттерн). Call-sites эндпоинтов: `api/v1/geocode.py`, `api/v1/admin.py`. core/db.py — sync `create_engine`/`sessionmaker` → plain `Session` инъектируется в async-хендлеры. - Vault: релевантных заметок не найдено (`geocoder cache async to_thread event loop blocking` — пусто).
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#1349
No description provided.