tradein: тот же потолок второму движку + тесты, которые ловят испорченное значение (#3463)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 4m45s
CI / changes (pull_request) Successful in 11s
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped

Правки по deep-ревью PR #3508.

1. HIGH. `app/core/auth_db.py` строил второй движок БЕЗ `connect_args`, а моё
обоснование пропуска было ложным: на проде `IDENTITY_STORE=auth` во всех трёх
сервисах образа и `AUTH_DB_PASSWORD` задан (сверено `printenv` в контейнерах),
то есть реестр живой. Путь горячий: `core/rbac.py` резолвит session-cookie в
middleware, синхронно на event loop'е, на каждом запросе с cookie — значит
`ACCESS EXCLUSIVE` на `auth.sessions` вешал бы не четыре слота `/estimate`, а
весь uvicorn-воркер (он один), включая `/health`. Потолок — та же константа
`DB_CONNECT_ARGS`: одна на оба движка, а не защита на одном и мина на втором.
Срабатывание безопасно — вызов уже под `except Exception` с фолбэком.

2. MEDIUM. Испорченный `options` (`statement_timeout=30000zz`) проходил ЗЕЛЁНЫМ:
статическая проверка искала ПОДСТРОКУ (а `…=30000` — префикс испорченного), а
живая глушила отказ коннекта голым `except` → skip → запись в allowlist. На
проде это `FATAL: invalid value for parameter` на КАЖДОМ коннекте, то есть
полный отказ продукта при зелёном сьюте. Теперь: сравнение `options` на
РАВЕНСТВО, и `_live_engine` сначала пробует коннект БЕЗ `connect_args` — сервера
нет это пропуск, а «сервер есть, наши options он не принял» это падение.

3. LOW. У проверки согласованности был пол и не было крыши: `300_000` (пять
минут) зеленел. Добавлена симметричная граница `<= 2 ×` самого длинного
объявленного бюджета.

4. LOW. Три факта в комментариях исправлены:
   * `pg_stat_statements` НЕ опора по планировщику — вытесняет записи с calls=1
     (`dealloc` вырос за десять минут, из топа пропал `REFRESH MATERIALIZED VIEW`
     30.85 с). Основная опора — `scrape_runs`;
   * самый длинный set-based statement через движок — матч ГАР→houses: 2.07 с с
     городским фильтром и 6.46 с без. Запас ~3×, а не 7×;
   * `idle in transaction` 29 с — это tgbot (`services/tgbot/bridge.py`,
     транзакция поверх long-poll Telegram; сверено 3 пробами: одна и та же
     сессия, `SELECT value FROM tg_support_state …`), а не свипы. Решение не
     ставить потолок на простой от этого только крепче.

Мутационная проверка (обе лэйны краснеют на каждой): испорченный `options`,
`_STATEMENT_TIMEOUT_MS = 300_000`, снятый `connect_args`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bot-backend 2026-09-12 20:35:38 +05:00
parent a8503d3a58
commit 70bb5a3a8f
3 changed files with 68 additions and 24 deletions

View file

@ -47,6 +47,7 @@ from sqlalchemy.exc import ArgumentError
from sqlalchemy.orm import Session, sessionmaker
from app.core.config import settings
from app.core.db import DB_CONNECT_ARGS
class AuthDatabaseNotConfiguredError(RuntimeError):
@ -101,6 +102,18 @@ def _build() -> tuple[Engine, sessionmaker[Session]]:
# НЕ закрывает: текст ошибки самого драйвера (Postgres DETAIL со значением)
# и сырые psycopg-подключения мимо движков — это отдельный класс.
hide_parameters=True,
# #3463. Те же потолки, что у продуктового движка, — ОДНОЙ константой на оба:
# потолок на одном движке и мина на втором это не починка, а половина.
# Этот движок живёт на ГОРЯЧЕМ пути: `core/rbac.py` резолвит session-cookie
# в middleware, синхронно на event loop'е, на КАЖДОМ запросе с cookie
# (на проде IDENTITY_STORE=auth во всех трёх сервисах образа — сверено 12.09,
# `printenv` в контейнерах). Без потолка `ACCESS EXCLUSIVE` на `auth.sessions`
# вешает не четыре слота `/estimate`, а весь uvicorn-воркер (он один, без
# --workers) — включая `/health`.
# Срабатывание потолка безопасно: вызов в rbac.py уже под `except Exception`
# с фолбэком на заголовочную аутентификацию, то есть отмена запроса даёт тот
# же путь, что и любой другой сбой реестра, а не 500.
connect_args=DB_CONNECT_ARGS,
)
except (ArgumentError, ValueError):
# ValueError — не паранойя: на «почти URL» разбор SQLAlchemy доходит до

