From 68fd07fba8c0662de9fb5f053f867a852b47d3a2 Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Sun, 4 Oct 2026 11:34:15 -0600 Subject: [PATCH] fix(library): back off after a failed index instead of retrying for every search result --- shelfmark/core/library_index.py | 11 ++++++++ tests/core/test_library_index.py | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/shelfmark/core/library_index.py b/shelfmark/core/library_index.py index 26d25db8a..3e91a9805 100644 --- a/shelfmark/core/library_index.py +++ b/shelfmark/core/library_index.py @@ -37,6 +37,9 @@ logger = setup_logger(__name__) _CACHE_TTL_SECONDS = 600 # Re-index a library at most every 10 minutes unless it changed. +# After a failed index, answer from the stale cache for this long before trying again, so a +# search page with dozens of results costs one failed attempt and one warning, not one each. +_FAILURE_BACKOFF_SECONDS = 60 @dataclass @@ -44,6 +47,7 @@ class _CacheSlot: entries: list[LibraryEntry] | None = None fetched_at: float = 0.0 fingerprint: object | None = None + failed_at: float | None = None _lock = threading.Lock() @@ -61,11 +65,17 @@ def _store(provider_name: str, entries: list[LibraryEntry], fingerprint: object slot.entries = entries slot.fetched_at = time.monotonic() slot.fingerprint = fingerprint + slot.failed_at = None def _entries_for(provider: LibraryProvider) -> list[LibraryEntry]: """Cached entries for one provider, re-indexed past the TTL or when the library changed.""" slot = _slot(provider.name) + with _lock: + if slot.failed_at is not None and time.monotonic() - slot.failed_at < ( + _FAILURE_BACKOFF_SECONDS + ): + return slot.entries or [] try: fingerprint = provider.fingerprint() with _lock: @@ -78,6 +88,7 @@ def _entries_for(provider: LibraryProvider) -> list[LibraryEntry]: except Exception as exc: # noqa: BLE001 - any failure must fail open logger.warning("library check: %s unavailable (%s); failing open", provider.describe(), exc) with _lock: + slot.failed_at = time.monotonic() return slot.entries or [] # Use the stale cache if we have one. _store(provider.name, entries, fingerprint) diff --git a/tests/core/test_library_index.py b/tests/core/test_library_index.py index 74c86fbf9..08d55d91e 100644 --- a/tests/core/test_library_index.py +++ b/tests/core/test_library_index.py @@ -234,6 +234,53 @@ def test_provider_error_keeps_answering_from_the_stale_cache( assert len(warnings) == 1 +def test_a_failing_library_is_tried_once_per_search_not_once_per_result( + providers: list[_Provider], monkeypatch: pytest.MonkeyPatch, clock: list[float] +) -> None: + provider = _Provider("ebook", {"ebook"}, [_DCC_ENTRY]) + providers.append(provider) + assert library_index.is_in_library(_book(), "ebook") is True + + provider.error = RuntimeError("boom") + clock[0] += library_index._CACHE_TTL_SECONDS + 1 + warnings = _warnings(monkeypatch) + + for _ in range(40): # one search page, every result asks + assert library_index.is_in_library(_book(), "ebook") is True + + assert provider.fetches == 2 # the first index, then one failed attempt + assert len(warnings) == 1 + + +def test_a_failing_library_with_no_cache_is_not_retried_for_every_result( + providers: list[_Provider], clock: list[float] +) -> None: + provider = _Provider("ebook", {"ebook"}, error=OSError("no such file")) + providers.append(provider) + + for _ in range(10): + assert library_index.is_in_library(_book(), "ebook") is False + + assert provider.fetches == 1 + + +def test_a_failing_library_is_retried_once_the_back_off_has_passed( + providers: list[_Provider], clock: list[float] +) -> None: + provider = _Provider("ebook", {"ebook"}, [_DCC_ENTRY], error=OSError("down")) + providers.append(provider) + assert library_index.is_in_library(_book(), "ebook") is False + + provider.error = None + clock[0] += library_index._FAILURE_BACKOFF_SECONDS - 1 + assert library_index.is_in_library(_book(), "ebook") is False + assert provider.fetches == 1 + + clock[0] += 2 + assert library_index.is_in_library(_book(), "ebook") is True + assert provider.fetches == 2 + + def test_entries_are_cached_until_the_ttl_expires( providers: list[_Provider], clock: list[float] ) -> None: