All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI / changes (pull_request) Successful in 9s
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 / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 4m44s
На проде — 194 события «429 Too many requests» от Nominatim за неделю. Это не
внешняя случайность: политику 1 req/sec изображали три разрозненных
asyncio.sleep(1.0), и ни один не давал ограничения на самом деле.
geocode() — сон ВНУТРИ `if result is not None`, то есть только
после УСПЕХА; после неудачи паузы не было;
_nominatim_lookup() — сон МЕЖДУ typo-вариантами, но не перед tier-1;
цикл бэкфилла — своей паузы не имеет вовсе, зовёт geocode()
последовательно.
Для неразрешимого адреса выходило 5 запросов за 4 секунды и сразу следующий
адрес без паузы. То есть темп сбрасывался ровно тогда, когда сервер и просит
притормозить, и разгонялся ровно на тех адресах, что не резолвятся.
А череда неудач — не редкость, а норма прогона. Разбор 16947 объявлений без
координат на 20.08: активных 1948, из них с пригодным адресом 1471, доступно
бэкфиллу прямо сейчас 47, а 1424 лежат в семидневном backoff — то есть 97%
активных «без координат» это повторные неудачи, возвращающиеся в очередь
каждые 7 суток. Разгон происходит каждый прогон.
Теперь пауза берётся ПЕРЕД каждым обращением и одна на все три точки вызова
(/search x2, /reverse). Четыре прежних sleep убраны как избыточные.
Повторы на 429 намеренно НЕ трогал. Если убрать их той же правкой, падение
счётчика 429 нельзя будет приписать именно ограничителю темпа — а критерий
приёмки в #2953 сформулирован через этот счётчик.
Тест проверяет ПОВЕДЕНИЕ (расстояние между исходящими запросами), а не наличие
нового символа: импорт _nominatim_throttle дал бы на origin/main ImportError,
то есть «возможности нет», и красный ничего бы не доказывал. Против кода
origin/main:
зазоры между тремя запросами [0.0003, 0.0001] вместо >=0.08 -> падает
стык двух неудачных поисков 0.0022с вместо >=0.08 -> падает
первый запрос не ждёт впустую контроль, зелёный на обеих сторонах
Прогон tradein-сьюта: 280 failed / 2099 passed против базы 280 failed /
2096 passed на origin/main — ни одного нового падения, +3 своих. Падения и
216 collection-errors средовые (нет scraper_kit/bcrypt/lxml в локальном venv).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
137 lines
7.1 KiB
Python
137 lines
7.1 KiB
Python
"""Темп обращений к Nominatim ограничен ПЕРЕД запросом, а не после успеха (#2953).
|
||
|
||
Политику 1 req/sec раньше изображали три разрозненных `asyncio.sleep(1.0)`, и ни
|
||
один не давал ограничения на самом деле: в `geocode()` сон стоял внутри
|
||
`if result is not None` (то есть только после УСПЕХА), в `_nominatim_lookup` —
|
||
между typo-вариантами, но не перед tier-1, а цикл бэкфилла
|
||
(`tasks/geocode_missing.py`) своей паузы не имел вовсе. Для неразрешимого адреса
|
||
выходило 5 запросов за 4 секунды и сразу следующий адрес без паузы — 194 события
|
||
«429 Too many requests» за неделю на проде.
|
||
|
||
Тест проверяет ПОВЕДЕНИЕ — расстояние между соседними исходящими запросами, — а
|
||
не наличие нового символа. Это принципиально: `from app.services.geocoder import
|
||
_nominatim_throttle` на origin/main дал бы ImportError, то есть «возможности
|
||
нет», а не «значение неверное», и красный ничего бы не доказывал. Здесь на
|
||
origin/main тесты падают именно на замере: соседние запросы идут вплотную.
|
||
|
||
Интервал подменяется на малый (`raising=False` — на origin/main такого атрибута
|
||
просто нет, подмена там безвредна), иначе тест ждал бы секунды реального времени.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import os
|
||
|
||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||
|
||
import time
|
||
from itertools import pairwise
|
||
from unittest.mock import patch
|
||
|
||
import httpx
|
||
import pytest
|
||
|
||
from app.services import geocoder
|
||
from app.services.geocoder import _nominatim_query
|
||
|
||
# Тестовый интервал: достаточно велик, чтобы отличаться от нуля на любом железе,
|
||
# и достаточно мал, чтобы тест шёл десятки миллисекунд, а не секунды.
|
||
_TEST_INTERVAL = 0.08
|
||
|
||
_REAL_ASYNC_CLIENT = httpx.AsyncClient
|
||
|
||
# Пустой ответ: Nominatim ничего не нашёл. Именно НЕУДАЧНЫЙ путь и разгонял темп —
|
||
# сон-после-успеха на нём не срабатывал.
|
||
_EMPTY_BODY: list[dict] = []
|
||
|
||
|
||
def _timestamping_transport(stamps: list[float]) -> httpx.MockTransport:
|
||
def handler(_request: httpx.Request) -> httpx.Response:
|
||
stamps.append(time.monotonic())
|
||
return httpx.Response(200, json=_EMPTY_BODY)
|
||
|
||
return httpx.MockTransport(handler)
|
||
|
||
|
||
def _gaps(stamps: list[float]) -> list[float]:
|
||
return [b - a for a, b in pairwise(stamps)]
|
||
|
||
|
||
@pytest.fixture
|
||
def throttle_reset(monkeypatch: pytest.MonkeyPatch) -> None:
|
||
"""Малый интервал + сброс «времени последнего вызова» между тестами.
|
||
|
||
Без сброса второй тест в файле унаследовал бы отметку от первого и мог бы
|
||
пройти/упасть по чужой причине.
|
||
"""
|
||
monkeypatch.setattr(geocoder, "_NOMINATIM_MIN_INTERVAL_SEC", _TEST_INTERVAL, raising=False)
|
||
monkeypatch.setattr(geocoder, "_nominatim_last_call_at", 0.0, raising=False)
|
||
|
||
|
||
async def test_consecutive_queries_are_spaced(throttle_reset: None) -> None:
|
||
"""Три запроса подряд разнесены не меньше чем на интервал.
|
||
|
||
На origin/main `_nominatim_query` не содержит паузы вообще, запросы уходят
|
||
вплотную — тест падает на первом же зазоре.
|
||
"""
|
||
stamps: list[float] = []
|
||
transport = _timestamping_transport(stamps)
|
||
|
||
async with _REAL_ASYNC_CLIENT(transport=transport) as client:
|
||
for _ in range(3):
|
||
await _nominatim_query(client, "заведомо ненаходимый адрес")
|
||
|
||
assert len(stamps) == 3, f"ожидали 3 запроса, ушло {len(stamps)}"
|
||
gaps = _gaps(stamps)
|
||
assert all(g >= _TEST_INTERVAL * 0.9 for g in gaps), (
|
||
f"запросы идут вплотную: зазоры {[round(g, 4) for g in gaps]}, "
|
||
f"ожидалось ≥ {_TEST_INTERVAL}"
|
||
)
|
||
|
||
|
||
async def test_failed_lookup_does_not_reset_the_pace(throttle_reset: None) -> None:
|
||
"""Два НЕУДАЧНЫХ поиска подряд не разгоняют темп на стыке.
|
||
|
||
Это ровно тот путь, который темп не сдерживал: сон стоял после успеха, а
|
||
неудача уходила к следующему адресу без паузы. Проверяем зазор МЕЖДУ
|
||
вызовами — между последним запросом первого и первым запросом второго.
|
||
"""
|
||
stamps: list[float] = []
|
||
transport = _timestamping_transport(stamps)
|
||
|
||
with patch(
|
||
"app.services.geocoder.httpx.AsyncClient",
|
||
lambda *a, **k: _REAL_ASYNC_CLIENT(transport=transport),
|
||
):
|
||
await geocoder._nominatim_lookup("ненаходимый адрес один")
|
||
boundary_index = len(stamps)
|
||
await geocoder._nominatim_lookup("ненаходимый адрес два")
|
||
|
||
assert boundary_index > 0 and len(stamps) > boundary_index, (
|
||
f"оба поиска должны были сходить в сеть: {len(stamps)} запросов, "
|
||
f"граница {boundary_index}"
|
||
)
|
||
boundary_gap = stamps[boundary_index] - stamps[boundary_index - 1]
|
||
assert boundary_gap >= _TEST_INTERVAL * 0.9, (
|
||
f"на стыке двух неудачных поисков зазор {boundary_gap:.4f}с "
|
||
f"(ожидалось ≥ {_TEST_INTERVAL}) — темп сбрасывается именно после неудачи"
|
||
)
|
||
|
||
|
||
async def test_first_call_is_not_delayed(throttle_reset: None) -> None:
|
||
"""Контроль: первый запрос не ждёт впустую.
|
||
|
||
Если бы «время последнего вызова» инициализировалось текущим временем, каждый
|
||
холодный старт платил бы интервал ни за что. Тест сломается и в случае, если
|
||
интервал по недосмотру станет применяться дважды за запрос.
|
||
"""
|
||
stamps: list[float] = []
|
||
transport = _timestamping_transport(stamps)
|
||
|
||
started = time.monotonic()
|
||
async with _REAL_ASYNC_CLIENT(transport=transport) as client:
|
||
await _nominatim_query(client, "первый запрос")
|
||
elapsed = time.monotonic() - started
|
||
|
||
assert len(stamps) == 1
|
||
assert elapsed < _TEST_INTERVAL, f"первый запрос прождал {elapsed:.4f}с — это лишнее"
|