fix(worker): retry transient MusicBrainz failures with backoff
The matching stage makes up to three musicbrainzngs calls per job, none of which retried. A dropped TLS connection (surfacing as NetworkError, "<urlopen error [SSL: UNEXPECTED_EOF_WHILE_READING]>") or a 503 rate-limit (ResponseError) failed the entire pipeline for the job, which then re-queued and re-ran from scratch — ~36% of jobs churned this way over a 6h window. Wrap the three MB calls in _with_retry (4 attempts, 1s/2s/4s exponential backoff), catching only the two transient musicbrainzngs error types so real bugs still propagate. musicbrainzngs stays lazily imported to keep module import offline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import time
|
||||||
from difflib import SequenceMatcher
|
from difflib import SequenceMatcher
|
||||||
|
|
||||||
from lyra_worker.types import MBTarget
|
from lyra_worker.types import MBTarget
|
||||||
@@ -5,6 +6,29 @@ from lyra_worker.types import MBTarget
|
|||||||
_MIN_TITLE_RATIO = 0.6
|
_MIN_TITLE_RATIO = 0.6
|
||||||
|
|
||||||
|
|
||||||
|
def _with_retry(fn, *, attempts: int = 4, base_delay: float = 1.0, sleep=time.sleep):
|
||||||
|
"""Call `fn`, retrying transient MusicBrainz failures with exponential backoff.
|
||||||
|
|
||||||
|
A dropped TLS connection to the MusicBrainz webservice surfaces as
|
||||||
|
musicbrainzngs.NetworkError (`<urlopen error [SSL: UNEXPECTED_EOF_WHILE_READING]>`),
|
||||||
|
and MusicBrainz's own 503 rate-limit surfaces as ResponseError; both are
|
||||||
|
transient, so we retry rather than failing the whole pipeline for the job.
|
||||||
|
Any other exception (bug, non-transient) propagates immediately.
|
||||||
|
|
||||||
|
musicbrainzngs is imported lazily so importing this module stays offline.
|
||||||
|
"""
|
||||||
|
import musicbrainzngs
|
||||||
|
|
||||||
|
transient = (musicbrainzngs.NetworkError, musicbrainzngs.ResponseError)
|
||||||
|
for i in range(attempts):
|
||||||
|
try:
|
||||||
|
return fn()
|
||||||
|
except transient:
|
||||||
|
if i == attempts - 1:
|
||||||
|
raise
|
||||||
|
sleep(base_delay * (2**i))
|
||||||
|
|
||||||
|
|
||||||
def _ratio(a: str, b: str) -> float:
|
def _ratio(a: str, b: str) -> float:
|
||||||
return SequenceMatcher(None, a.strip().casefold(), b.strip().casefold()).ratio()
|
return SequenceMatcher(None, a.strip().casefold(), b.strip().casefold()).ratio()
|
||||||
|
|
||||||
@@ -68,7 +92,7 @@ class MusicBrainzResolver:
|
|||||||
|
|
||||||
musicbrainzngs.set_useragent(self._app, self._version, self._contact)
|
musicbrainzngs.set_useragent(self._app, self._version, self._contact)
|
||||||
|
|
||||||
res = musicbrainzngs.search_release_groups(query=album, artist=artist, limit=5)
|
res = _with_retry(lambda: musicbrainzngs.search_release_groups(query=album, artist=artist, limit=5))
|
||||||
groups = res.get("release-group-list", [])
|
groups = res.get("release-group-list", [])
|
||||||
rg = _best_release_group(artist, album, groups)
|
rg = _best_release_group(artist, album, groups)
|
||||||
if rg is None:
|
if rg is None:
|
||||||
@@ -93,11 +117,11 @@ class MusicBrainzResolver:
|
|||||||
titles: tuple[str, ...] = ()
|
titles: tuple[str, ...] = ()
|
||||||
rgid = rg.get("id")
|
rgid = rg.get("id")
|
||||||
if rgid:
|
if rgid:
|
||||||
rgfull = musicbrainzngs.get_release_group_by_id(rgid, includes=["releases"])
|
rgfull = _with_retry(lambda: musicbrainzngs.get_release_group_by_id(rgid, includes=["releases"]))
|
||||||
releases = rgfull.get("release-group", {}).get("release-list", [])
|
releases = rgfull.get("release-group", {}).get("release-list", [])
|
||||||
if releases:
|
if releases:
|
||||||
rel = musicbrainzngs.get_release_by_id(
|
rel = _with_retry(
|
||||||
releases[0]["id"], includes=["recordings"]
|
lambda: musicbrainzngs.get_release_by_id(releases[0]["id"], includes=["recordings"])
|
||||||
).get("release", {})
|
).get("release", {})
|
||||||
tracks = []
|
tracks = []
|
||||||
for medium in rel.get("medium-list", []):
|
for medium in rel.get("medium-list", []):
|
||||||
|
|||||||
@@ -1,4 +1,7 @@
|
|||||||
from lyra_worker._musicbrainz import _best_release_group
|
import musicbrainzngs
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from lyra_worker._musicbrainz import _best_release_group, _with_retry
|
||||||
|
|
||||||
# Offline unit tests for the pure release-group selection logic. The live resolve()
|
# Offline unit tests for the pure release-group selection logic. The live resolve()
|
||||||
# path (network) is covered by test_musicbrainz_live.py.
|
# path (network) is covered by test_musicbrainz_live.py.
|
||||||
@@ -60,3 +63,73 @@ def test_prefers_credited_artist_over_same_titled_other_artist_album():
|
|||||||
]
|
]
|
||||||
best = _best_release_group("Lake Street Dive", "Fun Machine", groups)
|
best = _best_release_group("Lake Street Dive", "Fun Machine", groups)
|
||||||
assert best["artist-credit"][0]["artist"]["name"] == "Lake Street Dive"
|
assert best["artist-credit"][0]["artist"]["name"] == "Lake Street Dive"
|
||||||
|
|
||||||
|
|
||||||
|
# --- _with_retry: transient MusicBrainz failures (SSL EOF, 503 rate-limit) ---
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_returns_value_without_backoff_when_first_call_succeeds():
|
||||||
|
sleeps: list[float] = []
|
||||||
|
calls = {"n": 0}
|
||||||
|
|
||||||
|
def fn():
|
||||||
|
calls["n"] += 1
|
||||||
|
return "ok"
|
||||||
|
|
||||||
|
assert _with_retry(fn, sleep=sleeps.append) == "ok"
|
||||||
|
assert calls["n"] == 1
|
||||||
|
assert sleeps == [] # no backoff when nothing failed
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_recovers_from_transient_network_error():
|
||||||
|
# The real symptom: <urlopen error [SSL: UNEXPECTED_EOF_WHILE_READING]> arrives
|
||||||
|
# as musicbrainzngs.NetworkError. A couple of retries clears it.
|
||||||
|
sleeps: list[float] = []
|
||||||
|
calls = {"n": 0}
|
||||||
|
|
||||||
|
def fn():
|
||||||
|
calls["n"] += 1
|
||||||
|
if calls["n"] < 3:
|
||||||
|
raise musicbrainzngs.NetworkError("SSL: UNEXPECTED_EOF_WHILE_READING")
|
||||||
|
return "recovered"
|
||||||
|
|
||||||
|
assert _with_retry(fn, sleep=sleeps.append) == "recovered"
|
||||||
|
assert calls["n"] == 3
|
||||||
|
assert sleeps == [1.0, 2.0] # exponential backoff before each retry
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_recovers_from_transient_response_error():
|
||||||
|
# MusicBrainz's own 503 rate-limit surfaces as ResponseError.
|
||||||
|
calls = {"n": 0}
|
||||||
|
|
||||||
|
def fn():
|
||||||
|
calls["n"] += 1
|
||||||
|
if calls["n"] < 2:
|
||||||
|
raise musicbrainzngs.ResponseError("503 Service Unavailable")
|
||||||
|
return "ok"
|
||||||
|
|
||||||
|
assert _with_retry(fn, sleep=lambda _: None) == "ok"
|
||||||
|
assert calls["n"] == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_reraises_after_exhausting_attempts():
|
||||||
|
sleeps: list[float] = []
|
||||||
|
|
||||||
|
def fn():
|
||||||
|
raise musicbrainzngs.NetworkError("still down")
|
||||||
|
|
||||||
|
with pytest.raises(musicbrainzngs.NetworkError):
|
||||||
|
_with_retry(fn, attempts=4, sleep=sleeps.append)
|
||||||
|
assert sleeps == [1.0, 2.0, 4.0] # 3 backoffs across 4 attempts
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_does_not_swallow_non_transient_errors():
|
||||||
|
calls = {"n": 0}
|
||||||
|
|
||||||
|
def fn():
|
||||||
|
calls["n"] += 1
|
||||||
|
raise ValueError("bug, not a network blip")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
_with_retry(fn, sleep=lambda _: None)
|
||||||
|
assert calls["n"] == 1 # non-transient errors propagate immediately, no retry
|
||||||
|
|||||||
Reference in New Issue
Block a user