Skip to content

Fix worker-death stalls and pull orphaned files from a watch folder (fixes #34)#35

Open
bugrax wants to merge 8 commits into
wouterdebie:mainfrom
bugrax:scan-download-folder-for-orphans
Open

Fix worker-death stalls and pull orphaned files from a watch folder (fixes #34)#35
bugrax wants to merge 8 commits into
wouterdebie:mainfrom
bugrax:scan-download-folder-for-orphans

Conversation

@bugrax

@bugrax bugrax commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #34. Two related parts.

Part 1 — workers no longer die (and silently stall the whole process)

work() in both the orchestration and download loops ended on the first ? that errored, and the spawned task's Result is dropped, so a dying worker logged nothing. The death also cascaded: one failing get_download_targets (e.g. a put.io error, common when a transfer is removed mid-flight) ends its orchestration worker → its done channels drop → download workers feeding it die on send() → orchestration workers waiting on them die on recv() → … until every worker is gone and the process sits idle (~0.1% CPU, ~7 fds, no error logged). Files would be fully downloaded on disk yet download done never fired (the download worker finished, then died at the reporting step). A restart "fixed" it until the next error.

  • Orchestration: the QueuedForDownload body moved into handle_queued(); the worker loop logs its error and continues instead of propagating.
  • Download: a failed done-status send() is logged and ignored rather than ?-propagated.

This (together with the request timeouts in #32/#33, which turn a hung put.io call into an error this change then survives) is what keeps the pipeline up under load. Verified live: a previously-stalling backlog now runs to the NAS write ceiling and stays there.

Part 2 — pull orphaned files from a watch folder

putioarr only discovers work from transfers/list. If a transfer is removed before it's pulled — most commonly with put.io's "clear completed transfers" enabled, which deletes the transfer record while leaving the file in the folder — the file can never be pulled and is stranded (I had ~500 GB sitting in the download folder with no transfers).

New optional watch_folders config (list of put.io folder ids). Each poll, any video file in those folders with no active transfer and not already imported is pulled like a normal download:

  • routed to the Sonarr/Radarr category folder based on the name (SxxExx ⇒ tv else movie),
  • reported to the *arr via torrent-get (a synthetic transfer) so it imports normally,
  • and, once imported, deleted from put.io directly in the import watcher (it has no transfer to remove/seed). If a prior run already imported it, the next scan deletes it.

Empty by default, so it's a no-op unless configured.

Testing

Live, against a real Sonarr/Radarr setup with ~56 orphaned files (~500 GB): they were scanned, queued, pulled with correct tv/movie routing, imported by the *arr (no "No files found eligible"), the local copies removed, and the files deleted from put.io. End-to-end at the NAS write ceiling, no stalls.

bugrax added 3 commits June 10, 2026 12:03
…outerdebie#34)

The Imported-message route to cleanup ran on an orchestration worker,
which can be permanently busy in handle_queued while downloads are
active, so orphan put.io deletes never fired. Do the delete in the
watch_for_import task itself (which already deletes the local copy).
Copilot AI review requested due to automatic review settings June 10, 2026 10:07

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 support for scanning configurable put.io “watch folders” to discover and process orphaned completed files (files that exist on put.io without a corresponding transfer record), and hardens worker orchestration so individual task failures don’t stall the system.

Changes:

  • Introduces OrphanFile tracking in state and reports these orphans through the Transmission-compatible torrent-get endpoint so *arr apps can import them normally.
  • Adds watch_folders config and a periodic scan that queues synthetic transfers for orphaned files.
  • Refactors download orchestration and download workers to avoid cascading worker shutdowns on channel/task failures.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/state.rs Adds in-memory orphan tracking (OrphanFile, orphans map) to support reporting/import lifecycle.
src/services/putio.rs Extends file listing response with size (defaulted) to support torrent-get reporting.
src/main.rs Adds watch_folders config to enable orphan scanning.
src/http/handlers.rs Extends torrent-get results to include orphan downloads for *arr import.
src/download_system/transfer.rs Adds orphan transfer construction + watch-folder scanning and queuing logic.
src/download_system/orchestration.rs Refactors queued-download handling to avoid killing workers on errors; adds orphan-specific import cleanup path.
src/download_system/download.rs Prevents download worker from dying when status reporting channel is gone.

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

Comment thread src/http/handlers.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/orchestration.rs Outdated
Comment thread src/download_system/orchestration.rs
- handle_queued: skip transfers with no targets instead of vacuously
  marking them complete
- scan: O(1) HashSet membership for active_file_ids/orphan_seen
- synthetic hash from u64 so it stays a clean 40-hex value
- log the folder id (not the file id) on a state-save failure
- only add_orphan after the queue send succeeds
- torrent-get: keep orphan total_size/left_until_done consistent
@bugrax

bugrax commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — addressed the substantive points (commit 8b2fab5):

  • Vacuous-truth on empty targets: handle_queued now skips a transfer with no targets (warn + return) instead of letting .all(...) pass and marking it complete.
  • O(n) lookups: active_file_ids and orphan_seen are now HashSet<i64> for O(1) membership.
  • Synthetic hash: formatted from file_id as u64 so it stays a clean 40-hex value even for unusual ids.
  • Misleading log: the state-save failure now logs the folder_id (and the file id), not the file id alone.
  • Orphan reported before queue: add_orphan(...) now runs only after the queue send succeeds, so a failed send can't advertise a phantom download via torrent-get.
  • Inconsistent size/left: clamped so left_until_done <= total_size (size can be 0 if put.io omits it).

On the synthetic torrent id colliding with a real transfer id: the *arr matches on hashString (which is unique here), and these ids share put.io's id space in practice, so I kept file_id as u64; happy to add an offset if you'd prefer.

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

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 7 out of 7 changed files in this pull request and generated 9 comments.

Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/http/handlers.rs Outdated
Comment thread src/http/handlers.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs Outdated
- Track in-progress orphans via state.has_orphan instead of an
  ever-growing orphan_seen set; a failed orphan is dropped from it
  (orchestration) so a later scan retries it
- Route orphans with an explicit base dir (get_download_targets_in)
  instead of persisting per-orphan TransferState that never got cleaned
- Skip non-media / negative-id watch-folder entries up front
- torrent-get: checked u64::try_from for the orphan id
- looks_like_episode: case-insensitive 'season' check without allocating
@bugrax bugrax requested a review from Copilot June 10, 2026 10:38

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 7 out of 7 changed files in this pull request and generated 5 comments.

Comment thread src/http/handlers.rs
Comment thread src/download_system/orchestration.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs
- handle_queued reuses precomputed t.targets when present so watch-folder
  orphans download to their intended category dir (get_download_targets_in)
  instead of being mis-routed by a stateless get_download_targets()
- torrent-get uses the already-validated u64 id for the struct field too
- throttle watch-folder scans to a 60s interval instead of every poll
- log (not swallow) delete_file failures in the already-imported path
- debug_assert the non-negative file_id invariant in from_orphan
@bugrax bugrax requested a review from Copilot June 10, 2026 10:54

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 7 out of 7 changed files in this pull request and generated 5 comments.

Comment thread src/http/handlers.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs
…erval

- torrent-get: report a non-zero remaining amount for an incomplete orphan
  even when put.io omitted its size, so 0/0 isn't read as complete
- get_download_targets[_in]: return an error instead of panicking when a
  transfer has no file_id (shared generate_targets helper)
- make the watch-folder scan interval configurable via
  watch_folder_interval_secs (default 60), documented in the template
@bugrax bugrax requested a review from Copilot June 10, 2026 11:09

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 8 out of 8 changed files in this pull request and generated 6 comments.

Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs Outdated
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/orchestration.rs
- add request timeouts to putio::list_files and putio::url so a hung
  connection during a watch-folder scan can't stall transfer production
- clamp watch_folder_interval_secs to >= 1s so a misconfigured 0 doesn't
  become a zero Duration that scans every poll
- derive OrphanFile.hash from the transfer's hash instead of formatting
  file_id a second time (single source of truth)
- clone only the Sender, not the unused Receiver, per done channel
@bugrax bugrax requested a review from Copilot June 10, 2026 11:23

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 8 out of 8 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/services/putio.rs:1

  • size is modeled as i64, but file sizes are naturally non-negative and are typically represented as u64 to avoid sentinel negatives and overflow edge cases. Since downstream code now has to clamp negatives (max(0)), consider switching size (and corresponding OrphanFile.size) to u64 and performing a checked conversion when parsing API responses if needed.
use anyhow::{bail, Result};

Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/transfer.rs
Comment thread src/download_system/orchestration.rs
Comment thread src/download_system/transfer.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.

Orchestration deadlocks/parks with a large backlog: workers go idle after 'generating targets', nothing downloads

2 participants