From 8cf50f73c23efe26a960751e7245128c34826a5d Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Fri, 2 Oct 2026 19:26:30 -0600 Subject: [PATCH 1/5] feat(download): make the torrent stall window a setting #1420 gave a torrent 15 minutes to start and 15 minutes after each step forward. That suits a slow swarm, but someone else will want less or more. TORRENT_STALL_TIMEOUT_MINUTES (Download Clients, 5 to 60) sets it. The default is 15, so nothing changes. Five is the orchestrator's own stall timeout, so choosing it asks for nothing extra. The orchestrator's cap on a single activity grace goes from 16 minutes to an hour to leave room for the maximum. --- docs/environment-variables.md | 11 ++ shelfmark/download/clients/base_handler.py | 38 ++++-- shelfmark/download/clients/settings.py | 22 ++++ shelfmark/download/orchestrator.py | 3 +- tests/download/test_torrent_queue_stall.py | 128 ++++++++++++++++++--- 5 files changed, 180 insertions(+), 22 deletions(-) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 6ef1ea554..22478b7ed 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -1734,6 +1734,7 @@ How long to keep cached search results before they expire. | `RTORRENT_DOWNLOAD_DIR` | Server-side directory where torrents are downloaded (optional, uses rTorrent default if not specified) | string | _none_ | | `PROWLARR_TORRENT_ACTION` | After a successful import, Remove keeps downloaded files in qBittorrent, Transmission, and Deluge; Remove & Delete Files also deletes them. rTorrent cannot delete download data, so this action leaves its torrent untouched. Blackhole does not support removal. Debrid clients delete temporary local files for either Remove action. | string (choice) | `keep` | | `PROWLARR_TORRENT_POST_IMPORT_CATEGORY` | Category or label to assign after a successful import | string | _empty string_ | +| `TORRENT_STALL_TIMEOUT_MINUTES` | How long a torrent can go without any progress before Shelfmark cancels it. Applies from when the torrent starts and restarts each time it moves. Raise it for slow swarms or magnets that take a while to find peers, or lower it to give up on dead torrents sooner. | number | `15` | | `PROWLARR_USENET_CLIENT` | Choose which usenet client to use | string (choice) | _empty string_ | | `NZBGET_URL` | URL of your NZBGet instance | string | _none_ | | `NZBGET_USERNAME` | NZBGet control username | string | `nzbget` | @@ -2057,6 +2058,16 @@ Category or label to assign after a successful import - **Type:** string - **Default:** _empty string_ +#### `TORRENT_STALL_TIMEOUT_MINUTES` + +**Stall Timeout (minutes)** + +How long a torrent can go without any progress before Shelfmark cancels it. Applies from when the torrent starts and restarts each time it moves. Raise it for slow swarms or magnets that take a while to find peers, or lower it to give up on dead torrents sooner. + +- **Type:** number +- **Default:** `15` +- **Constraints:** min: 5, max: 60 + #### `PROWLARR_USENET_CLIENT` **Usenet Client** diff --git a/shelfmark/download/clients/base_handler.py b/shelfmark/download/clients/base_handler.py index f24b49bbe..e2e67e061 100644 --- a/shelfmark/download/clients/base_handler.py +++ b/shelfmark/download/clients/base_handler.py @@ -23,6 +23,12 @@ get_client, list_configured_clients, ) +from shelfmark.download.clients.settings import ( + STALL_TIMEOUT_DEFAULT_MINUTES, + STALL_TIMEOUT_MAX_MINUTES, + STALL_TIMEOUT_MIN_MINUTES, + STALL_TIMEOUT_SETTING, +) from shelfmark.download.fs import run_blocking_io from shelfmark.download.permissions_debug import log_path_permission_context from shelfmark.release_sources import DownloadHandler, HandoffResult @@ -53,18 +59,18 @@ def _is_sabnzbd_like_client(candidate: DownloadClient) -> TypeGuard[_SabnzbdLike POLL_INTERVAL = 2 # A torrent the client has queued is waiting for a free slot, not stalled. Its status never # changes while it waits, so the orchestrator's stall timer would cancel it after -# STALL_TIMEOUT. Ask for a grace instead (the orchestrator caps one grace at 16 minutes), +# STALL_TIMEOUT. Ask for a grace instead (the orchestrator caps one grace at an hour), # renew it while the torrent is still queued, and stop at a hard ceiling so a queue that # never moves still ends. QUEUE_GRACE_SECONDS = 900.0 QUEUE_GRACE_RENEW_SECONDS = 600.0 QUEUE_MAX_WAIT_SECONDS = 7200.0 # Torrents are jerky: a swarm with one seed can sit still for several minutes and then carry -# on. The orchestrator's default window (STALL_TIMEOUT) is five minutes of no change. A torrent -# that has already moved gets this longer window, restarted every time its progress goes up, -# so a dead one still ends 15 minutes after its last movement. One that never moved (a magnet -# that cannot fetch metadata) keeps the short window and is cleaned up quickly. -MOVING_STALL_SECONDS = 900.0 +# on, and a magnet can take a while to fetch metadata or find a first peer. The orchestrator +# ends a download after STALL_TIMEOUT (five minutes) without a change. This setting is the +# time a torrent may sit without progress before it is cancelled, given once when the +# torrent starts and restarted every time its progress goes up. The default is 15 minutes; +# five is the shortest, which is the orchestrator's own timeout and so adds nothing. WINDOWS_DRIVE_PREFIX_LENGTH = 2 SECONDS_PER_MINUTE = 60 SECONDS_PER_HOUR = 3600 @@ -217,6 +223,19 @@ def _list_configured_clients(self) -> list[str]: """List protocols with configured clients.""" return list_configured_clients() + @staticmethod + def _stall_window_seconds() -> float: + """How long a torrent may go without progress, from the user's setting (seconds).""" + raw = config.get(STALL_TIMEOUT_SETTING, STALL_TIMEOUT_DEFAULT_MINUTES) + try: + minutes = float(raw) # pyright: ignore[reportArgumentType] + except TypeError, ValueError: + minutes = float(STALL_TIMEOUT_DEFAULT_MINUTES) + if not math.isfinite(minutes): + minutes = float(STALL_TIMEOUT_DEFAULT_MINUTES) + minutes = min(max(minutes, STALL_TIMEOUT_MIN_MINUTES), STALL_TIMEOUT_MAX_MINUTES) + return minutes * SECONDS_PER_MINUTE + def _poll_interval(self) -> float: """Return the polling interval for status checks.""" return POLL_INTERVAL @@ -989,6 +1008,7 @@ def _poll_and_complete( grace_requested_at = 0.0 last_progress = 0.0 start_window_given = False + stall_window = self._stall_window_seconds() # Track consecutive "not found" errors - torrents may take time to appear in client not_found_count = 0 max_not_found_retries = 15 # 15 retries * poll interval ~= 30s grace period @@ -1090,12 +1110,14 @@ def _poll_and_complete( grace_requested_at = 0.0 if status.progress > last_progress: - request_activity_grace(status_callback, MOVING_STALL_SECONDS) + if stall_window > STALL_TIMEOUT_MIN_MINUTES * SECONDS_PER_MINUTE: + request_activity_grace(status_callback, stall_window) start_window_given = True elif not start_window_given and status.state != DownloadState.QUEUED: # Metadata and the first peer can take longer than the orchestrator's # stall timeout, so a torrent gets one full window to get going. - request_activity_grace(status_callback, MOVING_STALL_SECONDS) + if stall_window > STALL_TIMEOUT_MIN_MINUTES * SECONDS_PER_MINUTE: + request_activity_grace(status_callback, stall_window) start_window_given = True last_progress = status.progress diff --git a/shelfmark/download/clients/settings.py b/shelfmark/download/clients/settings.py index 07dc6e276..af5bae369 100644 --- a/shelfmark/download/clients/settings.py +++ b/shelfmark/download/clients/settings.py @@ -9,6 +9,7 @@ from shelfmark.core.settings_registry import ( ActionButton, HeadingField, + NumberField, PasswordField, SelectField, SettingsField, @@ -604,6 +605,13 @@ def _test_torbox_connection(current_values: dict[str, Any] | None = None) -> dic # ==================== Download Clients Tab ==================== +# Torrent stall window (see ExternalClientHandler._stall_window_seconds). +STALL_TIMEOUT_SETTING = "TORRENT_STALL_TIMEOUT_MINUTES" +STALL_TIMEOUT_DEFAULT_MINUTES = 15 +STALL_TIMEOUT_MIN_MINUTES = 5 # the orchestrator's STALL_TIMEOUT; a shorter window cannot apply +STALL_TIMEOUT_MAX_MINUTES = 60 + + @register_settings( name="prowlarr_clients", display_name="Download Clients", @@ -948,6 +956,20 @@ def prowlarr_clients_settings() -> list[SettingsField]: default="", show_when={"field": "PROWLARR_TORRENT_ACTION", "value": "change_category"}, ), + NumberField( + key=STALL_TIMEOUT_SETTING, + label="Stall Timeout (minutes)", + description=( + "How long a torrent can go without any progress before Shelfmark cancels it. " + "Applies from when the torrent starts and restarts each time it moves. " + "Raise it for slow swarms or magnets that take a while to find peers, or lower it " + "to give up on dead torrents sooner." + ), + default=STALL_TIMEOUT_DEFAULT_MINUTES, + min_value=STALL_TIMEOUT_MIN_MINUTES, + max_value=STALL_TIMEOUT_MAX_MINUTES, + show_when={"field": "PROWLARR_TORRENT_CLIENT", "notEmpty": True}, + ), # --- Usenet Client Selection --- HeadingField( key="usenet_heading", diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index 5f5b8ad26..e5cc7d6ce 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -76,7 +76,8 @@ _activity_grace: dict[str, float] = {} # A caller cannot buy immortality: the largest grace any operation may request. Must stay # above the largest budget any caller can declare (see http._bypass_grace_seconds). -_MAX_ACTIVITY_GRACE_SECONDS = 960.0 +# The torrent stall setting (base_handler.STALL_TIMEOUT_MAX_MINUTES) can ask for up to an hour. +_MAX_ACTIVITY_GRACE_SECONDS = 3600.0 COORDINATOR_LOOP_ERROR_RETRY_DELAY = 1.0 # Ceiling for the exponential backoff applied to repeated coordinator loop failures. _COORDINATOR_LOOP_ERROR_MAX_DELAY = 30.0 diff --git a/tests/download/test_torrent_queue_stall.py b/tests/download/test_torrent_queue_stall.py index ae0258eb6..19080a5bf 100644 --- a/tests/download/test_torrent_queue_stall.py +++ b/tests/download/test_torrent_queue_stall.py @@ -6,6 +6,8 @@ from threading import Event from unittest.mock import MagicMock, patch +import pytest + from shelfmark.core.models import DownloadTask from shelfmark.download.activity import ACTIVITY_GRACE_STATUS from shelfmark.download.clients import DownloadState, DownloadStatus @@ -36,7 +38,15 @@ def graces(self) -> list[float]: return [float(m) for s, m in self.events if s == ACTIVITY_GRACE_STATUS] -def _poll(statuses: list[DownloadStatus], clock: list[float] | None = None) -> _Recorder: +WINDOW = 15 * 60.0 # what `_poll` configures unless a test asks for something else + + +def _poll( + statuses: list[DownloadStatus], + clock: list[float] | None = None, + *, + minutes: object = 15, +) -> _Recorder: """Run the poll loop over a scripted list of client statuses, then cancel.""" cancel = Event() seen = iter(statuses) @@ -53,7 +63,16 @@ def get_status(_id: str) -> DownloadStatus: client.get_status.side_effect = get_status rec = _Recorder() ticks = iter(clock) if clock is not None else None - patches = [patch.object(ProwlarrHandler, "_poll_interval", return_value=0.0)] + patches = [ + patch.object(ProwlarrHandler, "_poll_interval", return_value=0.0), + patch.object( + bh.config, + "get", + side_effect=lambda key, default=None, **_kw: ( + minutes if key == bh.STALL_TIMEOUT_SETTING and minutes is not None else default + ), + ), + ] if ticks is not None: patches.append(patch.object(bh.time, "monotonic", side_effect=lambda: next(ticks))) for p in patches: @@ -90,15 +109,15 @@ def test_the_grace_is_released_when_the_torrent_starts() -> None: assert rec.graces == [ bh.QUEUE_GRACE_SECONDS, 0.0, - bh.MOVING_STALL_SECONDS, - bh.MOVING_STALL_SECONDS, + WINDOW, + WINDOW, ] def test_a_torrent_that_never_queued_only_gets_the_moving_window() -> None: rec = _poll([_status(D, 1.0), _status(D, 2.0)]) - assert rec.graces == [bh.MOVING_STALL_SECONDS] * 2 # no release, no queue renewal + assert rec.graces == [WINDOW] * 2 # no release, no queue renewal # --- a torrent that is moving gets a longer window each time it moves --------------------- @@ -107,31 +126,31 @@ def test_a_torrent_that_never_queued_only_gets_the_moving_window() -> None: def test_every_step_forward_pushes_the_stall_deadline_out() -> None: rec = _poll([_status(D, 1.0), _status(D, 2.0), _status(D, 3.5)]) - assert rec.graces == [bh.MOVING_STALL_SECONDS] * 3 + assert rec.graces == [WINDOW] * 3 def test_a_torrent_that_stops_moving_stops_getting_more_time() -> None: rec = _poll([_status(D, 5.0), _status(D, 5.0), _status(D, 5.0), _status(D, 5.0)]) - assert rec.graces == [bh.MOVING_STALL_SECONDS] # only the first reading moved + assert rec.graces == [WINDOW] # only the first reading moved def test_a_torrent_gets_a_full_window_to_start_even_before_any_data_arrives() -> None: # Metadata and the first peer can take longer than the orchestrator's 5 minutes. - assert _poll([_status(D, 0.0)] * 4).graces == [bh.MOVING_STALL_SECONDS] + assert _poll([_status(D, 0.0)] * 4).graces == [WINDOW] def test_the_start_window_is_given_once_not_renewed_while_stuck_at_zero() -> None: graces = _poll([_status(D, 0.0)] * 20).graces - assert graces.count(bh.MOVING_STALL_SECONDS) == 1 + assert graces.count(WINDOW) == 1 def test_a_torrent_leaving_the_queue_at_zero_gets_a_fresh_start_window() -> None: rec = _poll([_status(Q), _status(Q), _status(D, 0.0), _status(D, 0.0)]) # queue grace, its release, then the start window - assert rec.graces == [bh.QUEUE_GRACE_SECONDS, 0.0, bh.MOVING_STALL_SECONDS] + assert rec.graces == [bh.QUEUE_GRACE_SECONDS, 0.0, WINDOW] def test_a_queued_torrent_is_not_given_the_start_window_yet() -> None: @@ -141,7 +160,7 @@ def test_a_queued_torrent_is_not_given_the_start_window_yet() -> None: def test_going_backwards_is_not_movement() -> None: rec = _poll([_status(D, 50.0), _status(D, 40.0), _status(D, 40.0)]) - assert rec.graces == [bh.MOVING_STALL_SECONDS] + assert rec.graces == [WINDOW] def test_the_windows_fit_inside_what_the_orchestrator_allows() -> None: @@ -149,8 +168,91 @@ def test_the_windows_fit_inside_what_the_orchestrator_allows() -> None: cap = orchestrator._MAX_ACTIVITY_GRACE_SECONDS assert bh.QUEUE_GRACE_SECONDS <= cap - assert bh.MOVING_STALL_SECONDS <= cap - assert bh.MOVING_STALL_SECONDS > orchestrator.STALL_TIMEOUT + assert bh.STALL_TIMEOUT_MAX_MINUTES * 60 <= cap + assert bh.STALL_TIMEOUT_MIN_MINUTES * 60 == orchestrator.STALL_TIMEOUT + + +# --- the window is the user's to set; the default leaves behaviour exactly as it was --------- + + +def test_by_default_a_moving_torrent_gets_the_fifteen_minute_window() -> None: + rec = _poll([_status(D, 1.0), _status(D, 2.0), _status(D, 2.0)], minutes=None) + + assert rec.graces == [WINDOW, WINDOW] + + +def test_by_default_a_new_torrent_gets_the_fifteen_minute_start_window() -> None: + assert _poll([_status(D, 0.0)] * 4, minutes=None).graces == [WINDOW] + + +def test_the_default_is_the_fifteen_minutes_that_shipped() -> None: + assert bh.STALL_TIMEOUT_DEFAULT_MINUTES == 15 + + +def test_a_queued_torrent_still_gets_its_grace_by_default() -> None: + assert _poll([_status(Q)] * 3, minutes=None).graces == [bh.QUEUE_GRACE_SECONDS] + + +def test_a_longer_setting_sets_the_window() -> None: + rec = _poll([_status(D, 1.0), _status(D, 2.0)], minutes=30) + + assert rec.graces == [1800.0, 1800.0] + + +def test_the_start_window_follows_the_setting_too() -> None: + assert _poll([_status(D, 0.0)] * 3, minutes=30).graces == [1800.0] + + +def test_five_minutes_is_the_orchestrators_own_timeout_so_it_adds_nothing() -> None: + assert _poll([_status(D, 1.0), _status(D, 2.0)], minutes=5).graces == [] + + +def test_a_value_above_the_maximum_is_capped() -> None: + rec = _poll([_status(D, 1.0)], minutes=500) + + assert rec.graces == [bh.STALL_TIMEOUT_MAX_MINUTES * 60.0] + + +@pytest.mark.parametrize( + ("configured", "seconds"), + [ + (None, 900.0), # not set + (5, 300.0), + (15, 900.0), + ("30", 1800.0), # an env var arrives as text + (7.5, 450.0), + (1, 300.0), # below the minimum + (0, 300.0), + (-10, 300.0), + (500, 3600.0), # above the maximum + ("soon", 900.0), # not a number: the default + ("", 900.0), + (float("nan"), 900.0), + (float("inf"), 900.0), + ], +) +def test_the_configured_window_is_read_and_kept_in_range( + configured: object, seconds: float +) -> None: + with patch.object(bh.config, "get", return_value=configured): + # `get` returns the configured value; None stands in for "unset" via the default + value = ProwlarrHandler._stall_window_seconds() + + assert value == seconds + + +def test_the_setting_is_offered_in_the_download_clients_tab() -> None: + from shelfmark.core.settings_registry import get_settings_field_map + from shelfmark.download.clients import settings as client_settings # noqa: F401 + + field, tab_name = get_settings_field_map("prowlarr_clients")[bh.STALL_TIMEOUT_SETTING] + assert tab_name == "prowlarr_clients" + + assert (field.min_value, field.max_value) == ( + bh.STALL_TIMEOUT_MIN_MINUTES, + bh.STALL_TIMEOUT_MAX_MINUTES, + ) + assert field.default == bh.STALL_TIMEOUT_DEFAULT_MINUTES def test_a_long_queue_renews_the_grace_but_only_until_the_ceiling() -> None: From 63b961b99b41e43a5c8f10fa96866984f6706071 Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Sun, 4 Oct 2026 03:04:18 -0600 Subject: [PATCH 2/5] fix(download): send the first queue grace even when the clock is low grace_requested_at started at 0.0 and was compared against time.monotonic(). On a host that booted less than 10 minutes ago the first queued poll saw now - 0.0 < 600 and sent no grace. Start from None instead, and make the queue grace test run on a small clock so it no longer depends on uptime. --- shelfmark/download/clients/base_handler.py | 12 +++++++----- tests/download/test_torrent_queue_stall.py | 10 +++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/shelfmark/download/clients/base_handler.py b/shelfmark/download/clients/base_handler.py index e2e67e061..f0e01072f 100644 --- a/shelfmark/download/clients/base_handler.py +++ b/shelfmark/download/clients/base_handler.py @@ -1005,7 +1005,9 @@ def _poll_and_complete( """ poll_interval = self._poll_interval() queued_since: float | None = None - grace_requested_at = 0.0 + # None, not 0.0: time.monotonic() can be below the renew interval on a host that + # booted recently, which would skip the first request. + grace_requested_at: float | None = None last_progress = 0.0 start_window_given = False stall_window = self._stall_window_seconds() @@ -1098,16 +1100,16 @@ def _poll_and_complete( now = time.monotonic() if queued_since is None: queued_since = now - if ( - now - queued_since < QUEUE_MAX_WAIT_SECONDS - and now - grace_requested_at >= QUEUE_GRACE_RENEW_SECONDS + if now - queued_since < QUEUE_MAX_WAIT_SECONDS and ( + grace_requested_at is None + or now - grace_requested_at >= QUEUE_GRACE_RENEW_SECONDS ): request_activity_grace(status_callback, QUEUE_GRACE_SECONDS) grace_requested_at = now elif queued_since is not None: release_activity_grace(status_callback) queued_since = None - grace_requested_at = 0.0 + grace_requested_at = None if status.progress > last_progress: if stall_window > STALL_TIMEOUT_MIN_MINUTES * SECONDS_PER_MINUTE: diff --git a/tests/download/test_torrent_queue_stall.py b/tests/download/test_torrent_queue_stall.py index 19080a5bf..f72fdd3ee 100644 --- a/tests/download/test_torrent_queue_stall.py +++ b/tests/download/test_torrent_queue_stall.py @@ -96,7 +96,8 @@ def get_status(_id: str) -> DownloadStatus: def test_a_queued_torrent_asks_for_a_grace_once() -> None: - rec = _poll([_status(Q)] * 4) + # A clock that starts below the renew interval, like a host that booted a minute ago. + rec = _poll([_status(Q)] * 4, clock=[5.0, 5.0, 6.0, 7.0]) assert rec.graces[0] == bh.QUEUE_GRACE_SECONDS assert rec.graces.count(bh.QUEUE_GRACE_SECONDS) == 1 @@ -269,3 +270,10 @@ def test_a_queue_wait_past_the_ceiling_gets_no_more_grace() -> None: rec = _poll([_status(Q)] * 3, clock=[5000 + t for t in times]) assert rec.graces.count(bh.QUEUE_GRACE_SECONDS) == 1 + + +def test_the_first_queue_grace_is_sent_on_a_freshly_booted_host() -> None: + assert bh.QUEUE_GRACE_RENEW_SECONDS > 5.0 + rec = _poll([_status(Q)] * 2, clock=[1.0, 2.0]) + + assert rec.graces == [bh.QUEUE_GRACE_SECONDS] From 6f562807e729fc6178cb460a1045fef03dc74977 Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Sun, 4 Oct 2026 03:04:36 -0600 Subject: [PATCH 3/5] fix(download): give a torrent its stall window again after it leaves the queue Leaving the client's queue releases the grace, which puts the stall deadline back to the orchestrator's 5 minutes. A torrent that had already used its start window, or was moving before the client requeued it, got no new window until its progress next went up. Reset the start window on release so the configured window applies again. --- shelfmark/download/clients/base_handler.py | 3 +++ tests/download/test_torrent_queue_stall.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/shelfmark/download/clients/base_handler.py b/shelfmark/download/clients/base_handler.py index f0e01072f..d1fac5bec 100644 --- a/shelfmark/download/clients/base_handler.py +++ b/shelfmark/download/clients/base_handler.py @@ -1110,6 +1110,9 @@ def _poll_and_complete( release_activity_grace(status_callback) queued_since = None grace_requested_at = None + # The release drops the stall deadline back to the orchestrator's own + # timeout, so the torrent needs a fresh window even if it had one before. + start_window_given = False if status.progress > last_progress: if stall_window > STALL_TIMEOUT_MIN_MINUTES * SECONDS_PER_MINUTE: diff --git a/tests/download/test_torrent_queue_stall.py b/tests/download/test_torrent_queue_stall.py index f72fdd3ee..c0db6fa08 100644 --- a/tests/download/test_torrent_queue_stall.py +++ b/tests/download/test_torrent_queue_stall.py @@ -277,3 +277,22 @@ def test_the_first_queue_grace_is_sent_on_a_freshly_booted_host() -> None: rec = _poll([_status(Q)] * 2, clock=[1.0, 2.0]) assert rec.graces == [bh.QUEUE_GRACE_SECONDS] + + +def test_a_torrent_requeued_by_the_client_gets_its_window_back_when_it_leaves() -> None: + rec = _poll([_status(D, 0.0), _status(Q), _status(Q), _status(D, 0.0), _status(D, 0.0)]) + + # start window, queue grace, release, then a new window instead of the bare 5 minutes + assert rec.graces == [WINDOW, bh.QUEUE_GRACE_SECONDS, 0.0, WINDOW] + + +def test_a_moving_torrent_that_gets_requeued_is_not_left_on_the_short_timeout() -> None: + rec = _poll([_status(D, 5.0), _status(Q, 5.0), _status(D, 5.0), _status(D, 5.0)]) + + assert rec.graces == [WINDOW, bh.QUEUE_GRACE_SECONDS, 0.0, WINDOW] + + +def test_a_requeued_torrent_follows_the_setting_when_it_leaves_the_queue() -> None: + rec = _poll([_status(D, 1.0), _status(Q, 1.0), _status(D, 1.0)], minutes=30) + + assert rec.graces == [1800.0, bh.QUEUE_GRACE_SECONDS, 0.0, 1800.0] From b77db09107323751de89a8c3d4145d51e1d31cca Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Sun, 4 Oct 2026 03:05:51 -0600 Subject: [PATCH 4/5] refactor(download): derive the stall setting minimum from the orchestrator timeout STALL_TIMEOUT_MIN_MINUTES was a second copy of 300 seconds. Put the number in download/activity.py, which has no imports, and have both the orchestrator and the settings module read it. --- shelfmark/download/activity.py | 5 +++++ shelfmark/download/clients/settings.py | 3 ++- shelfmark/download/orchestrator.py | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/shelfmark/download/activity.py b/shelfmark/download/activity.py index 10d9789fa..438163a44 100644 --- a/shelfmark/download/activity.py +++ b/shelfmark/download/activity.py @@ -37,6 +37,11 @@ from collections.abc import Callable +# Seconds without a changed status or progress value before the orchestrator cancels a +# download. Lives here so the settings module can derive its minimum without importing the +# orchestrator. +STALL_TIMEOUT_SECONDS = 300 + # Not a QueueStatus value, so `update_download_status` would reject it anyway; the # orchestrator's status_callback intercepts it before that point. ACTIVITY_GRACE_STATUS = "__activity_grace__" diff --git a/shelfmark/download/clients/settings.py b/shelfmark/download/clients/settings.py index af5bae369..df673fd39 100644 --- a/shelfmark/download/clients/settings.py +++ b/shelfmark/download/clients/settings.py @@ -18,6 +18,7 @@ register_settings, ) from shelfmark.core.utils import get_hardened_xmlrpc_client, normalize_http_url +from shelfmark.download.activity import STALL_TIMEOUT_SECONDS from shelfmark.download.network import get_ssl_verify try: @@ -608,7 +609,7 @@ def _test_torbox_connection(current_values: dict[str, Any] | None = None) -> dic # Torrent stall window (see ExternalClientHandler._stall_window_seconds). STALL_TIMEOUT_SETTING = "TORRENT_STALL_TIMEOUT_MINUTES" STALL_TIMEOUT_DEFAULT_MINUTES = 15 -STALL_TIMEOUT_MIN_MINUTES = 5 # the orchestrator's STALL_TIMEOUT; a shorter window cannot apply +STALL_TIMEOUT_MIN_MINUTES = STALL_TIMEOUT_SECONDS // 60 # a shorter window cannot apply STALL_TIMEOUT_MAX_MINUTES = 60 diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index e5cc7d6ce..31f519bab 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -24,7 +24,7 @@ ) from shelfmark.core.utils import is_audiobook as check_audiobook from shelfmark.core.utils import transform_cover_url -from shelfmark.download.activity import parse_activity_grace +from shelfmark.download.activity import STALL_TIMEOUT_SECONDS, parse_activity_grace from shelfmark.download.fs import run_blocking_io from shelfmark.download.postprocess.pipeline import is_torrent_source, safe_cleanup_path from shelfmark.download.postprocess.router import post_process_download @@ -69,7 +69,7 @@ _last_progress_value: dict[str, float] = {} # De-duplicate status updates (keep-alive updates shouldn't spam clients) _last_status_event: dict[str, tuple[str, str | None]] = {} -STALL_TIMEOUT = 300 # 5 minutes without progress/status update = stalled +STALL_TIMEOUT = STALL_TIMEOUT_SECONDS # 5 minutes without progress/status update = stalled # Absolute deadlines (time.time()) until which stall detection is suppressed for a task. # Long single-shot operations (protection bypass, etc.) declare their own upper bound via # `shelfmark.download.activity` instead of faking progress. See set_activity_grace(). From a8a2966ff8137cdc04514da998be66939cb240e3 Mon Sep 17 00:00:00 2001 From: Rob Martin Date: Sun, 4 Oct 2026 03:06:12 -0600 Subject: [PATCH 5/5] fix(download): keep the torrent stall window to torrents and report the real one The stall setting is documented for torrents, but _poll_and_complete asked for the window on every protocol, so usenet downloads got it too. Only torrents ask for it now. The stall message always said 300s. Remember the size of the last grace a task asked for and report that, falling back to the default once it is released. --- shelfmark/download/clients/base_handler.py | 8 +++++- shelfmark/download/orchestrator.py | 10 +++++++- tests/download/test_orchestrator_stall.py | 30 ++++++++++++++++++++++ tests/download/test_torrent_queue_stall.py | 17 +++++++++++- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/shelfmark/download/clients/base_handler.py b/shelfmark/download/clients/base_handler.py index d1fac5bec..622d898b6 100644 --- a/shelfmark/download/clients/base_handler.py +++ b/shelfmark/download/clients/base_handler.py @@ -1010,7 +1010,13 @@ def _poll_and_complete( grace_requested_at: float | None = None last_progress = 0.0 start_window_given = False - stall_window = self._stall_window_seconds() + # The setting is for torrents. Usenet keeps the orchestrator's own timeout, which is + # the floor of the setting, so no window is requested for it. + stall_window = ( + self._stall_window_seconds() + if protocol == "torrent" + else float(STALL_TIMEOUT_MIN_MINUTES * SECONDS_PER_MINUTE) + ) # Track consecutive "not found" errors - torrents may take time to appear in client not_found_count = 0 max_not_found_retries = 15 # 15 retries * poll interval ~= 30s grace period diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index 31f519bab..e37e99ae8 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -74,6 +74,9 @@ # Long single-shot operations (protection bypass, etc.) declare their own upper bound via # `shelfmark.download.activity` instead of faking progress. See set_activity_grace(). _activity_grace: dict[str, float] = {} +# The size of the last grace a task asked for, so a stall message can report the window the +# task actually had instead of the default. +_activity_grace_seconds: dict[str, float] = {} # A caller cannot buy immortality: the largest grace any operation may request. Must stay # above the largest budget any caller can declare (see http._bypass_grace_seconds). # The torrent stall setting (base_handler.STALL_TIMEOUT_MAX_MINUTES) can ask for up to an hour. @@ -1015,6 +1018,7 @@ def _cleanup_progress_tracking(task_id: str) -> None: _last_progress_value.pop(task_id, None) _last_status_event.pop(task_id, None) _activity_grace.pop(task_id, None) + _activity_grace_seconds.pop(task_id, None) def _finalize_download_failure(task_id: str) -> None: @@ -1078,6 +1082,7 @@ def set_activity_grace(book_id: str, seconds: float) -> None: grace = min(max(grace, 0.0), _MAX_ACTIVITY_GRACE_SECONDS) with _progress_lock: _activity_grace[book_id] = time.time() + grace + _activity_grace_seconds[book_id] = grace def clear_activity_grace(book_id: str) -> None: @@ -1088,6 +1093,7 @@ def clear_activity_grace(book_id: str) -> None: """ with _progress_lock: _activity_grace.pop(book_id, None) + _activity_grace_seconds.pop(book_id, None) _last_activity[book_id] = time.time() @@ -1117,9 +1123,11 @@ def _cancel_stalled_task(task_id: str) -> None: logger.warning("Download stalled for %s, cancelling", task_id) # The message goes first: the terminal hook copies it into the history row, and a cancel # that records the last poll's message ("Queued") hides that the timer ended the download. + with _progress_lock: + window = max(STALL_TIMEOUT, _activity_grace_seconds.get(task_id, 0.0)) book_queue.update_status_message( task_id, - f"Download stalled (no activity for {STALL_TIMEOUT}s)", + f"Download stalled (no activity for {int(window)}s)", ) book_queue.cancel_download(task_id) diff --git a/tests/download/test_orchestrator_stall.py b/tests/download/test_orchestrator_stall.py index 8dbdd9b31..656ad38a0 100644 --- a/tests/download/test_orchestrator_stall.py +++ b/tests/download/test_orchestrator_stall.py @@ -29,6 +29,7 @@ def _reset(orchestrator) -> None: orchestrator._last_progress_value.clear() orchestrator._last_status_event.clear() orchestrator._activity_grace.clear() + orchestrator._activity_grace_seconds.clear() def test_find_stalled_tasks_flags_task_past_stall_timeout(): @@ -305,3 +306,32 @@ def test_the_stall_reason_is_set_before_the_cancel_finalises_history(monkeypatch names = [call[0] for call in mock_queue.method_calls] assert names.index("update_status_message") < names.index("cancel_download") + + +def test_the_stall_message_reports_the_window_the_task_was_given(monkeypatch): + import shelfmark.download.orchestrator as orchestrator + + _reset(orchestrator) + monkeypatch.setattr(orchestrator, "book_queue", MagicMock()) + orchestrator.set_activity_grace("book", 1800) + + orchestrator._cancel_stalled_task("book") + + orchestrator.book_queue.update_status_message.assert_called_once_with( + "book", "Download stalled (no activity for 1800s)" + ) + + +def test_a_released_grace_goes_back_to_the_default_in_the_stall_message(monkeypatch): + import shelfmark.download.orchestrator as orchestrator + + _reset(orchestrator) + monkeypatch.setattr(orchestrator, "book_queue", MagicMock()) + orchestrator.set_activity_grace("book", 1800) + orchestrator.clear_activity_grace("book") + + orchestrator._cancel_stalled_task("book") + + orchestrator.book_queue.update_status_message.assert_called_once_with( + "book", f"Download stalled (no activity for {orchestrator.STALL_TIMEOUT}s)" + ) diff --git a/tests/download/test_torrent_queue_stall.py b/tests/download/test_torrent_queue_stall.py index c0db6fa08..a7fba9a9e 100644 --- a/tests/download/test_torrent_queue_stall.py +++ b/tests/download/test_torrent_queue_stall.py @@ -46,6 +46,7 @@ def _poll( clock: list[float] | None = None, *, minutes: object = 15, + protocol: str = "torrent", ) -> _Recorder: """Run the poll loop over a scripted list of client statuses, then cancel.""" cancel = Event() @@ -81,7 +82,7 @@ def get_status(_id: str) -> DownloadStatus: ProwlarrHandler()._poll_and_complete( client, "hash", - "torrent", + protocol, DownloadTask(task_id="t", source="prowlarr", title="Book"), cancel, lambda _p: None, @@ -296,3 +297,17 @@ def test_a_requeued_torrent_follows_the_setting_when_it_leaves_the_queue() -> No rec = _poll([_status(D, 1.0), _status(Q, 1.0), _status(D, 1.0)], minutes=30) assert rec.graces == [1800.0, bh.QUEUE_GRACE_SECONDS, 0.0, 1800.0] + + +def test_usenet_does_not_get_the_torrent_stall_window() -> None: + rec = _poll([_status(D, 1.0), _status(D, 2.0), _status(D, 0.0)], minutes=30, protocol="usenet") + + assert rec.graces == [] + + +def test_the_minimum_window_is_derived_from_the_orchestrators_timeout() -> None: + from shelfmark.download import orchestrator + from shelfmark.download.activity import STALL_TIMEOUT_SECONDS + + assert orchestrator.STALL_TIMEOUT == STALL_TIMEOUT_SECONDS + assert bh.STALL_TIMEOUT_MIN_MINUTES * 60 == STALL_TIMEOUT_SECONDS