fix(ptica): соединение БД отпускается до похода за фотографией наружу
All checks were successful
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 10s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 1m55s
CI / backend-tests (pull_request) Successful in 16m10s
All checks were successful
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 10s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 1m55s
CI / backend-tests (pull_request) Successful in 16m10s
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
This commit is contained in:
parent
d25ff668f7
commit
80b38fccaa
2 changed files with 134 additions and 0 deletions
|
|
@ -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 ──────────────────────────────────────────────────────────
|
||||
|
|
|
|||
116
backend/tests/test_2464c_photos_session_release.py
Normal file
116
backend/tests/test_2464c_photos_session_release.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Reference in a new issue