From 80b38fccaa6808d924e65f5ae48bf2f9cc7ec006 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Wed, 19 Aug 2026 15:37:39 +0500 Subject: [PATCH] =?UTF-8?q?fix(ptica):=20=D1=81=D0=BE=D0=B5=D0=B4=D0=B8?= =?UTF-8?q?=D0=BD=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=91=D0=94=20=D0=BE=D1=82?= =?UTF-8?q?=D0=BF=D1=83=D1=81=D0=BA=D0=B0=D0=B5=D1=82=D1=81=D1=8F=20=D0=B4?= =?UTF-8?q?=D0=BE=20=D0=BF=D0=BE=D1=85=D0=BE=D0=B4=D0=B0=20=D0=B7=D0=B0=20?= =?UTF-8?q?=D1=84=D0=BE=D1=82=D0=BE=D0=B3=D1=80=D0=B0=D1=84=D0=B8=D0=B5?= =?UTF-8?q?=D0=B9=20=D0=BD=D0=B0=D1=80=D1=83=D0=B6=D1=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v1/photos/{obj}/{file} берёт сессию через Depends(get_db), делает SELECT — и держит соединение пула всё время синхронного фетча к ДОМ.РФ. SQLAlchemy открывает транзакцию на первом запросе, поэтому она ещё и висит idle-in-transaction. ЗАМЕР 19.08 по domrf_kn_photos: всего фотографий 165 208 закешировано локально 1 889 (1.1%) пойдут ленивым путём 163 319 (98.9%) То есть почти каждый запрос картинки — удержание соединения на время внешнего похода (_UPSTREAM_TIMEOUT 8 с, connect 4 с). Пул при этом дефолтный: create_engine в app/core/db.py без pool_size, значит 5 + 10 overflow = 15 соединений на весь бэкенд. Страница отчёта тянет картинки пачкой — пятнадцать таких запросов занимают пул целиком, и за ними встают ВСЕ остальные ручки. Правка: db.close() сразу после того, как значения строки разложены по локальным переменным, — до фетча и до генерации миниатюры. close() не делает сессию непригодной: следующий db.execute прозрачно берёт новое соединение. Тест проверяет ФАКТ отпускания (in_transaction() в момент фетча), а не наличие db.close() в тексте — иначе он фиксировал бы реализацию, а не свойство. Сессия в тесте настоящая (SQLite), не мок: на моке in_transaction был бы выдумкой. Наружу тест не ходит — _fetch_upstream подменён, ДОМ.РФ трогать нельзя. Двусторонний: против main падает ровно проверка отпускания; два контроля — «закешированная миниатюра всё ещё отдаётся с диска» и «незарегистрированная фотография всё ещё 404» — зелёные с обеих сторон. Хунк форматирования — не мой: pre-commit ruff v0.7.4 против 0.15.12 (#2864). Refs #2464 --- backend/app/api/v1/photos.py | 18 +++ .../test_2464c_photos_session_release.py | 116 ++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 backend/tests/test_2464c_photos_session_release.py diff --git a/backend/app/api/v1/photos.py b/backend/app/api/v1/photos.py index 2f781f8a..5f883ea5 100644 --- a/backend/app/api/v1/photos.py +++ b/backend/app/api/v1/photos.py @@ -85,6 +85,24 @@ def get_photo( upstream = row["photo_url"] photo_name = row["photo_name"] + # #2464-C: отпускаем соединение ДО любой медленной работы — внешнего фетча + # (до 8 с) и генерации миниатюры. SELECT выше открыл транзакцию (SQLAlchemy + # начинает её на первом запросе), и без этого она висела бы idle-in-transaction + # всё это время, занимая соединение пула. + # + # Почему это важно именно здесь: закешировано локально 1 889 фотографий из + # 165 208 (замер 19.08.2026), то есть 98.9% запросов идут «ленивым» путём с + # походом наружу. Пул дефолтный — `create_engine` в app/core/db.py без + # pool_size, значит 5 + 10 overflow = 15 соединений на весь бэкенд. Страница + # отчёта тянет картинки пачкой, и пятнадцать таких запросов занимают пул + # целиком, а за ними встают ВСЕ остальные ручки. + # + # `close()` не делает сессию непригодной: следующий `db.execute` ниже + # прозрачно возьмёт новое соединение и откроет свою транзакцию. Значения из + # `row` уже разложены по локальным переменным выше — после закрытия они + # остаются доступны. + db.close() + headers = {"Cache-Control": "public, max-age=604800, immutable"} # ── size=thumb ────────────────────────────────────────────────────────── diff --git a/backend/tests/test_2464c_photos_session_release.py b/backend/tests/test_2464c_photos_session_release.py new file mode 100644 index 00000000..ce7afe9a --- /dev/null +++ b/backend/tests/test_2464c_photos_session_release.py @@ -0,0 +1,116 @@ +"""#2464-C: сессия БД не должна держаться на время внешнего HTTP-фетча. + +`GET /api/v1/photos/{obj}/{file}` берёт сессию через `Depends(get_db)`, делает +SELECT — и (SQLAlchemy открывает транзакцию на первом запросе) ДЕРЖИТ соединение +пула всё время синхронного похода к ДОМ.РФ. + +Цена на проде, замер 19.08.2026 по `domrf_kn_photos`: + + всего фотографий 165 208 + закешировано локально 1 889 (1.1%) + пойдут «ленивым» путём 163 319 (98.9%) + +То есть почти каждый запрос картинки — это удержание соединения на время +внешнего фетча (`_UPSTREAM_TIMEOUT` = 8 с, connect 4 с). Пул при этом +дефолтный: `create_engine(...)` в `app/core/db.py` без `pool_size`, то есть +5 + 10 overflow = 15 соединений на весь бэкенд. Страница отчёта тянет +несколько картинок разом — пятнадцать таких запросов занимают пул целиком, и +за ними встают ВСЕ остальные ручки. + +Хуже, чем просто занятое соединение: транзакция открыта и висит +idle-in-transaction, что мешает vacuum'у. + +Тест проверяет ФАКТ отпускания соединения в момент фетча, а не наличие +`db.close()` в тексте — иначе он бы фиксировал реализацию, а не свойство. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from sqlalchemy import create_engine, text +from sqlalchemy.orm import Session + + +@pytest.fixture +def _session_with_photo_row(tmp_path: Path): + """Настоящая сессия SQLAlchemy (SQLite) с одной строкой фотографии. + + Настоящая, а не MagicMock: проверяется свойство сессии (`in_transaction`), + и на моке оно было бы выдумкой. + """ + engine = create_engine(f"sqlite:///{tmp_path / 'photos.db'}", future=True) + with engine.begin() as conn: + conn.execute( + text( + "CREATE TABLE domrf_kn_photos (" + " obj_id INTEGER, obj_file_id TEXT, local_path TEXT," + " thumb_path TEXT, photo_url TEXT, photo_name TEXT, size_bytes INTEGER)" + ) + ) + conn.execute( + text( + "INSERT INTO domrf_kn_photos VALUES" + " (1, 'f1', NULL, NULL, 'https://upstream.example/p.jpg', 'p.jpg', 100)" + ) + ) + session = Session(engine, future=True) + yield session + session.close() + engine.dispose() + + +def test_connection_released_before_upstream_fetch(_session_with_photo_row, monkeypatch) -> None: + """В момент похода наружу транзакция закрыта — соединение вернулось в пул. + + На main: SELECT открыл транзакцию, она висит все 8 с фетча. + """ + from app.api.v1 import photos + + seen: dict[str, bool] = {} + + def _spy_fetch(url: str): + seen["in_transaction"] = _session_with_photo_row.in_transaction() + return None # не ходим наружу: ДОМ.РФ трогать нельзя + + monkeypatch.setattr(photos, "_fetch_upstream", _spy_fetch) + + resp = photos.get_photo(db=_session_with_photo_row, obj_id=1, file_id="f1", size="thumb") + + assert seen.get("in_transaction") is False, ( + "во время внешнего фетча сессия держит открытую транзакцию — " + "соединение пула занято, и при пуле в 15 штук страница отчёта его исчерпает" + ) + # Поведение не изменилось: картинки локально нет → редирект на upstream. + assert resp.status_code == 302 + + +def test_still_serves_cached_thumb(_session_with_photo_row, tmp_path: Path) -> None: + """Контроль: закешированная миниатюра по-прежнему отдаётся с диска. + + Зелёный с обеих сторон — доказывает, что раннее закрытие сессии не сломало + быстрый путь (98.9% запросов идут не им, но именно он — цель кеша). + """ + from app.api.v1 import photos + + thumb = tmp_path / "t.webp" + thumb.write_bytes(b"webp") + _session_with_photo_row.execute( + text("UPDATE domrf_kn_photos SET thumb_path = :t"), {"t": str(thumb)} + ) + _session_with_photo_row.commit() + + resp = photos.get_photo(db=_session_with_photo_row, obj_id=1, file_id="f1", size="thumb") + assert getattr(resp, "path", None) == str(thumb) + + +def test_missing_row_still_404(_session_with_photo_row) -> None: + """Контроль: незарегистрированная фотография по-прежнему 404, а не 500.""" + from fastapi import HTTPException + + from app.api.v1 import photos + + with pytest.raises(HTTPException) as exc: + photos.get_photo(db=_session_with_photo_row, obj_id=999, file_id="nope", size="thumb") + assert exc.value.status_code == 404 -- 2.45.3