From 4c0abb8a0df6906b1bba6779eb6f7a00dff1e154 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Wed, 19 Aug 2026 20:17:17 +0500 Subject: [PATCH] =?UTF-8?q?fix(ptica):=20job=5Fsettings=20=D0=BD=D0=B5=20?= =?UTF-8?q?=D0=BE=D1=82=D1=80=D0=B0=D0=B2=D0=BB=D1=8F=D0=B5=D1=82=20=D1=87?= =?UTF-8?q?=D1=83=D0=B6=D1=83=D1=8E=20=D1=81=D0=B5=D1=81=D1=81=D0=B8=D1=8E?= =?UTF-8?q?=20=D0=BF=D1=80=D0=B8=20=D1=81=D0=B1=D0=BE=D0=B5=20=D0=91=D0=94?= =?UTF-8?q?=20(#2464=20=D0=BA=D0=BB=D0=B0=D1=81=D1=82=D0=B5=D1=80=20A)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_all/get_one глотают ошибку db.execute и возвращают fallback, но сессию им отдаёт вызывающий: admin-ручки, beat_schedule, и get_setting_value из cadastre_fetch/nspd_geo. На Postgres упавший execute оставляет транзакцию в aborted-состоянии — все последующие запросы ЭТОЙ ЖЕ сессии падают с «current transaction is aborted», то есть падает не тот, кто виноват. Голый db.rollback() здесь запрещён: он снёс бы незакоммиченную работу вызывающего. Средство — SAVEPOINT вокруг самого execute, как в уже закрытом пункте того же кластера (developer_attribution.py:152). Мок в тесте ВОСПРОИЗВОДИТ семантику Postgres: упавший запрос переводит сессию в aborted, дальнейшие падают, откат SAVEPOINT восстанавливает. Это не педантизм. Существующий образец в tests/test_saturation.py устроен иначе — begin_nested там пустой контекст-менеджер без aborted-состояния, — и проверка на отравление проходит независимо от наличия защиты. Проверено экспериментом: со СНЯТЫМ SAVEPOINT в saturation.py все 19 тестов файла зелёные. Тест, который не может покраснеть при снятии охраняемого, охраны не проверяет. Поэтому здесь есть ещё и контроль на сам мок (test_mock_actually_poisons_ without_a_savepoint): без него проверки были бы зелёными по построению. Тесты: 2 красных на origin/main с настоящим текстом ошибки Postgres («current transaction is aborted, commands ignored until end of transaction block»), 2 контроля зелёные с обеих сторон. pytest tests/services: 3063 passed, 14 skipped, rc=0 --- backend/app/services/job_settings.py | 66 +++++---- .../test_2464a_job_settings_savepoint.py | 139 ++++++++++++++++++ 2 files changed, 177 insertions(+), 28 deletions(-) create mode 100644 backend/tests/services/test_2464a_job_settings_savepoint.py diff --git a/backend/app/services/job_settings.py b/backend/app/services/job_settings.py index 4d12d32d..23532bfd 100644 --- a/backend/app/services/job_settings.py +++ b/backend/app/services/job_settings.py @@ -124,22 +124,30 @@ def _fallback(job_type: str) -> dict[str, Any]: def get_all(db) -> list[dict[str, Any]]: """Вернуть все строки job_settings. При ошибке БД — fallback на _DEFAULTS.""" try: - rows = ( - db.execute( - text( - """ - SELECT job_type, enabled, queue_name, cron_schedule, rate_ms, - max_retries, max_concurrency, extra_config, - updated_at, updated_by, description - FROM job_settings - ORDER BY job_type - """ + with db.begin_nested(): + rows = ( + db.execute( + text( + """ + SELECT job_type, enabled, queue_name, cron_schedule, rate_ms, + max_retries, max_concurrency, extra_config, + updated_at, updated_by, description + FROM job_settings + ORDER BY job_type + """ + ) ) + .mappings() + .all() ) - .mappings() - .all() - ) except Exception as e: + # #2464 cluster A: сессия ЧУЖАЯ — её отдаёт вызывающий (admin-ручка, + # beat_schedule, get_setting_value из cadastre_fetch/nspd_geo). Ошибка + # db.execute на Postgres оставляет транзакцию в aborted-состоянии, и все + # последующие запросы этой же сессии падают с «current transaction is + # aborted». Голый db.rollback() здесь НЕЛЬЗЯ: он снёс бы незакоммиченную + # работу вызывающего. Поэтому SAVEPOINT вокруг самого execute (см. + # developer_attribution.py:152, тот же кластер) — откатывается только он. logger.warning("get_all job_settings: БД недоступна — fallback. %s", e) return [_fallback(jt) for jt in _DEFAULTS] @@ -153,23 +161,25 @@ def get_all(db) -> list[dict[str, Any]]: def get_one(job_type: str, db) -> dict[str, Any]: """Вернуть одну строку по job_type. При отсутствии — fallback с warning.""" try: - row = ( - db.execute( - text( - """ - SELECT job_type, enabled, queue_name, cron_schedule, rate_ms, - max_retries, max_concurrency, extra_config, - updated_at, updated_by, description - FROM job_settings - WHERE job_type = :jt - """ - ), - {"jt": job_type}, + with db.begin_nested(): + row = ( + db.execute( + text( + """ + SELECT job_type, enabled, queue_name, cron_schedule, rate_ms, + max_retries, max_concurrency, extra_config, + updated_at, updated_by, description + FROM job_settings + WHERE job_type = :jt + """ + ), + {"jt": job_type}, + ) + .mappings() + .first() ) - .mappings() - .first() - ) except Exception as e: + # См. get_all выше: SAVEPOINT, а не rollback — сессия принадлежит вызывающему. logger.warning("get_one job_settings '%s': БД недоступна — fallback. %s", job_type, e) return _fallback(job_type) diff --git a/backend/tests/services/test_2464a_job_settings_savepoint.py b/backend/tests/services/test_2464a_job_settings_savepoint.py new file mode 100644 index 00000000..5539f82c --- /dev/null +++ b/backend/tests/services/test_2464a_job_settings_savepoint.py @@ -0,0 +1,139 @@ +"""#2464 кластер A: job_settings не должен отравлять ЧУЖУЮ сессию. + +`get_all`/`get_one` глотают ошибку БД и возвращают fallback. Сессию им отдаёт +вызывающий — admin-ручка, `beat_schedule`, либо `get_setting_value` из +`cadastre_fetch`/`nspd_geo`. На Postgres упавший `db.execute` оставляет транзакцию +в aborted-состоянии, и ВСЕ последующие запросы этой же сессии падают с +«current transaction is aborted, commands ignored until end of transaction block» — +падает не тот, кто виноват. + +Голый `db.rollback()` здесь запрещён: он снёс бы незакоммиченную работу +вызывающего. Правильное средство — SAVEPOINT вокруг самого execute +(`developer_attribution.py:152`, тот же кластер). + +О МОКЕ +────── +Мок ниже ВОСПРОИЗВОДИТ семантику Postgres: после упавшего execute сессия помечается +aborted и дальнейшие запросы падают, пока откат SAVEPOINT её не восстановит. + +Это существенно. Обычный «мок с пустым begin_nested» (см. tests/test_saturation.py) +такую проверку не даёт: у него нет aborted-состояния, поэтому второй execute +проходит в любом случае — и тест зелёный независимо от того, есть SAVEPOINT в коде +или нет. Проверка, которую нельзя уронить, сняв защиту, защиты не проверяет. +""" + +from __future__ import annotations + +from contextlib import contextmanager +from typing import Any + +import pytest + +from app.services.job_settings import get_all, get_one + + +class AbortedTransactionError(RuntimeError): + """Аналог psycopg InFailedSqlTransaction.""" + + +class _PostgresLikeDb: + """Сессия с aborted-состоянием и настоящей семантикой SAVEPOINT.""" + + def __init__(self, *, fail_first: bool = True) -> None: + self.calls = 0 + self.aborted = False + self._fail_first = fail_first + self._savepoint_depth = 0 + + @contextmanager + def begin_nested(self): # type: ignore[no-untyped-def] + self._savepoint_depth += 1 + try: + yield + except Exception: + # Откат SAVEPOINT: снимаем aborted, внешняя транзакция цела. + self.aborted = False + raise + finally: + self._savepoint_depth -= 1 + + def execute(self, *_args: Any, **_kwargs: Any) -> Any: + if self.aborted: + raise AbortedTransactionError( + "current transaction is aborted, commands ignored until end of " "transaction block" + ) + self.calls += 1 + if self.calls == 1 and self._fail_first: + # Ошибка внутри транзакции переводит её в aborted. + self.aborted = True + raise RuntimeError("simulated DB failure") + return _Result() + + +# Строка в форме, которую ждёт _row_to_dict: одиннадцать колонок SELECT'а. +_DB_ROW: dict[str, Any] = { + "job_type": "scrape_kn", + "enabled": True, + "queue_name": "celery", + "cron_schedule": "0 3 * * *", + "rate_ms": 1000, + "max_retries": 3, + "max_concurrency": 1, + "extra_config": {"marker": "из-БД"}, + "updated_at": None, + "updated_by": None, + "description": "тестовая строка", +} + + +class _Result: + def mappings(self) -> _Result: + return self + + def all(self) -> list[dict[str, Any]]: + return [_DB_ROW] + + def first(self) -> dict[str, Any]: + return _DB_ROW + + +def test_get_all_leaves_the_caller_session_usable() -> None: + """После сбоя внутри get_all следующий запрос вызывающего должен пройти.""" + db = _PostgresLikeDb() + + rows = get_all(db) + assert rows, "fallback не вернулся — сломано само graceful-поведение" + + # Это и есть проверка: на origin/main здесь AbortedTransaction. + assert db.execute("SELECT 1").mappings().first() == _DB_ROW + + +def test_get_one_leaves_the_caller_session_usable() -> None: + db = _PostgresLikeDb() + + row = get_one("scrape_kn", db) + assert row, "fallback не вернулся" + + assert db.execute("SELECT 1").mappings().first() == _DB_ROW + + +def test_mock_actually_poisons_without_a_savepoint() -> None: + """Контроль на сам мок: без SAVEPOINT он ОБЯЗАН отравляться. + + Без этой проверки тесты выше были бы зелёными по построению — ровно та ловушка, + из-за которой существующий мок в test_saturation.py ничего не проверяет. + """ + db = _PostgresLikeDb() + with pytest.raises(RuntimeError): + db.execute("boom") # первый вызов падает и переводит в aborted + with pytest.raises(AbortedTransactionError): + db.execute("SELECT 1") + + +def test_healthy_session_is_not_disturbed() -> None: + """Контроль: без сбоя поведение прежнее — данные из БД, а не fallback.""" + db = _PostgresLikeDb(fail_first=False) + rows = get_all(db) + assert len(rows) == 1 + assert rows[0]["job_type"] == "scrape_kn" + assert rows[0]["extra_config"] == {"marker": "из-БД"}, "вернулся fallback вместо данных БД" -- 2.45.3