Skip to content

Bound watch_for_import so stuck transfers can't stall downloads (fixes #30)#31

Open
bugrax wants to merge 4 commits into
wouterdebie:mainfrom
bugrax:bound-watch-for-import
Open

Bound watch_for_import so stuck transfers can't stall downloads (fixes #30)#31
bugrax wants to merge 4 commits into
wouterdebie:mainfrom
bugrax:bound-watch-for-import

Conversation

@bugrax

@bugrax bugrax commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #30.

Problem

watch_for_import polls is_imported() in an unbounded loop and only exits once every video file of a transfer is imported:

loop {
    if transfer.is_imported().await { /* clean up, send Imported */ break; }
    sleep(polling_interval).await;
}

If a transfer contains a video file the *arr will never import — most commonly a sample that isn't inside a skip_directories folder — is_imported() is permanently false and the task loops forever. Each such transfer leaves a forever-running task polling the *arr/put.io APIs every polling_interval. They accumulate, and after ~10h with a large library the process stops picking up newly-completed transfers entirely (downloads stall with many COMPLETED transfers left un-pulled on put.io). A restart only clears it temporarily.

The tell-tale sign in the logs is the main file of the same transfer being reported found imported by radarr over and over every ~10s — the movie imported fine, but a sibling sample keeps the transfer from ever completing.

Fix

Give up watching after MAX_IMPORT_WAIT (2h) with a warning, so a stuck transfer ends its task instead of polling forever. Genuine imports are detected within a poll or two of the *arr importing, so this generous bound doesn't affect normal operation; it just caps how long a never-importing transfer can sit in a polling loop.

This is a targeted fix for the stall. The deeper fix is better import-mapping (the existing skip_directories TODO — e.g. ignoring sample files, or treating a transfer as imported once it has left the *arr download queue), which would let these transfers complete and be cleaned up rather than be abandoned.

Note

A given-up transfer's local and put.io copies are no longer auto-cleaned (it stops being tracked). That's a deliberate trade-off — leaking one stuck transfer is far better than stalling every download — and it is re-evaluated on the next restart.

…wouterdebie#30)

watch_for_import polled is_imported() in an unbounded loop that only
exits once every video file is imported. A transfer containing a file
the *arr never imports (typically a sample outside a skip_directories
folder) makes is_imported() permanently false, so the task loops
forever. These accumulate over time until the process stops picking up
new transfers entirely (downloads stall; a restart clears it).

Give up watching after MAX_IMPORT_WAIT (2h) with a warning, so a stuck
transfer ends its task instead of polling the *arr/put.io APIs forever.
Genuine imports complete within a poll or two, so the bound doesn't
affect normal operation. A better long-term fix is improved import
mapping (the skip_directories TODO), noted in the issue.
Copilot AI review requested due to automatic review settings June 10, 2026 07:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a bounded wait to prevent watch_for_import from polling indefinitely when is_imported() can never become true (e.g., transfers containing files the *arr won’t import).

Changes:

  • Introduce MAX_IMPORT_WAIT (2h) as an upper bound for import polling.
  • Track start time with Instant and stop watching after the timeout with a warning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/download_system/orchestration.rs Outdated
Comment thread src/download_system/orchestration.rs Outdated
Comment thread src/download_system/orchestration.rs Outdated
bugrax added 2 commits June 10, 2026 10:40
The unconditional 'removed' log fired on the give-up path too, even
though the warning there says nothing was cleaned up. Move it into the
imported branch so the log matches what happened.
The give-up timeout was a hard-coded 2h. The right value depends on
library size, IO speed and *arr load, so expose it as import_timeout_secs
(default 7200). Setting it to 0 disables the bound (watch indefinitely).
@bugrax bugrax requested a review from Copilot June 10, 2026 07:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/main.rs
Comment thread src/download_system/orchestration.rs
Use #[serde(default = ...)] returning 7200 instead of #[serde(default)],
which would default to 0 (disabling the bound) if the field is missing
from a config source without the Figment default layer.
@bugrax

bugrax commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

These were addressed in follow-up commits already on the branch:

  • removed log now only fires on the actual cleanup/import path, not on timeout (95bc322); the timeout path has its own explicit warn ("giving up watching …"), which is the terminal log for that case.
  • The 2h bound is now configurable via import_timeout_secs (9645afb), defaulting at the type level with #[serde(default = "default_import_timeout_secs")] returning 7200 (94206d0), and 0 disables it.

On giving up leaving local/remote copies behind: that's an intentional trade-off — leaking one stuck transfer is far better than the unbounded loop stalling every download — and it's re-evaluated on the next restart. The accompanying log makes it greppable. Open to surfacing it as an explicit failure event if you'd like.

@bugrax bugrax requested a review from Copilot June 10, 2026 10:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/download_system/orchestration.rs
Comment thread src/main.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

watch_for_import loops forever for transfers that never fully import (e.g. samples), eventually stalling all downloads

2 participants