Genius 429 backoff: fail-fast gate instead of napping the import pipeline
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.
This commit is contained in:
parent
88da265ef4
commit
b7fc6c3361
2 changed files with 102 additions and 8 deletions
|
|
@ -16,8 +16,24 @@ _rate_limit_backoff = 0 # Extra backoff seconds after 429
|
||||||
_rate_limit_until = 0 # Timestamp until which all calls should wait
|
_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):
|
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)
|
@wraps(func)
|
||||||
def wrapper(*args, **kwargs):
|
def wrapper(*args, **kwargs):
|
||||||
global _last_api_call_time, _rate_limit_backoff, _rate_limit_until
|
global _last_api_call_time, _rate_limit_backoff, _rate_limit_until
|
||||||
|
|
@ -25,11 +41,12 @@ def rate_limited(func):
|
||||||
with _api_call_lock:
|
with _api_call_lock:
|
||||||
current_time = time.time()
|
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:
|
if current_time < _rate_limit_until:
|
||||||
wait = _rate_limit_until - current_time
|
remaining = _rate_limit_until - current_time
|
||||||
logger.debug(f"Genius rate limit backoff: waiting {wait:.1f}s")
|
raise GeniusRateLimitedError(
|
||||||
time.sleep(wait)
|
f"Genius in 429 backoff for another {remaining:.0f}s — skipping"
|
||||||
|
)
|
||||||
|
|
||||||
time_since_last_call = time.time() - _last_api_call_time
|
time_since_last_call = time.time() - _last_api_call_time
|
||||||
if time_since_last_call < MIN_API_INTERVAL:
|
if time_since_last_call < MIN_API_INTERVAL:
|
||||||
|
|
@ -48,11 +65,11 @@ def rate_limited(func):
|
||||||
return result
|
return result
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
if "429" in str(e) or "rate limit" in str(e).lower():
|
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_backoff = min(120, max(30, _rate_limit_backoff * 2) if _rate_limit_backoff else 30)
|
||||||
_rate_limit_until = time.time() + _rate_limit_backoff
|
_rate_limit_until = time.time() + _rate_limit_backoff
|
||||||
logger.warning(f"Genius 429 rate limit — backing off {_rate_limit_backoff}s")
|
logger.warning(f"Genius 429 rate limit — gating calls for {_rate_limit_backoff}s")
|
||||||
time.sleep(_rate_limit_backoff)
|
|
||||||
raise e
|
raise e
|
||||||
return wrapper
|
return wrapper
|
||||||
|
|
||||||
|
|
|
||||||
77
tests/test_genius_backoff.py
Normal file
77
tests/test_genius_backoff.py
Normal file
|
|
@ -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
|
||||||
Loading…
Reference in a new issue