fix: hold one connection to Lidarr open, and retry what deserves retrying
Build and publish container / build (pull_request) Successful in 6m0s
Build and publish container / build (pull_request) Successful in 6m0s
Indexing makes two requests per artist, and more when the album fallback fires. urllib opens a new TCP connection and performs a new DNS lookup for every one of them, so a large library becomes thousands of lookups inside a few minutes. That is enough to exhaust a container's resolver, and the result is "[Errno -3] Try again" on every artist at once -- a failure caused entirely by how the requests were made rather than by anything wrong with Lidarr. Add a transport that keeps one connection open per host, so the name is resolved once and the socket is reused. It retries once on a connection the server has already closed, since a stale keep-alive announces itself only on use. Retries were previously declined on the grounds that Lidarr is on the same LAN. That is not a safe assumption -- it may sit behind a public hostname and a reverse proxy -- and a transient failure currently costs an artist their entire entry for that pass. Transient failures are now retried with a backoff. HTTP 500 is deliberately excluded: it is an exception inside Lidarr's serialisation, not a busy server, and three attempts only delay finding that out. The same distinction gates the album probe added alongside this. Naming the offending album costs one request per album of that artist, which is worth it for a deterministic fault and actively harmful during a network-wide one, where every artist fails and probing each of them multiplies the load responsible. The keep-alive transport is tested against a real local HTTP server rather than a fake, because connection reuse and status mapping are exactly the properties a fake would assume rather than demonstrate.
This commit is contained in:
@@ -1,7 +1,9 @@
|
||||
import http.server
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import threading
|
||||
import urllib.error
|
||||
import urllib.parse
|
||||
|
||||
@@ -201,6 +203,19 @@ class FakeLidarr:
|
||||
url, 500, "Internal Server Error", {}, io.BytesIO(b'{"message": "boom"}')
|
||||
)
|
||||
|
||||
album_ids = query.get("albumIds")
|
||||
if path == "album" and album_ids:
|
||||
album_id = int(album_ids)
|
||||
if ("albumid", album_id) in self.fail:
|
||||
raise urllib.error.HTTPError(
|
||||
url,
|
||||
500,
|
||||
"Internal Server Error",
|
||||
{},
|
||||
io.BytesIO(b'{"message": "Sequence contains more than one element"}'),
|
||||
)
|
||||
return json.dumps([row for row in self.albums if row["id"] == album_id])
|
||||
|
||||
if path == "artist":
|
||||
return json.dumps(self.artists)
|
||||
source = {"album": self.albums, "track": self.tracks, "trackfile": self.files}[path]
|
||||
@@ -228,3 +243,53 @@ def now_playing():
|
||||
"mbid": "",
|
||||
"@attr": {"nowplaying": "true"},
|
||||
}
|
||||
|
||||
|
||||
class _CountingServer(http.server.ThreadingHTTPServer):
|
||||
"""Counts accepted connections, which is what connection reuse is about."""
|
||||
|
||||
daemon_threads = True
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
self.connections = 0
|
||||
super().__init__(*args, **kwargs)
|
||||
|
||||
def process_request(self, request, client_address):
|
||||
self.connections += 1
|
||||
super().process_request(request, client_address)
|
||||
|
||||
|
||||
class _Handler(http.server.BaseHTTPRequestHandler):
|
||||
# Without HTTP/1.1 the server closes after every response and no client
|
||||
# could reuse anything, which would make the test prove nothing.
|
||||
protocol_version = "HTTP/1.1"
|
||||
|
||||
def do_GET(self):
|
||||
if self.path.startswith("/boom"):
|
||||
body, status = b'{"message": "boom"}', 500
|
||||
else:
|
||||
body = json.dumps(
|
||||
{"path": self.path, "key": self.headers.get("X-Api-Key")}
|
||||
).encode()
|
||||
status = 200
|
||||
self.send_response(status)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.send_header("Content-Length", str(len(body)))
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
|
||||
def log_message(self, *args):
|
||||
pass
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def http_server():
|
||||
"""A real local HTTP server, for the one component that talks sockets."""
|
||||
server = _CountingServer(("127.0.0.1", 0), _Handler)
|
||||
thread = threading.Thread(target=server.serve_forever, daemon=True)
|
||||
thread.start()
|
||||
try:
|
||||
yield server, f"http://127.0.0.1:{server.server_port}"
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
|
||||
Reference in New Issue
Block a user