From c4485d43b148dd415944675360ee0db4f2a69962 Mon Sep 17 00:00:00 2001 From: Max Ronzhin Date: Mon, 10 Aug 2026 16:09:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(metrics):=20=D1=80=D0=B0=D0=B7=D0=B2=D1=8F?= =?UTF-8?q?=D0=B7=D0=B0=D1=82=D1=8C=20vidconf=5Fpipeline=5Fsessions=20?= =?UTF-8?q?=D1=81=20=D0=BE=D1=81=D0=BD=D0=BE=D0=B2=D0=BD=D1=8B=D0=BC=20?= =?UTF-8?q?=D0=BF=D1=83=D0=BB=D0=BE=D0=BC=20=D0=91=D0=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _refresh_pipeline_sessions_gauge и metrics_endpoint ходили через Depends(get_session) — основной пул, разделяемый с API-запросами. В инциденте 07.08 это дало 16 падений в api/metrics.py ровно тогда, когда метрики были нужнее всего (пул исчерпан). db_up/db_pool_* уже были развязаны в 0.0.32, эта метрика — нет (мешал тестовый харнесс). Добавлен get_metrics_session (core/db.py) — отдельный движок с NullPool, как у check_db_up, но с полноценной ORM-сессией для репозитория. Тестовый харнесс (app-фикстура) подменяет её на ту же savepoint-сессию, что и get_session, — иначе /metrics не видел бы данные теста. --- backend/api/metrics.py | 48 ++++++++++++++++++++------------------- backend/core/db.py | 32 ++++++++++++++++++++++++++ backend/tests/conftest.py | 10 +++++++- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/backend/api/metrics.py b/backend/api/metrics.py index af84c19..c8b3d8b 100644 --- a/backend/api/metrics.py +++ b/backend/api/metrics.py @@ -14,17 +14,19 @@ Redis) можно опросить обычным `await` вместо реал и вовсе синхронный — настройки уже в памяти процесса). 🔴 Метрики о состоянии основного пула БД (`vidconf_db_up`, -`vidconf_db_pool_*`) обязаны читаться БЕЗ обращения к самому пулу — иначе -в момент его исчерпания (см. `.forcc/session-results/32-loadtest-07-08-debug.md`) -эндпоинт метрик падал бы вместе со всем остальным ровно тогда, когда нужнее -всего. `vidconf_db_pool_*` — синхронный снимок `engine.pool` (см. -`core/db.py::db_pool_stats`), `vidconf_db_up` — отдельное соединение вне -основного пула (`core/db.py::check_db_up`). `_refresh_pipeline_sessions_gauge` -по-прежнему ходит через основной пул (`Depends(get_session)`, тестовый -харнесс подменяет её на savepoint-сессию — см. `tests/conftest.py`; развести -полностью, как `vidconf_db_up`, значило бы переделывать харнесс ради того же -эффекта — цена не оправдана, см. прецедент `f7c4fb4`/session 32), но обёрнута -таймаутом и try/except, чтобы её недоступность не роняла остальные метрики. +`vidconf_db_pool_*`, `vidconf_pipeline_sessions`) обязаны читаться БЕЗ +обращения к самому пулу — иначе в момент его исчерпания (см. +`.forcc/session-results/32-loadtest-07-08-debug.md`) эндпоинт метрик падал бы +вместе со всем остальным ровно тогда, когда нужнее всего. `vidconf_db_pool_*` +— синхронный снимок `engine.pool` (см. `core/db.py::db_pool_stats`), +`vidconf_db_up` — отдельное соединение вне основного пула +(`core/db.py::check_db_up`). `_refresh_pipeline_sessions_gauge` с сессии 37 +тоже читает через отдельный движок (`Depends(get_metrics_session)`, +`core/db.py`) — тестовый харнесс подменяет её на savepoint-сессию теста так +же, как `get_session` (см. `tests/conftest.py`). До сессии 37 она ходила +через основной пул (`Depends(get_session)`) — прецедент `f7c4fb4`/session 32; +дополнительная обёртка таймаутом и try/except ниже осталась как вторая +линия обороны на случай, если сама БД (а не пул) не отвечает. """ import asyncio @@ -37,7 +39,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from starlette.routing import Match from core.config import get_settings -from core.db import check_db_up, db_pool_checked_out, get_session +from core.db import check_db_up, db_pool_checked_out, get_metrics_session from core.redis import redis_client from models.session import PIPELINE_STATUSES from repositories.conferences import ConferenceSessionRepository @@ -104,12 +106,12 @@ _PIPELINE_GAUGE_TIMEOUT_S = 2.0 async def _refresh_pipeline_sessions_gauge(session: AsyncSession) -> None: """Пересчитать `vidconf_pipeline_sessions` по всем статусам `pipeline_status`. - Ходит через основной пул (`session` — из `Depends(get_session)`, см. - докстринг модуля про ограничения тестового харнесса). Если пул занят - или БД недоступна, запрос не должен держать весь `/metrics` — таймаут - короче `db_pool_timeout`, ошибка гасится, gauge остаётся на прежнем - значении (не обнуляется — обнулять его при недоступности БД так же - неверно, как считать сеансы пропавшими). + `session` — из `Depends(get_metrics_session)` (отдельный от основного + пула движок, см. докстринг модуля и `core/db.py`). Таймаут и try/except + ниже — вторая линия обороны на случай, если недоступна сама БД (а не + только основной пул): запрос не должен держать весь `/metrics`, gauge + остаётся на прежнем значении (не обнуляется — обнулять его при + недоступности БД так же неверно, как считать сеансы пропавшими). """ try: counts = await asyncio.wait_for( @@ -249,7 +251,7 @@ def _refresh_host_info_gauge() -> None: @router.get("/metrics") -async def metrics_endpoint(session: AsyncSession = Depends(get_session)) -> Response: +async def metrics_endpoint(session: AsyncSession = Depends(get_metrics_session)) -> Response: """Отдать метрики Prometheus в формате text exposition. Gauge'и пересчитываются прямо здесь (а не по расписанию/периодическим @@ -259,10 +261,10 @@ async def metrics_endpoint(session: AsyncSession = Depends(get_session)) -> Resp редко (обычно раз в 15–30с), нагрузка пренебрежимо мала. Порядок важен: метрики о состоянии основного пула БД (`_refresh_db_up_gauge`, - `_refresh_db_pool_gauges`) считаются первыми и не зависят от самого пула - (см. докстринг модуля) — они гарантированно попадут в ответ, даже если - следующий за ними `_refresh_pipeline_sessions_gauge` (основной пул) зависнет - или упадёт под нагрузкой. + `_refresh_db_pool_gauges`) считаются первыми — они гарантированно попадут в + ответ, даже если следующая за ними `_refresh_pipeline_sessions_gauge` + (отдельный движок, но всё ещё сама БД) зависнет или упадёт (таймаут/ + try-except внутри неё гасят это, не роняя остальные метрики). """ await _refresh_db_up_gauge() _refresh_db_pool_gauges() diff --git a/backend/core/db.py b/backend/core/db.py index bcf7d52..9022bd1 100644 --- a/backend/core/db.py +++ b/backend/core/db.py @@ -65,6 +65,38 @@ async def check_db_up() -> bool: return True +# --- Сессия для метрик, читающих данные (не только «жив/мёртв»), вне основного +# пула (сессия 37, доделка session 32/33 — см. докстринг `api/metrics.py`) ---- +# +# `check_db_up` выше обходится голым соединением ("SELECT 1"), но +# `vidconf_pipeline_sessions` нужна полноценная ORM-сессия (репозиторий, +# группировка по статусу) — `NullPool`, как и у `_probe_engine`: каждый вызов +# открывает новое соединение и сразу закрывает его, бюджет основного пула +# (`engine.pool`) не расходуется. `async_sessionmaker` — тот же паттерн, что +# `async_session_maker` выше, просто на другом движке. +_metrics_probe_engine: AsyncEngine = create_async_engine( + settings.database_url, + poolclass=NullPool, + connect_args={"timeout": settings.db_probe_timeout_s}, +) +_metrics_session_maker = async_sessionmaker(_metrics_probe_engine, expire_on_commit=False) + + +async def get_metrics_session() -> AsyncGenerator[AsyncSession, None]: + """Зависимость FastAPI для метрик, которым нужна БД, но не основной пул. + + В отличие от `get_session()` (основной пул `engine.pool`, конкурирует за + те же 10+10×воркеров соединений, что и API-запросы), сессия здесь открыта + на `_metrics_probe_engine` — исчерпание основного пула эту метрику не + заденет, как и `vidconf_db_up`/`vidconf_db_pool_*`. Тестовый харнесс + (`tests/conftest.py`) подменяет и её на savepoint-сессию теста — так же, + как `get_session` — иначе тест `vidconf_pipeline_sessions` не видел бы + данные, ещё не закоммиченные за пределы savepoint. + """ + async with _metrics_session_maker() as session: + yield session + + def db_pool_checked_out() -> int: """Число соединений основного пула, занятых прямо сейчас — без обращения к БД. diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 71684d0..159cea5 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -145,13 +145,21 @@ async def db_session(db_connection: AsyncConnection) -> AsyncGenerator[AsyncSess @pytest_asyncio.fixture async def app(db_session: AsyncSession) -> AsyncGenerator[FastAPI, None]: - """Экземпляр FastAPI-приложения с `get_session`, подменённым на тестовую (savepoint) сессию.""" + """Экземпляр FastAPI-приложения с `get_session`/`get_metrics_session`, + подменёнными на тестовую (savepoint) сессию. + + `get_metrics_session` (сессия 37, `core/db.py`) в проде — отдельный от + основного пула движок, но в тестах должен указывать на ТУ ЖЕ savepoint- + сессию, что и `get_session` — иначе `/metrics` не видел бы данные теста, + ещё не закоммиченные за пределы savepoint (см. `test_metrics_api.py`). + """ application = create_app() async def _override_get_session() -> AsyncGenerator[AsyncSession, None]: yield db_session application.dependency_overrides[get_session] = _override_get_session + application.dependency_overrides[get_metrics_session] = _override_get_session yield application