From b7fc6c336120c9ff0765a95893604865cf294f88 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 6 Jun 2026 20:51:33 -0700 Subject: [PATCH] Genius 429 backoff: fail-fast gate instead of napping the import pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught live by the new lookup timing ("Genius track lookup took 242.4s"): the 429 handler slept the backoff (30/60/120s) in the CALLING thread and then re-raised anyway — the import pipeline waited 2x120s per track for lookups that still failed. Worse, the pre-flight backoff wait also slept while HOLDING the global Genius API lock, so every other Genius caller queued serially behind the nap. Now the backoff is a gate: a 429 opens a 30s->60s->120s window and re-raises immediately; any call inside the window raises GeniusRateLimitedError on the spot. The error subclasses requests.RequestException, so every existing caller (the import's source lookups catch RequestException and skip; the worker's per-item guards) already handles it as a one-line skip — lyrics and Genius tags are garnish, nothing is allowed to WAIT for them. Tests: backoff window fails fast (<0.5s vs the old full-window sleep), a 429 opens and escalates the gate without sleeping, the error is a RequestException (the no-call-site-changes hinge), success decays the gate. --- core/genius_client.py | 33 ++++++++++++---- tests/test_genius_backoff.py | 77 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 tests/test_genius_backoff.py diff --git a/core/genius_client.py b/core/genius_client.py index f255a2a3..08431926 100644 --- a/core/genius_client.py +++ b/core/genius_client.py @@ -16,8 +16,24 @@ _rate_limit_backoff = 0 # Extra backoff seconds after 429 _rate_limit_until = 0 # Timestamp until which all calls should wait +class GeniusRateLimitedError(requests.exceptions.RequestException): + """Raised IMMEDIATELY while Genius is inside a 429 backoff window. + + Subclasses RequestException so every existing caller (the import + pipeline's source lookups, the enrichment worker's per-item guards) + already treats it as a plain network failure: log one line, skip + Genius, move on. Lyrics/metadata garnish — nothing is allowed to WAIT + for it.""" + + def rate_limited(func): - """Decorator to enforce rate limiting on Genius API calls with exponential backoff on 429""" + """Decorator to enforce rate limiting on Genius API calls. + + The 429 backoff is a fail-fast GATE, not a sleep. The old version + slept the backoff in the calling thread — while HOLDING the API lock, + so every other Genius caller queued behind it — and then re-raised + anyway. The import pipeline measurably napped 2x120s per track + ("Genius track lookup took 242.4s") for lookups that still failed.""" @wraps(func) def wrapper(*args, **kwargs): global _last_api_call_time, _rate_limit_backoff, _rate_limit_until @@ -25,11 +41,12 @@ def rate_limited(func): with _api_call_lock: current_time = time.time() - # If in backoff period from a previous 429, wait it out + # Inside a backoff window: fail fast, never wait. if current_time < _rate_limit_until: - wait = _rate_limit_until - current_time - logger.debug(f"Genius rate limit backoff: waiting {wait:.1f}s") - time.sleep(wait) + remaining = _rate_limit_until - current_time + raise GeniusRateLimitedError( + f"Genius in 429 backoff for another {remaining:.0f}s — skipping" + ) time_since_last_call = time.time() - _last_api_call_time if time_since_last_call < MIN_API_INTERVAL: @@ -48,11 +65,11 @@ def rate_limited(func): return result except Exception as e: if "429" in str(e) or "rate limit" in str(e).lower(): - # Exponential backoff: 30s → 60s → 120s (cap at 120s) + # Open the gate: 30s → 60s → 120s (cap). Callers fail fast + # against it instead of sleeping here. _rate_limit_backoff = min(120, max(30, _rate_limit_backoff * 2) if _rate_limit_backoff else 30) _rate_limit_until = time.time() + _rate_limit_backoff - logger.warning(f"Genius 429 rate limit — backing off {_rate_limit_backoff}s") - time.sleep(_rate_limit_backoff) + logger.warning(f"Genius 429 rate limit — gating calls for {_rate_limit_backoff}s") raise e return wrapper diff --git a/tests/test_genius_backoff.py b/tests/test_genius_backoff.py new file mode 100644 index 00000000..c187535a --- /dev/null +++ b/tests/test_genius_backoff.py @@ -0,0 +1,77 @@ +"""Genius 429 backoff must be a fail-fast gate, never a sleep. + +The old wrapper slept the backoff (30-120s) in the calling thread — while +holding the global API lock, serializing every other Genius caller behind +it — and then re-raised anyway. The import pipeline measurably napped +2x120s per track ("Genius track lookup took 242.4s") for lookups that +still failed. +""" + +from __future__ import annotations + +import time + +import pytest +import requests + +import core.genius_client as gc + + +def _fresh(monkeypatch): + monkeypatch.setattr(gc, '_rate_limit_until', 0) + monkeypatch.setattr(gc, '_rate_limit_backoff', 0) + monkeypatch.setattr(gc, '_last_api_call_time', 0) + + +def test_backoff_window_fails_fast_without_sleeping(monkeypatch): + _fresh(monkeypatch) + monkeypatch.setattr(gc, '_rate_limit_until', time.time() + 120) + + @gc.rate_limited + def call(): + raise AssertionError('must not reach the API during a backoff window') + + started = time.time() + with pytest.raises(gc.GeniusRateLimitedError): + call() + assert time.time() - started < 0.5 # the old code slept the full window here + + +def test_429_opens_the_gate_without_sleeping_and_escalates(monkeypatch): + _fresh(monkeypatch) + + @gc.rate_limited + def call(): + raise requests.exceptions.HTTPError('429 Client Error: Too Many Requests') + + started = time.time() + with pytest.raises(requests.exceptions.HTTPError): + call() + assert time.time() - started < 0.5 # old code slept 30s+ here + assert gc._rate_limit_until > time.time() # the gate is open + assert gc._rate_limit_backoff == 30 + + # Next 429 (after the window expires) doubles the gate: 30 -> 60 + monkeypatch.setattr(gc, '_rate_limit_until', 0) + monkeypatch.setattr(gc, '_last_api_call_time', 0) + with pytest.raises(requests.exceptions.HTTPError): + call() + assert gc._rate_limit_backoff == 60 + + +def test_rate_limited_error_is_a_request_exception(): + # The design hinge: existing callers (import source lookups, worker item + # guards) catch RequestException and skip — no call-site changes needed. + assert issubclass(gc.GeniusRateLimitedError, requests.RequestException) + + +def test_success_decays_backoff(monkeypatch): + _fresh(monkeypatch) + monkeypatch.setattr(gc, '_rate_limit_backoff', 30) + + @gc.rate_limited + def call(): + return 'ok' + + assert call() == 'ok' + assert gc._rate_limit_backoff == 25