lekss361/gendesign

Fork 0

MERA: на /trade-in/estimate нет ограничителя одновременности — рейт-лимит меряет частоту, а не параллелизм #3082

New issue

Closed

opened 2026-08-24 16:55:17 +00:00 by bot-backend · 4 comments

bot-backend commented

2026-08-24 16:55:17 +00:00

Collaborator

Симптом

Секундная публичная ручка автодополнения прикрыта семафором: 4 слота, ожидание слота 2.0s, отбой 429 (backend/app/api/public/mera.py:129-131, :249-256, release :268). У тяжёлой ручки оценки такого нет: async def estimate (backend/app/api/v1/trade_in.py:408-409) работает без ограничителя одновременности.

По всему tradein-mvp/backend Semaphore есть ровно в одном месте — public/mera.py:131. На пути /estimate ни семафора, ни очереди, ни пула: только покалльные таймауты _with_budget (services/estimator.py:2689).

Прод — один процесс uvicorn, единственный event loop (tradein-mvp/docker-compose.prod.yml:281, комментарий :276-280). Роут async, то есть расчёт (await estimate_quality, trade_in.py:437 → services/estimator.py:3852) идёт в самом лупе и держит соединение общего пула. Пила одновременных оценок выедает пул и внешние тиры и тормозит весь /api/v1/*.

Пути ниже — от tradein-mvp/.

Почему так

Два существующих ограничителя /estimate меряют не то.

Рейт-лимит — это ЧАСТОТА, а не одновременность. RateLimitMiddleware (core/ratelimit.py:49, dispatch :55, окно :91-96, подключена в app/main.py:240) — 300 запросов / 60.0s (config.py:318-319), ключ username либо IP (:82-85). Все 300 вправе стартовать в одну секунду.

Квота — счётная и помесячная. account_quota (services/account_quota.py:42, check_and_raise:160, атомарный increment:195 из trade_in.py:462) — 15 успешных оценок в месяц на аккаунт (config.py:751). Сто аккаунтов дадут сто параллельных расчётов, не нарушив квоту.

Вбок семафор не наследуется: публичная ручка делегирует в app/api/v1/geocode.suggest_addresses, у которой своего ограничителя нет (public/mera.py:261).

Что сделать

Завести asyncio.Semaphore на пути /estimate (trade_in.py:409) по образцу public/mera.py:129-131: wait_for(acquire) с коротким ожиданием слота, timeout → 429 (mera.py:249-256), release в finally (mera.py:268)
Число слотов согласовать с пулом SQLAlchemy и уже занятыми 4 слотами suggest — мотив тот же, что в mera.py:117-127: внешний тир держит соединение общего пула
Слить три независимых внешних вызова в asyncio.gather: estimator.py:4337 (IMV), :4355 (Yandex Valuation, _with_budget), :4404 (Cian Valuation, _with_budget). Входы у всех готовые — geo.full_address + payload, зависимостей между ними нет, латентности сейчас складываются. :4372 зависимый, его не сливать
Отдельным решением: нужен ли ограничитель на v1/geocode.suggest_addresses (public/mera.py:261)

Как проверить

grep по tradein-mvp/backend: Semaphore появился на пути /estimate, не только в public/mera.py:131
N параллельных POST /api/v1/trade-in/estimate: сверх лимита лишние получают 429 быстро, а не висят; в пределах лимита поведение прежнее, account_quota.py:195 считает только успешные
под нагрузкой p95 остальных /api/v1/* не растёт линейно от числа параллельных оценок
тайминги в логах: блок 4337 / 4355 / 4404 ≈ максимум из трёх, а не сумма

## Симптом Секундная публичная ручка автодополнения прикрыта семафором: 4 слота, ожидание слота 2.0s, отбой 429 (`backend/app/api/public/mera.py:129-131`, `:249-256`, release `:268`). У тяжёлой ручки оценки такого нет: `async def estimate` (`backend/app/api/v1/trade_in.py:408-409`) работает без ограничителя одновременности. По всему `tradein-mvp/backend` `Semaphore` есть ровно в одном месте — `public/mera.py:131`. На пути `/estimate` ни семафора, ни очереди, ни пула: только покалльные таймауты `_with_budget` (`services/estimator.py:2689`). Прод — один процесс uvicorn, единственный event loop (`tradein-mvp/docker-compose.prod.yml:281`, комментарий `:276-280`). Роут `async`, то есть расчёт (`await estimate_quality`, `trade_in.py:437` → `services/estimator.py:3852`) идёт в самом лупе и держит соединение общего пула. Пила одновременных оценок выедает пул и внешние тиры и тормозит весь `/api/v1/*`. Пути ниже — от `tradein-mvp/`. ## Почему так Два существующих ограничителя `/estimate` меряют не то. **Рейт-лимит — это ЧАСТОТА, а не одновременность.** `RateLimitMiddleware` (`core/ratelimit.py:49`, dispatch `:55`, окно `:91-96`, подключена в `app/main.py:240`) — 300 запросов / 60.0s (`config.py:318-319`), ключ username либо IP (`:82-85`). Все 300 вправе стартовать в одну секунду. **Квота — счётная и помесячная.** `account_quota` (`services/account_quota.py:42`, `check_and_raise:160`, атомарный `increment:195` из `trade_in.py:462`) — 15 успешных оценок в месяц на аккаунт (`config.py:751`). Сто аккаунтов дадут сто параллельных расчётов, не нарушив квоту. Вбок семафор не наследуется: публичная ручка делегирует в `app/api/v1/geocode.suggest_addresses`, у которой своего ограничителя нет (`public/mera.py:261`). ## Что сделать - [ ] Завести `asyncio.Semaphore` на пути `/estimate` (`trade_in.py:409`) по образцу `public/mera.py:129-131`: `wait_for(acquire)` с коротким ожиданием слота, timeout → 429 (`mera.py:249-256`), `release` в `finally` (`mera.py:268`) - [ ] Число слотов согласовать с пулом SQLAlchemy и уже занятыми 4 слотами `suggest` — мотив тот же, что в `mera.py:117-127`: внешний тир держит соединение общего пула - [ ] Слить три независимых внешних вызова в `asyncio.gather`: `estimator.py:4337` (IMV), `:4355` (Yandex Valuation, `_with_budget`), `:4404` (Cian Valuation, `_with_budget`). Входы у всех готовые — `geo.full_address` + payload, зависимостей между ними нет, латентности сейчас складываются. `:4372` зависимый, его не сливать - [ ] Отдельным решением: нужен ли ограничитель на `v1/geocode.suggest_addresses` (`public/mera.py:261`) ## Как проверить - [ ] grep по `tradein-mvp/backend`: `Semaphore` появился на пути `/estimate`, не только в `public/mera.py:131` - [ ] N параллельных `POST /api/v1/trade-in/estimate`: сверх лимита лишние получают 429 быстро, а не висят; в пределах лимита поведение прежнее, `account_quota.py:195` считает только успешные - [ ] под нагрузкой p95 остальных `/api/v1/*` не растёт линейно от числа параллельных оценок - [ ] тайминги в логах: блок `4337 / 4355 / 4404` ≈ максимум из трёх, а не сумма

bot-backend added the

labels 2026-08-24 16:55:17 +00:00

bot-backend referenced this issue

2026-08-24 16:55:49 +00:00

MERA: бэкенд оценки крутится в один воркер uvicorn — тяжёлый /estimate внутри event loop блокирует всех #3083

bot-backend commented

2026-08-24 16:55:49 +00:00

Author

Collaborator

Смежная: #3083 — один воркер uvicorn. Задачи надо согласовать: семафор отсюда живёт в памяти процесса, поэтому при N воркерах фактический лимит умножается на N.

Light1YT referenced this issue from a commit

2026-08-26 07:30:00 +00:00

feat(tradein/estimate): потолок одновременных оценок — 4 слота, быстрый 429 (#3082)

bot-backend referenced this issue

2026-08-26 07:30:02 +00:00

feat(tradein/estimate): потолок одновременных оценок — 4 слота, быстрый 429 (#3082) #3097

bot-backend referenced this issue from a commit

2026-08-26 07:36:40 +00:00

Merge pull request 'feat(tradein/estimate): потолок одновременных оценок — 4 слота, быстрый 429 (#3082)' (#3097) from fix/3082-estimate-concurrency into main

bot-backend commented

2026-08-26 07:45:44 +00:00

Author

Collaborator

Семафор на проде с 26.08 ~07:55 UTC (PR #3097, деплой `929eff11` зелёный, код в контейнере проверен)

asyncio.Semaphore(4) на пути /estimate по образцу public/mera.py: acquire после дешёвых отказов (рейт-лимит, квота pre-check), ожидание слота 5с ≈ две длительности оценки, дальше быстрый 429 + Retry-After; release в finally сразу после дорогой части. 4+4 слота (estimate+suggest) = 8 удерживаемых соединений из 15 пула.

По чеклисту:

семафор на пути /estimate — live, тест: 4 висящих оценки → 5-я быстрый 429, после освобождения слоты возвращаются (красный на main по значению)
слоты согласованы с пулом и suggest (расчёт в комментарии у семафора)
asyncio.gather трёх внешних вызовов — сознательно НЕ сделано, и вот находка: все три (_get_or_fetch_imv_cached :4337, yandex :4355, cian :4404) делят один SQLAlchemy Session, и часть работы внутри уходит в asyncio.to_thread (например _save_yandex_history_items). Параллелить их без разбора всех трёх деревьев вызовов = конкурентный доступ к сессии из разных потоков — ровно ловушка session-poisoning из #2464. Если делать — то после аудита каждого дерева на предмет «кто трогает db и из какого потока», либо каждому вызову свою сессию.
ограничитель на v1/geocode.suggest_addresses — «отдельным решением» по тексту задачи, не трогал

Согласование с #3083 задокументировано прямо у семафора: лимит per-process, при N воркерах умножается на N.

Приёмка под реальной нагрузкой (429 сверх лимита, p95 соседних ручек) — на первом же всплеске параллельных оценок; синтетический прогон на проде гонять не стал, чтобы не жечь внешние тиры.

## Семафор на проде с 26.08 ~07:55 UTC (PR #3097, деплой 929eff11 зелёный, код в контейнере проверен) `asyncio.Semaphore(4)` на пути `/estimate` по образцу `public/mera.py`: acquire после дешёвых отказов (рейт-лимит, квота pre-check), ожидание слота 5с ≈ две длительности оценки, дальше быстрый 429 + Retry-After; release в `finally` сразу после дорогой части. 4+4 слота (estimate+suggest) = 8 удерживаемых соединений из 15 пула. По чеклисту: - [x] семафор на пути `/estimate` — live, тест: 4 висящих оценки → 5-я быстрый 429, после освобождения слоты возвращаются (красный на main по значению) - [x] слоты согласованы с пулом и suggest (расчёт в комментарии у семафора) - [ ] **`asyncio.gather` трёх внешних вызовов — сознательно НЕ сделано, и вот находка**: все три (`_get_or_fetch_imv_cached` :4337, yandex :4355, cian :4404) делят один SQLAlchemy `Session`, и часть работы внутри уходит в `asyncio.to_thread` (например `_save_yandex_history_items`). Параллелить их без разбора всех трёх деревьев вызовов = конкурентный доступ к сессии из разных потоков — ровно ловушка session-poisoning из #2464. Если делать — то после аудита каждого дерева на предмет «кто трогает db и из какого потока», либо каждому вызову свою сессию. - [ ] ограничитель на `v1/geocode.suggest_addresses` — «отдельным решением» по тексту задачи, не трогал Согласование с #3083 задокументировано прямо у семафора: лимит per-process, при N воркерах умножается на N. Приёмка под реальной нагрузкой (429 сверх лимита, p95 соседних ручек) — на первом же всплеске параллельных оценок; синтетический прогон на проде гонять не стал, чтобы не жечь внешние тиры.

bot-backend referenced this issue

2026-08-26 07:54:59 +00:00

fix(mera/ui): карточка прогресса перестала выдумывать стадии (#3081, вариант A) #3101

bot-backend referenced this issue

2026-08-26 08:01:28 +00:00

MERA: бэкенд оценки крутится в один воркер uvicorn — тяжёлый /estimate внутри event loop блокирует всех #3083

lekss361 commented

2026-08-26 16:44:40 +00:00

Owner

`asyncio.gather` — аудит проведён, предлагаю пункт снять: складывать нечего

В прошлый раз пункт отложили до разбора трёх деревьев вызовов. Разобрал. Ответ оказался не «опасно, но выгодно», а «выгоды в проде нет вовсе», и это меняет решение.

Замер на проде — вызовы уже не ходят наружу в запросе

estimate_external_sources_background = True

При этом флаге все три вызова идут с fetch_on_miss=False: промах кэша сразу возвращает None, а догрузка уходит в фон через _defer_external_refresh. То есть в пути запроса остаются три обращения к локальной кэш-таблице, а не три внешних HTTP.

Складывать нечего: латентности, которые пункт предлагал наложить друг на друга, из запроса уже вынесены — и вынесены более радикально, чем это сделал бы gather. Причём БД теперь локальная для бэкенда (оба на Poincare), так что это три быстрых SELECT.

Дефолт в репозитории — False (core/config.py:773), поэтому по коду пункт выглядел актуальным. Значение задаётся окружением на машине, ровно та ловушка, о которой предупреждает #3083 («рантайм-окружение пишется вручную на VPS, override оттуда из кода не виден»).

Опасность подтвердилась, но причина другая — не потоки, а общая транзакция

В прошлом комментарии я записал риск как конкурентный доступ к сессии из разных потоков (asyncio.to_thread). Разбор показал, что дело проще и серьёзнее.

Все три работают с синхронной сессией напрямую, вперемежку с await на HTTP:

Вызов	Что делает с `db`
`_get_or_fetch_imv_cached`	`db.execute` (чтение кэша), затем `await evaluate_via_imv(...)`
`_get_or_fetch_yandex_valuation_cached`	`db.execute` → `await fetch_house_history` → `db.execute` → `db.commit()`, на ошибке `db.rollback()`
`estimate_via_cian_valuation`	`load_session(db)` → `await session.get(...)` → `mark_session_invalid(db, ...)`

Сами db.execute() блокирующие и наложиться не могут. Опасно другое: под gather они чередуются на границах await, а сессия и транзакция у них одна. Пока яндексовая ветка ждёт HTTP, циановская может дойти до своей записи; чей-нибудь commit() зафиксирует чужую незавершённую работу, а rollback() — молча её выбросит. Ни исключения, ни лога.

Это не гипотеза: ровно так объясняет своё устройство сосед по файлу, _defer_external_refresh (estimator.py:883-885) — «своя сессия намеренно: обе функции источников делают внутри себя db.commit(), переиспользование чужой сессии зафиксировало бы её незавершённую работу». Фоновая догрузка получает SessionLocal(), и по той же причине gather на общей сессии недопустим.

Предложение

Снять пункт как рассмотренный и отклонённый, а не держать открытым: сегодня он даёт ноль выигрыша и ненулевой риск тихой порчи транзакции.

Если ESTIMATE_EXTERNAL_SOURCES_BACKGROUND когда-нибудь вернут в False, вопрос оживёт — и тогда правильная реализация уже известна: каждой ветке своя SessionLocal(), по образцу _defer_external_refresh, а не gather поверх общей. Записываю это здесь, чтобы следующий заход не начинал с нуля.

Состояние чеклиста

Семафор на пути /estimate — на проде с 26.08
Слоты согласованы с пулом (4+4 из 15) — пул подтверждён по коду: create_engine(...) в core/db.py:8 без pool_size/max_overflow, то есть дефолт 5+10. Раньше это число было известно только из комментария
~~asyncio.gather трёх внешних вызовов~~ — отклонено по замеру, см. выше
Ограничитель на v1/geocode.suggest_addresses — «отдельным решением», за владельцем

## `asyncio.gather` — аудит проведён, предлагаю пункт снять: складывать нечего В прошлый раз пункт отложили до разбора трёх деревьев вызовов. Разобрал. Ответ оказался не «опасно, но выгодно», а «выгоды в проде нет вовсе», и это меняет решение. ### Замер на проде — вызовы уже не ходят наружу в запросе ``` estimate_external_sources_background = True ``` При этом флаге все три вызова идут с `fetch_on_miss=False`: промах кэша **сразу** возвращает `None`, а догрузка уходит в фон через `_defer_external_refresh`. То есть в пути запроса остаются три обращения к локальной кэш-таблице, а не три внешних HTTP. Складывать нечего: латентности, которые пункт предлагал наложить друг на друга, из запроса уже вынесены — и вынесены более радикально, чем это сделал бы `gather`. Причём БД теперь локальная для бэкенда (оба на Poincare), так что это три быстрых SELECT. Дефолт в репозитории — `False` (`core/config.py:773`), поэтому по коду пункт выглядел актуальным. Значение задаётся окружением на машине, ровно та ловушка, о которой предупреждает #3083 («рантайм-окружение пишется вручную на VPS, override оттуда из кода не виден»). ### Опасность подтвердилась, но причина другая — не потоки, а общая транзакция В прошлом комментарии я записал риск как конкурентный доступ к сессии из разных потоков (`asyncio.to_thread`). Разбор показал, что дело проще и серьёзнее. Все три работают с **синхронной** сессией напрямую, вперемежку с `await` на HTTP: | Вызов | Что делает с `db` | |---|---| | `_get_or_fetch_imv_cached` | `db.execute` (чтение кэша), затем `await evaluate_via_imv(...)` | | `_get_or_fetch_yandex_valuation_cached` | `db.execute` → `await fetch_house_history` → `db.execute` → **`db.commit()`**, на ошибке **`db.rollback()`** | | `estimate_via_cian_valuation` | `load_session(db)` → `await session.get(...)` → `mark_session_invalid(db, ...)` | Сами `db.execute()` блокирующие и наложиться не могут. Опасно другое: под `gather` они **чередуются на границах `await`**, а сессия и транзакция у них одна. Пока яндексовая ветка ждёт HTTP, циановская может дойти до своей записи; чей-нибудь `commit()` зафиксирует чужую незавершённую работу, а `rollback()` — молча её выбросит. Ни исключения, ни лога. Это не гипотеза: ровно так объясняет своё устройство сосед по файлу, `_defer_external_refresh` (`estimator.py:883-885`) — «своя сессия намеренно: обе функции источников делают внутри себя `db.commit()`, переиспользование чужой сессии зафиксировало бы её незавершённую работу». Фоновая догрузка получает `SessionLocal()`, и по той же причине `gather` на общей сессии недопустим. ### Предложение **Снять пункт как рассмотренный и отклонённый**, а не держать открытым: сегодня он даёт ноль выигрыша и ненулевой риск тихой порчи транзакции. Если `ESTIMATE_EXTERNAL_SOURCES_BACKGROUND` когда-нибудь вернут в `False`, вопрос оживёт — и тогда правильная реализация уже известна: каждой ветке своя `SessionLocal()`, по образцу `_defer_external_refresh`, а не `gather` поверх общей. Записываю это здесь, чтобы следующий заход не начинал с нуля. ### Состояние чеклиста - [x] Семафор на пути `/estimate` — на проде с 26.08 - [x] Слоты согласованы с пулом (4+4 из 15) — пул подтверждён по коду: `create_engine(...)` в `core/db.py:8` без `pool_size`/`max_overflow`, то есть дефолт 5+10. Раньше это число было известно только из комментария - [x] ~~`asyncio.gather` трёх внешних вызовов~~ — **отклонено по замеру**, см. выше - [ ] Ограничитель на `v1/geocode.suggest_addresses` — «отдельным решением», за владельцем

lekss361 referenced this issue

2026-08-27 10:43:43 +00:00

fix(mera/ui): карта перестала перекрывать интерфейс, страница помещается в окно #3131

bot-backend commented

2026-08-27 11:01:45 +00:00

Author

Collaborator

Закрываю — все четыре пункта чеклиста получили исход

Семафор на /estimate — сделан, на проде с 26.08, принят (комментарий выше с тестами и расчётом слотов).
Слоты согласованы с пулом и suggest (4+4 из 15).
asyncio.gather — снят по твоему же аудиту (комментарий 26.08 16:44): при estimate_external_sources_background=True в пути запроса три локальных SELECT, складывать нечего — вынесено радикальнее, чем сделал бы gather. Моя session-poisoning-нота остаётся в истории как довод, почему не делали вслепую.
Ограничитель на suggest_addresses — «отдельным решением» по постановке; если понадобится — новая задача.

Согласование с #3083 задокументировано у семафора в коде.

## Закрываю — все четыре пункта чеклиста получили исход 1. **Семафор на `/estimate`** — сделан, на проде с 26.08, принят (комментарий выше с тестами и расчётом слотов). 2. **Слоты согласованы** с пулом и suggest (4+4 из 15). 3. **`asyncio.gather`** — снят по твоему же аудиту (комментарий 26.08 16:44): при `estimate_external_sources_background=True` в пути запроса три локальных SELECT, складывать нечего — вынесено радикальнее, чем сделал бы gather. Моя session-poisoning-нота остаётся в истории как довод, почему не делали вслепую. 4. **Ограничитель на `suggest_addresses`** — «отдельным решением» по постановке; если понадобится — новая задача. Согласование с #3083 задокументировано у семафора в коде.

bot-backend closed this issue

2026-08-27 11:01:46 +00:00