View file

@ -20,15 +20,19 @@ logger = logging.getLogger(__name__)
# честной работы, но остался конечным:
# * самый длинный ОБЪЯВЛЕННЫЙ бюджет на `/estimate` — 20 с (`estimate_avito_imv_timeout_s`,
# config.py:852); дальше 12 с геокод, 8 с Yandex/Cian/house_meta. 30 с = 1.5× от максимума;
# * замер на боевой БД 12.09 (`pg_stat_statements`, накоплено с 27.08): САМЫЙ долгий
# запрос через этот движок — 4.27 с; всего 2 запроса длиннее 5 с за 16 суток, и оба
# мимо движка (`REFRESH MATERIALIZED VIEW CONCURRENTLY` 30.85 с — своё сырое
# psycopg-соединение в tasks/refresh_search_matview.py; `COPY _stg` 19.67 с — psql
# из scripts/local-avito-msk/collect.py);
# * самая долгая ЧИСТО-БД задача планировщика по `scrape_runs` за 14 суток —
# listing_source_snapshot, 9.7 с ЦЕЛИКОМ (и у неё сверх того свой
# `SET LOCAL statement_timeout = 900000`, который перекрывает это значение —
# гейт tests/test_3463_db_timeouts.py).
# * ОСНОВНАЯ опора по планировщику — `scrape_runs` (длительности целых прогонов, они
# не вытесняются): самая долгая ЧИСТО-БД задача за 14 суток — listing_source_snapshot,
# 9.7 с ЦЕЛИКОМ (и у неё сверх того свой `SET LOCAL statement_timeout = 900000`,
# который перекрывает это значение — гейт tests/test_3463_db_timeouts.py);
# * самый длинный set-based statement ЧЕРЕЗ движок из замеренных — матч ГАР→houses
# (`services/gar_flats_loader._MATCH_SQL`): 2.07 с с городским фильтром и 6.46 с без
# него (`city_filter=None`, флаг CLI). Запас ~3×, и это СЧИТАЮЩИЙ запрос, а не ждущий.
#
# `pg_stat_statements` опорой по планировщику НЕ является: при `max = 5000` он вытесняет
# редкие записи (проверено 12.09 — `dealloc` вырос на единицу за десять минут, и из топа
# пропали ВСЕ записи с `calls = 1`, включая `REFRESH MATERIALIZED VIEW` 30.85 с и KNN
# `cadastral_geo_match` 2.45 с). Суточная задача до следующих суток там не доживает, так
# что «самый долгий запрос 4.27 с» верно только для ВЫСОКОЧАСТОТНЫХ запросов.
_STATEMENT_TIMEOUT_MS = 30_000
# Ожидание БЛОКИРОВКИ — заведомо меньше: ждать лок дольше секунд смысла нет, лучше
@ -39,10 +43,12 @@ _STATEMENT_TIMEOUT_MS = 30_000
# считает, — statement_timeout тут только страховка от «считает вечно».
_LOCK_TIMEOUT_MS = 5_000
# idle_in_transaction_session_timeout НАМЕРЕННО не трогаем: тот же движок обслуживает
# и планировщик, а его свипы держат транзакцию открытой всё время внешнего HTTP
# (замер на проде 12.09: живая сессия `idle in transaction` 29 с). Сессионный потолок
# на простой в транзакции убивал бы рабочий сбор, а не зависший запрос.
# idle_in_transaction_session_timeout НАМЕРЕННО не трогаем: тем же движком живёт tgbot,
# и `services/tgbot/bridge.py` держит транзакцию открытой ПОВЕРХ long-poll Telegram
# (замер на проде 12.09, 3 пробы с шагом 7 с: одна и та же сессия, запрос
# `SELECT value FROM tg_support_state …`, возраст транзакции циклически растёт до ~29 с).
# Сессионный потолок на простой в транзакции ронял бы long-poll КАЖДЫЙ цикл —
# гарантированно, а не в редком случае.
DB_CONNECT_ARGS = {
"options": f"-c statement_timeout={_STATEMENT_TIMEOUT_MS} -c lock_timeout={_LOCK_TIMEOUT_MS}"
}

View file

@ -74,13 +74,28 @@ def _live_engine(connect_args: dict[str, str]) -> Engine | None:
dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "")
if not dsn or "localhost:5432/test" in dsn:
return None
# Сначала проба БЕЗ connect_args: она отделяет «сервера нет» (честный пропуск)
# от «сервер есть, но наши `options` он не принял». Глушить второе нельзя —
# именно так испорченное значение (`statement_timeout=30000zz`) проходило
# зелёным: коннект падал, тест пропускался, запись в allowlist гасила сигнал,
# а на проде это FATAL на КАЖДОМ коннекте.
try:
eng = create_engine(dsn, future=True, connect_args=connect_args)
with eng.connect() as conn:
conn.execute(text("SELECT 1"))
return eng
probe = create_engine(dsn, future=True)
except Exception:
return None
try:
with probe.connect() as conn:
conn.execute(text("SELECT 1"))
except Exception:
return None
finally:
probe.dispose()
eng = create_engine(dsn, future=True, connect_args=connect_args)
with eng.connect() as conn: # НЕ под except: сервер живой, виноваты connect_args
conn.execute(text("SELECT 1"))
return eng
@pytest.fixture
@ -111,14 +126,16 @@ def test_engine_opens_connections_with_both_ceilings() -> None:
Фальсификация: убрать `connect_args=DB_CONNECT_ARGS` из `create_engine`
`options` станет пустой, тест краснеет.
"""
expected = f"-c statement_timeout={_STATEMENT_TIMEOUT_MS} -c lock_timeout={_LOCK_TIMEOUT_MS}"
options = _engine_connect_options(engine)
assert f"-c statement_timeout={_STATEMENT_TIMEOUT_MS}" in options, (
f"движок открывает коннекты без потолка на запрос (options={options!r}) — "
"заблокированный запрос снова висит бесконечно и жжёт слот /estimate (#3463)"
)
assert f"-c lock_timeout={_LOCK_TIMEOUT_MS}" in options, (
f"движок открывает коннекты без потолка на ожидание блокировки "
f"(options={options!r}) — это ровно сценарий отказа из #3463"
# РАВЕНСТВО, а не `in`: подстрочная проверка пропускала испорченный хвост
# (`…=30000zz` содержит `…=30000`), а Postgres на такое значение отвечает
# `FATAL: invalid value for parameter "statement_timeout"` — ни одного коннекта
# ни в одном из трёх сервисов образа, полный отказ продукта.
assert options == expected, (
f"движок открывает коннекты с options={options!r}, ожидалось {expected!r}: "
"либо потолка нет вовсе (заблокированный запрос снова висит и жжёт слот "
"/estimate, #3463), либо значение испорчено — тогда libpq отвергнет КАЖДЫЙ коннект"
)
assert DB_CONNECT_ARGS["options"] == options
@ -139,6 +156,14 @@ def test_ceilings_are_coherent_with_declared_estimate_budgets() -> None:
f"потолок запроса {_STATEMENT_TIMEOUT_MS} мс не выше самого длинного объявленного "
f"бюджета {longest_ms:.0f} мс — потолок стал бы биндящим ограничением честной работы"
)
# Потолок сверху — иначе у проверки есть пол и нет крыши: `_STATEMENT_TIMEOUT_MS =
# 300_000` (пять минут) зеленел бы, а пять минут ожидания это тот же отказ, только
# медленнее: четыре таких запроса всё так же выедают `_ESTIMATE_CONCURRENCY`.
assert _STATEMENT_TIMEOUT_MS <= 2 * longest_ms, (
f"потолок запроса {_STATEMENT_TIMEOUT_MS} мс больше чем вдвое превышает самый "
f"длинный объявленный бюджет {longest_ms:.0f} мс — это уже не защита, а отсрочка: "
"слот /estimate держится всё это время"
)
assert 0 < _LOCK_TIMEOUT_MS < _STATEMENT_TIMEOUT_MS, (
"ожидание блокировки обязано обрываться РАНЬШЕ потолка на сам запрос: "
"деградировать лучше, чем держать слот"