fix(tradein/geocoder): темп Nominatim сдерживается перед запросом, а не после успеха (#2953)
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
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>
This commit is contained in:
parent
c93d6cbde1
commit
5222f477db
2 changed files with 178 additions and 5 deletions
|
|
@ -16,6 +16,7 @@ from __future__ import annotations
|
|||
import asyncio
|
||||
import logging
|
||||
import re
|
||||
import time
|
||||
from dataclasses import dataclass, replace
|
||||
from typing import Literal
|
||||
|
||||
|
|
@ -29,6 +30,43 @@ from app.services import dadata
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# ── Общий ограничитель темпа обращений к Nominatim (#2953) ──────────────────
|
||||
#
|
||||
# Политика Nominatim — 1 req/sec. Раньше её изображали три разрозненных
|
||||
# `asyncio.sleep(1.0)`, и ни один не давал ограничения на самом деле:
|
||||
#
|
||||
# geocode() — сон стоял ВНУТРИ `if result is not None`, то есть
|
||||
# только после УСПЕХА; после неудачи паузы не было;
|
||||
# _nominatim_lookup() — сон МЕЖДУ typo-вариантами, но не перед tier-1;
|
||||
# цикл бэкфилла — своей паузы не имеет вовсе.
|
||||
#
|
||||
# Для неразрешимого адреса получалось 5 запросов за 4 секунды, и сразу
|
||||
# следующий адрес без паузы. А неразрешимые адреса — не редкость: на
|
||||
# 20.08.2026 из 1471 активного объявления без координат 1424 лежали в
|
||||
# семидневном backoff, то есть череда неудач случается КАЖДЫЙ прогон
|
||||
# бэкфилла. Отсюда 194 события «429 Too many requests» за неделю.
|
||||
#
|
||||
# Здесь пауза берётся ПЕРЕД каждым запросом и одна на все точки вызова, а не
|
||||
# после успеха и не на каждую по отдельности.
|
||||
#
|
||||
# ponytail: ограничитель внутрипроцессный. Если бэкфилл и пользовательские
|
||||
# запросы разъедут по разным процессам, их темпы снова сложатся — тогда
|
||||
# понадобится общий счётчик (Redis). Пока доминирующий источник один
|
||||
# (бэкфилл, сотни запросов за прогон), внутрипроцессного достаточно.
|
||||
_NOMINATIM_MIN_INTERVAL_SEC = 1.0
|
||||
_nominatim_gate = asyncio.Lock()
|
||||
_nominatim_last_call_at = 0.0
|
||||
|
||||
|
||||
async def _nominatim_throttle() -> None:
|
||||
"""Держит паузу ≥ _NOMINATIM_MIN_INTERVAL_SEC между обращениями к Nominatim."""
|
||||
global _nominatim_last_call_at
|
||||
async with _nominatim_gate:
|
||||
overdue = _NOMINATIM_MIN_INTERVAL_SEC - (time.monotonic() - _nominatim_last_call_at)
|
||||
if overdue > 0:
|
||||
await asyncio.sleep(overdue)
|
||||
_nominatim_last_call_at = time.monotonic()
|
||||
|
||||
|
||||
# ── Result type ──────────────────────────────────────────────────────────────
|
||||
@dataclass(frozen=True, slots=True)
|
||||
|
|
@ -712,6 +750,7 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No
|
|||
(`address.state`) — отсекает кандидатов ЯВНО из другого региона (Тюмень и
|
||||
т.п.), даже если координаты попали в генеральный bbox.
|
||||
"""
|
||||
await _nominatim_throttle()
|
||||
response = await client.get(
|
||||
"https://nominatim.openstreetmap.org/search",
|
||||
params={
|
||||
|
|
@ -790,7 +829,6 @@ async def _nominatim_lookup(address: str, city_hint: str | None = None) -> Geoco
|
|||
# Tier 2: typo-variants
|
||||
if item is None:
|
||||
for variant in _typo_variants(address, limit=4):
|
||||
await asyncio.sleep(1.0) # Nominatim 1 req/sec policy
|
||||
variant_city, _ = _resolve_city_for_geocode(variant, city_hint)
|
||||
variant_query = f"{variant_city}, {variant}" if variant_city else variant
|
||||
item = await _nominatim_query(client, variant_query)
|
||||
|
|
@ -881,6 +919,7 @@ async def _dadata_suggest(query: str, limit: int = 8) -> list[GeocodeSuggestion]
|
|||
|
||||
async def _nominatim_query_multi(client: httpx.AsyncClient, query: str, limit: int) -> list[dict]:
|
||||
"""Один Nominatim search с фильтром по bbox области (region 66). Возвращает up to N items."""
|
||||
await _nominatim_throttle()
|
||||
response = await client.get(
|
||||
"https://nominatim.openstreetmap.org/search",
|
||||
params={
|
||||
|
|
@ -952,7 +991,6 @@ async def _nominatim_query_city_aware(
|
|||
if city_specified:
|
||||
return await _nominatim_query_multi(client, query, limit)
|
||||
ekb_data = await _nominatim_query_multi(client, f"{query}, Екатеринбург", limit)
|
||||
await asyncio.sleep(1.0) # Nominatim 1 req/sec policy — два запроса подряд
|
||||
bare_data = await _nominatim_query_multi(client, query, limit)
|
||||
return _dedupe_nominatim_items(ekb_data, bare_data)[:limit]
|
||||
|
||||
|
|
@ -986,7 +1024,6 @@ async def _nominatim_suggest(
|
|||
# Tier 2: typo-варианты если оригинал пустой
|
||||
if not data:
|
||||
for variant in _typo_variants(query, limit=3):
|
||||
await asyncio.sleep(1.0) # Nominatim 1 req/sec
|
||||
variant_city, variant_specified = _resolve_city_for_geocode(variant, city_hint)
|
||||
data = await _nominatim_query_city_aware(
|
||||
client, variant, variant_city, variant_specified, limit
|
||||
|
|
@ -1891,8 +1928,6 @@ async def _geocode_resolve(
|
|||
result = replace(result, city_ambiguous=city_ambiguous)
|
||||
await asyncio.to_thread(_cache_put, db, addr_norm, result)
|
||||
logger.info("geocode nominatim: %s → (%.5f, %.5f)", addr_norm, result.lat, result.lon)
|
||||
# Nominatim rate-limit policy: 1 req/sec — спим после успешного запроса
|
||||
await asyncio.sleep(1.0)
|
||||
return result
|
||||
except Exception:
|
||||
logger.exception("nominatim geocoder failed")
|
||||
|
|
@ -2018,6 +2053,7 @@ async def _nominatim_reverse(lat: float, lon: float) -> ReverseGeocodeResult | N
|
|||
"Accept-Language": "ru,en;q=0.8",
|
||||
}
|
||||
async with httpx.AsyncClient(timeout=10.0, headers=headers) as client:
|
||||
await _nominatim_throttle()
|
||||
response = await client.get(
|
||||
"https://nominatim.openstreetmap.org/reverse",
|
||||
params={
|
||||
|
|
|
|||
137
tradein-mvp/backend/tests/test_2953_nominatim_throttle.py
Normal file
137
tradein-mvp/backend/tests/test_2953_nominatim_throttle.py
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
"""Темп обращений к 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}с — это лишнее"
|
||||
Loading…
Add table
Reference in a new issue