From 29ac3043b4aa828fba9b5ff2be8e1234175b5478 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Thu, 17 Sep 2026 14:02:32 +0500 Subject: [PATCH] =?UTF-8?q?fix(tradein/proxy=5Fpool):=20=D0=B7=D0=B0=D1=89?= =?UTF-8?q?=D0=B8=D1=82=D0=B0=20=D0=B1=D0=B0=D0=BD=D0=B0=20=D0=BD=D0=B5=20?= =?UTF-8?q?=D1=81=D1=87=D0=B8=D1=82=D0=B0=D0=B5=D1=82=20=D1=83=D0=B7=D0=B5?= =?UTF-8?q?=D0=BB=20=D1=81=20=D0=B8=D1=81=D1=82=D1=91=D0=BA=D1=88=D0=B5?= =?UTF-8?q?=D0=B9=20=D0=B0=D1=80=D0=B5=D0=BD=D0=B4=D0=BE=D0=B9,=20=D1=83?= =?UTF-8?q?=D1=81=D0=BB=D0=BE=D0=B2=D0=B8=D1=8F=20=D1=80=D0=B5=D0=B7=D0=B5?= =?UTF-8?q?=D1=80=D0=B2=D0=B0=20=D0=BF=D1=80=D0=BE=D0=B2=D0=B5=D1=80=D0=B5?= =?UTF-8?q?=D0=BD=D1=8B=20=D0=BF=D0=BE=20=D0=B7=D0=BD=D0=B0=D1=87=D0=B5?= =?UTF-8?q?=D0=BD=D0=B8=D1=8E=20(#3299)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью PR #3565: у нового предиката резерва две части не проверялись ни одним тестом. Снятие `other.expires_at` из подзапроса acquire и снятие `other.consecutive_fails` из подзапроса mark_banned оставляли прогон test_3299 + services/test_proxy_pool.py зелёным (72 passed). Проверено тем же способом, и нашлась третья такая часть: `other.expires_at` в mark_banned. Добавлено по live-тесту на каждое условие: просроченный резерв в acquire, резерв в карантине и просроченный резерв в mark_banned. Попутно найден дефект рядом. Внешний отбор mark_banned («есть ли у источника другой узел») срок аренды не проверял, хотя acquire такой узел не выдаёт, а докстринг обещает «тем же правилом, что acquire()». Если у источника остались только просроченные узлы, бан уходил последнему живому, и источник оставался без прокси, пока healthcheck не наберёт просроченным узлам отказов. Добавлена проверка `sp.expires_at` и тест на этот случай. Co-Authored-By: Claude Opus 5 --- .../backend/app/services/proxy_pool.py | 4 ++ .../test_3299_fallback_counts_any_nodes.py | 58 +++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/tradein-mvp/backend/app/services/proxy_pool.py b/tradein-mvp/backend/app/services/proxy_pool.py index a57519d8..66c3153d 100644 --- a/tradein-mvp/backend/app/services/proxy_pool.py +++ b/tradein-mvp/backend/app/services/proxy_pool.py @@ -996,6 +996,10 @@ def mark_banned(db: Session, proxy_id: int, *, source: str, reason: str | None = WHERE sp.id <> CAST(:proxy_id AS bigint) AND sp.enabled AND sp.consecutive_fails < CAST(:max_fails AS integer) + -- Как в acquire(): узел с истёкшей арендой порта остаётся enabled, + -- но не выдаётся. Без этой строки он «спасал» источник, и бан уходил + -- последнему живому узлу (ревью #3565 к #3299). + AND (sp.expires_at IS NULL OR sp.expires_at > now()) AND NOT EXISTS ( SELECT 1 FROM scrape_proxy_source_bans b diff --git a/tradein-mvp/backend/tests/test_3299_fallback_counts_any_nodes.py b/tradein-mvp/backend/tests/test_3299_fallback_counts_any_nodes.py index 002ce1a5..70b378ec 100644 --- a/tradein-mvp/backend/tests/test_3299_fallback_counts_any_nodes.py +++ b/tradein-mvp/backend/tests/test_3299_fallback_counts_any_nodes.py @@ -63,16 +63,17 @@ def db() -> Iterator[Session]: conn.close() -def _node(db: Session, affinity: str, *, fails: int = 0) -> int: +def _node(db: Session, affinity: str, *, fails: int = 0, expired: bool = False) -> int: proxy_id = db.execute( text( """ - INSERT INTO scrape_proxies (url, provider_affinity, consecutive_fails) - VALUES ('http://t3299-' || gen_random_uuid(), :aff, :fails) + INSERT INTO scrape_proxies (url, provider_affinity, consecutive_fails, expires_at) + VALUES ('http://t3299-' || gen_random_uuid(), :aff, :fails, + CASE WHEN :expired THEN now() - interval '1 minute' END) RETURNING id """ ), - {"aff": affinity, "fails": fails}, + {"aff": affinity, "fails": fails, "expired": expired}, ).scalar_one() db.commit() return int(proxy_id) @@ -148,6 +149,19 @@ def test_unhealthy_any_node_is_not_a_backup(db: Session) -> None: assert _leased_by(db, dedicated) is None +def test_expired_any_node_is_not_a_backup(db: Session) -> None: + """Узел с истёкшей арендой порта acquire не выдаёт — резервом он тоже не считается. + + Отдельно от карантина: у предиката резерва два независимых условия пригодности, и + каждое проверяется своим узлом (ревью PR #3565: без этого теста снятие проверки + `expires_at` из подзапроса оставляло сьют зелёным).""" + dedicated = _node(db, "avito") + _node(db, "any", expired=True) + + assert acquire(db, "domclick", run_id=None) is None + assert _leased_by(db, dedicated) is None + + # ── mark_banned: защита считает так же, как acquire ────────────────────────── @@ -179,3 +193,39 @@ def test_mark_banned_still_protects_when_dedicated_node_has_no_usable_backup( assert mark_banned(db, banned, source="cian") == "protected" lease = acquire(db, "cian", run_id=None) assert lease is not None and lease.id == banned + + +def test_mark_banned_unhealthy_backup_does_not_count(db: Session) -> None: + """Резерв выделенного узла в карантине по consecutive_fails — для cian узла нет, + защита держит. Сам резерв кандидатом cian не считается по той же причине (карантин + проверяется и во внешнем отборе), так что ответ решает именно предикат резерва.""" + banned = _node(db, "any") + _node(db, "avito") + _ban(db, banned, "avito") + _node(db, "any", fails=MAX_CONSECUTIVE_FAILS) + + assert mark_banned(db, banned, source="cian") == "protected" + + +def test_mark_banned_expired_backup_does_not_count(db: Session) -> None: + """Резерв выделенного узла с истёкшей арендой — для cian узла нет, защита держит.""" + banned = _node(db, "any") + _node(db, "avito") + _ban(db, banned, "avito") + _node(db, "any", expired=True) + + assert mark_banned(db, banned, source="cian") == "protected" + + +def test_mark_banned_expired_node_does_not_save_the_source(db: Session) -> None: + """Внешний отбор: у cian остался только 'any'-узел с истёкшей арендой. acquire его не + выдаёт, значит банимый узел последний — бан не пишется, и cian получает этот узел. + + До правки защита засчитывала просроченный узел (пока healthcheck не наберёт ему + отказов), бан уходил, а cian оставался ни с чем.""" + banned = _node(db, "any") + _node(db, "any", expired=True) + + assert mark_banned(db, banned, source="cian") == "protected" + lease = acquire(db, "cian", run_id=None) + assert lease is not None and lease.id == banned