fix(download): release DB connections across the gallery-dl subprocess #60

Merged

bvandeusen merged 1 commits from dev into main 2026-06-03 22:11:36 -04:00

Conversation 0 Commits 1 Files Changed 3

+104 -2

bvandeusen commented 2026-06-03 22:11:30 -04:00

Owner

Copy Link

Copy Source

Follow-up to the timeout-ladder fix (already on main). Backfill events were still stranding empty — but the worker logs showed the timeout salvage working (Download timeout for anduo/patreon after 1170.0s (18 files written)), so this was a second, downstream bug.

Root cause

DownloadService held the async + sync DB connections checked out across the entire (≤19.5-min backfill) gallery-dl subprocess. The server reaped the idle connection, so phase 3's first query died with asyncpg ConnectionDoesNotExistError. That DBAPIError tripped download_source's autoretry_for; the retry re-entered _phase1_setup, hit the in-flight guard (event still running), returned in_flight and no-opped in ~0.15s — leaving the event to strand empty for the recovery sweep. pool_pre_ping was already on but can't validate a held connection (only pool checkouts).

Changes

1. DownloadService.download_source closes the async + sync sessions after phase 1, before the subprocess, so phase 3 re-acquires a live connection (makes the class's "Phase 2 — no DB connection" docstring true).
2. The per-task async engine switches to NullPool so the phase-3 reconnect always opens a fresh socket, not a reaped pooled one.

Tests

test_releases_db_connections_before_subprocess (release happens before gdl.download, event still finalizes) + test_async_task_engine_uses_nullpool. All existing test_download_service.py tests still pass. CI green on dev HEAD 576e16d.

🤖 Generated with Claude Code

Follow-up to the timeout-ladder fix (already on main). Backfill events were *still* stranding empty — but the worker logs showed the timeout salvage working (`Download timeout for anduo/patreon after 1170.0s (18 files written)`), so this was a second, downstream bug. ## Root cause `DownloadService` held the async + sync DB connections checked out across the entire (≤19.5-min backfill) gallery-dl subprocess. The server reaped the idle connection, so phase 3's first query died with `asyncpg ConnectionDoesNotExistError`. That `DBAPIError` tripped `download_source`'s `autoretry_for`; the retry re-entered `_phase1_setup`, hit the in-flight guard (event still `running`), returned `in_flight` and no-opped in ~0.15s — leaving the event to strand empty for the recovery sweep. `pool_pre_ping` was already on but can't validate a *held* connection (only pool checkouts). ## Changes 1. `DownloadService.download_source` closes the async + sync sessions after phase 1, before the subprocess, so phase 3 re-acquires a live connection (makes the class's "Phase 2 — no DB connection" docstring true). 2. The per-task async engine switches to `NullPool` so the phase-3 reconnect always opens a fresh socket, not a reaped pooled one. ## Tests `test_releases_db_connections_before_subprocess` (release happens before `gdl.download`, event still finalizes) + `test_async_task_engine_uses_nullpool`. All existing `test_download_service.py` tests still pass. CI green on dev HEAD `576e16d`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

bvandeusen added 1 commit 2026-06-03 22:11:31 -04:00

fix(download): release DB connections across the gallery-dl subprocess ... 576e16d14d

Backfill events were STILL stranding empty after the timeout-ladder fix.
Worker logs showed the salvage path working ("Download timeout for
anduo/patreon after 1170.0s (18 files written)") but then:
  Retry in 3s: DBAPIError(ConnectionDoesNotExistError: connection was
  closed in the middle of operation)
  ...succeeded in 0.149s   <- in-flight guard no-op

Root cause: DownloadService held the async + sync DB connections checked
out across the entire (≤19.5-min backfill) gallery-dl subprocess. The
server reaps the idle connection, so phase 3's first query hits a dead
socket. That DBAPIError trips download_source's autoretry_for, the retry
re-enters _phase1_setup, sees the event still 'running', returns
in_flight and no-ops — leaving the event to be stranded empty by the
recovery sweep. pool_pre_ping was already on both engines but can't help
a *held* connection (it only validates on pool checkout).

Fix:
- DownloadService.download_source closes the async + sync sessions after
  phase 1, before the subprocess, so phase 3 re-acquires a live
  connection (matches the class's "Phase 2 — no DB connection" docstring).
- The per-task async engine switches to NullPool so phase 3 always opens
  a fresh connection rather than a pooled one the server may have reaped.

Tests: assert connections are released before gdl.download runs and the
event still finalizes; assert the task engine uses NullPool. Also fixes a
stale 1800s->1170s comment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

bvandeusen merged commit 844bb86802 into main 2026-06-03 22:11:36 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#60