From 5222f477db0a3a473056712778af4cdf74dcfec8 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 20 Aug 2026 12:46:41 +0500 Subject: [PATCH] =?UTF-8?q?fix(tradein/geocoder):=20=D1=82=D0=B5=D0=BC?= =?UTF-8?q?=D0=BF=20Nominatim=20=D1=81=D0=B4=D0=B5=D1=80=D0=B6=D0=B8=D0=B2?= =?UTF-8?q?=D0=B0=D0=B5=D1=82=D1=81=D1=8F=20=D0=BF=D0=B5=D1=80=D0=B5=D0=B4?= =?UTF-8?q?=20=D0=B7=D0=B0=D0=BF=D1=80=D0=BE=D1=81=D0=BE=D0=BC,=20=D0=B0?= =?UTF-8?q?=20=D0=BD=D0=B5=20=D0=BF=D0=BE=D1=81=D0=BB=D0=B5=20=D1=83=D1=81?= =?UTF-8?q?=D0=BF=D0=B5=D1=85=D0=B0=20(#2953)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit На проде — 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 --- tradein-mvp/backend/app/services/geocoder.py | 46 +++++- .../tests/test_2953_nominatim_throttle.py | 137 ++++++++++++++++++ 2 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_2953_nominatim_throttle.py diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 2d3c7096..7863b308 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -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={ diff --git a/tradein-mvp/backend/tests/test_2953_nominatim_throttle.py b/tradein-mvp/backend/tests/test_2953_nominatim_throttle.py new file mode 100644 index 00000000..cbe363a7 --- /dev/null +++ b/tradein-mvp/backend/tests/test_2953_nominatim_throttle.py @@ -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}с — это лишнее"