From 426c80fa920eb83edc1badfa6b71ed07e3304ccf Mon Sep 17 00:00:00 2001 From: Amadeus Mader Date: Sat, 7 Mar 2026 18:31:45 +0100 Subject: [PATCH] feat(indexing): auto-advance download_cutoff_date after successful indexing After each successful indexing run, the source's download_cutoff_date is automatically advanced to 7 days ago if the current cutoff date is older (or nil). This prevents yt-dlp from scanning through months of old videos on every index run, significantly improving indexing performance for channels with large backlogs. The 7-day buffer ensures recent videos are still checked on subsequent index runs while keeping indexing fast (minutes instead of hours). Also updates AGENTS.md to document Docker-based development workflow and adds Pinchflat_improvements.md for tracking known issues. --- AGENTS.md | 28 ++- Pinchflat_improvements.md | 230 ++++++++++++++++++ .../slow_indexing/slow_indexing_helpers.ex | 34 ++- .../slow_indexing_helpers_test.exs | 31 +++ test/support/fixtures/sources_fixtures.ex | 13 +- 5 files changed, 320 insertions(+), 16 deletions(-) create mode 100644 Pinchflat_improvements.md diff --git a/AGENTS.md b/AGENTS.md index 4143f9e..5b55161 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,36 +13,40 @@ Pinchflat is a self-hosted media management app using: ## Build/Lint/Test Commands -```bash -# Enter Nix dev shell (required for tooling) -nix develop +**Always use Docker for development and testing.** The project has permission issues when mixing Docker and local builds. +**Database backup:** Use `pinchflat.db.backup-*` files for testing and development with real data. These contain production-like data for debugging issues. + +```bash # Run all checks (compile, format, credo, tests, sobelow, prettier) -mix check +docker compose run --rm phx mix check # Run tests only -mix test +docker compose run --rm phx mix test # Run a single test file -mix test test/pinchflat/media_test.exs +docker compose run --rm phx mix test test/pinchflat/media_test.exs # Run a specific test by line number -mix test test/pinchflat/media_test.exs:42 +docker compose run --rm phx mix test test/pinchflat/media_test.exs:42 # Run tests matching a pattern -mix test --only describe:"list_media_items" +docker compose run --rm phx mix test --only describe:"list_media_items" # Format Elixir code -mix format +docker compose run --rm phx mix format # Run Credo linter -mix credo +docker compose run --rm phx mix credo # Security analysis -mix sobelow --config +docker compose run --rm phx mix sobelow --config # Format JS/CSS/YAML/JSON -yarn run lint:fix +docker compose run --rm phx yarn run lint:fix + +# Start the development server +docker compose up ``` ## Code Style Guidelines diff --git a/Pinchflat_improvements.md b/Pinchflat_improvements.md new file mode 100644 index 0000000..1ea6f0a --- /dev/null +++ b/Pinchflat_improvements.md @@ -0,0 +1,230 @@ +# Pinchflat Improvements + +Issues discovered while debugging Jellyfin artwork problems. + +## Issue 1: Source images not downloaded for early sources + +**Problem:** Sources created before `download_source_images` was added to the media profile have empty `poster_filepath`, `fanart_filepath`, and `banner_filepath` in the `sources` table. The `source_metadata` table may have stale references to files that were never actually downloaded. + +**Example:** Tsoding (source id 11, created 2025-06-04) + +- `sources` table has empty artwork paths +- `source_metadata` references `/config/metadata/sources/11/*.jpg` but folder doesn't exist + +**Expected behavior:** When `download_source_images: true` is set on a media profile, existing sources using that profile should have their artwork downloaded on next index. + +**Suggested fix:** Add a migration or background job that: + +1. Checks all sources using profiles with `download_source_images: true` +2. Downloads missing source images for sources that don't have them +3. Updates both `sources` and `source_metadata` tables + +## Issue 2: Source images not downloaded for some new sources + +**Problem:** Some recently added sources have no `source_metadata` entry at all, even though they use a profile with `download_source_images: true`. + +**Example:** Jimmy_Broadbent (source id 97, created 2026-01-13) + +- No entry in `source_metadata` table +- No metadata folder at `/config/metadata/sources/97/` +- No artwork files in show folder +- 1521 media items indexed, videos were downloaded (then culled due to 3-day retention) + +**Expected behavior:** Source metadata and images should be downloaded when a source is first indexed, regardless of retention settings. + +**Suggested fix:** + +1. Investigate why `source_metadata` entry wasn't created during initial indexing +2. Ensure source image download happens before/independently of media downloads +3. Add error logging if source image download fails + +## Issue 3: No way to re-trigger source image download + +**Problem:** There's no UI option or API endpoint to re-download source images for an existing source. Users must delete and re-add the source. + +**Suggested fix:** Add a "Refresh source metadata" button in the source settings that: + +1. Re-fetches channel metadata from YouTube +2. Downloads/updates poster, fanart, and banner images +3. Updates the `source_metadata` and `sources` tables +4. Copies images to the show folder + +## Database schema reference + +```sql +-- sources table (relevant columns) +SELECT id, custom_name, poster_filepath, fanart_filepath, banner_filepath FROM sources; + +-- source_metadata table +SELECT source_id, metadata_filepath, fanart_filepath, poster_filepath, banner_filepath FROM source_metadata; + +-- Metadata files stored in +/config/metadata/sources/{source_id}/ + - metadata.json.gz + - poster.jpg + - fanart.jpg + - banner.jpg + +-- Show folder should have copies +/downloads/shows/{source_custom_name}/ + - poster.jpg + - fanart.jpg + - banner.jpg + - tvshow.nfo +``` + +## Issue 4: SQLite "Database busy" errors blocking job processing + +**Problem:** The Oban job queue can get stuck with "Database busy" errors, preventing all indexing and download jobs from processing. This occurs when: + +1. Long-running operations hold database locks +2. The WAL (Write-Ahead Log) file grows very large (observed 252MB) +3. Multiple concurrent operations compete for write access + +**Symptoms:** + +- Logs show repeated `(Exqlite.Error) Database busy` errors +- `Oban.Stager` GenServer terminates repeatedly +- Jobs stay in `available` state but never execute +- `last_indexed_at` for sources becomes very stale (months old) + +**Example log:** + +``` +[error] | GenServer {Oban.Registry, {Oban, Oban.Stager}} terminating +** (Exqlite.Error) Database busy +UPDATE "oban_jobs" AS o0 SET "state" = ? WHERE (o0."id" IN (?)) +``` + +**Root cause:** SQLite's `busy_timeout` PRAGMA defaults to 0 in the application, meaning queries fail immediately instead of waiting for locks to be released. + +**Suggested fixes:** + +1. **Set busy_timeout PRAGMA** - Configure Exqlite/Ecto to use a reasonable busy_timeout (e.g., 5000ms) so queries wait for locks instead of failing immediately +2. **Configure WAL autocheckpoint** - Ensure WAL doesn't grow unbounded; set `wal_autocheckpoint` to a reasonable value +3. **Add connection pool limits** - Limit concurrent database writers to reduce lock contention +4. **Implement job queue cleanup** - Automatically prune old completed/cancelled jobs to keep the oban_jobs table smaller + +**Workaround:** Restart Pinchflat container to clear stuck state and checkpoint WAL: + +```bash +docker compose restart pinchflat +``` + +**Manual database maintenance:** + +```bash +# Stop Pinchflat first +docker compose stop pinchflat + +# Clean old jobs and optimize +sqlite3 /path/to/pinchflat.db " + DELETE FROM oban_jobs WHERE state = 'completed' AND completed_at < datetime('now', '-7 days'); + DELETE FROM oban_jobs WHERE state = 'cancelled' AND scheduled_at < datetime('now', '-7 days'); + ANALYZE; + VACUUM; +" + +# Start Pinchflat +docker compose start pinchflat +``` + +## Issue 5: Orphaned deletion jobs for non-existent sources + +**Problem:** When a source is deleted, a `SourceDeletionWorker` job is created. If this job fails repeatedly (e.g., due to database busy errors or files already deleted), it keeps retrying indefinitely even after the source no longer exists. + +**Example:** Source ID 97 was deleted, but the deletion job kept retrying (14 attempts) even though there was nothing left to delete. + +**Suggested fix:** + +1. Add a check at the start of `SourceDeletionWorker` to verify the source still exists +2. If source doesn't exist, mark job as completed (or cancelled) instead of retrying +3. Consider adding idempotency - if files/records are already gone, succeed gracefully + +## Issue 6: Sources with `last_indexed_at = NULL` never get indexed + +**Problem:** Some sources show `last_indexed_at` as NULL in the database, indicating they were never successfully indexed. These sources have indexing jobs in the queue but the jobs may have failed silently or been stuck. + +**Example sources with NULL `last_indexed_at`:** + +- PietSmietTV (id 91) +- Rory Alexander (id 93) +- BigfryTV (id 94) +- Multiple sources added on 2026-03-06 (ids 101-118) + +**Suggested fix:** + +1. Add monitoring/alerting for sources that haven't been indexed within expected timeframe +2. Add a "Force re-index" button in the UI +3. Log more details when indexing jobs fail +4. Consider a health check that identifies sources with stale/null `last_indexed_at` + +## Issue 7: SlowIndexing jobs don't show as "executing" in UI + +**Problem:** When `MediaCollectionIndexingWorker` (SlowIndexing) jobs start running, they spawn yt-dlp processes but fail to update their job state from `available` to `executing` in the database. This causes: + +1. No tasks visible in the frontend at `/#tab-active-tasks` +2. Jobs appear stuck in `available` state even though yt-dlp is actively running +3. Users have no visibility into what Pinchflat is currently doing + +**Observed behavior:** + +```bash +# Database shows no executing jobs +sqlite3 pinchflat.db "SELECT state, COUNT(*) FROM oban_jobs WHERE state = 'executing'" +# Returns: (empty) + +# But yt-dlp processes are actively running +ps aux | grep yt-dlp +# Shows multiple yt-dlp processes for OutdoorBoys, USCSB, etc. +``` + +**Contrast with FastIndexing:** `FastIndexingWorker` jobs DO properly transition to `executing` state and show in the UI. Only `SlowIndexing` jobs have this issue. + +**Root cause hypothesis:** The SlowIndexing worker spawns an external yt-dlp process but the database state update either: + +1. Fails silently due to SQLite busy errors +2. Happens in a transaction that doesn't commit before the long-running process starts +3. Is not implemented for this worker type + +**Impact:** + +- Users think nothing is happening when indexing is actually in progress +- No way to see progress or estimate completion time +- Difficult to debug issues without visibility + +**Suggested fix:** + +1. Ensure job state is updated to `executing` BEFORE spawning the yt-dlp process +2. Use a database transaction with proper busy_timeout to ensure the state change persists +3. Add periodic heartbeat updates during long-running indexing operations +4. Consider adding progress tracking (e.g., "indexed 50/500 videos") + +## Issue 8: Old download_cutoff_date causes unnecessary full channel scans [COMPLETED] + +**Problem:** The `download_cutoff_date` field on sources doesn't automatically update over time. Sources created months ago still have their original cutoff date, causing yt-dlp to scan through thousands of old videos on every index. + +**Example:** OutdoorBoys had `download_cutoff_date = 2025-11-08` even though it was March 2026, causing yt-dlp to check 4+ months of videos (~493 videos) on every indexing run. + +**Impact:** + +- Extremely slow indexing (hours instead of minutes) +- Unnecessary YouTube API calls (risk of rate limiting) +- High CPU usage from yt-dlp processing old video metadata + +**Observed timing:** With 40-second sleep intervals between requests, indexing 493 videos takes ~5.5 hours minimum. + +**Suggested fixes:** + +1. **Auto-advance cutoff date** - After successful indexing, update `download_cutoff_date` to a recent date (e.g., 7 days before the oldest video that was actually downloaded) +2. **Use relative cutoff** - Instead of absolute dates, allow setting "download videos from last N days" +3. **Add --break-on-existing consistently** - Ensure all indexing jobs use this flag to stop early when hitting known videos +4. **UI warning** - Show a warning if cutoff date is very old + +**Manual fix:** + +```sql +-- Update all sources to have cutoff date of 7 days ago +UPDATE sources SET download_cutoff_date = date('now', '-7 days') +WHERE download_cutoff_date < date('now', '-7 days'); +``` diff --git a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex index 8721b21..4fcb19b 100644 --- a/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex +++ b/lib/pinchflat/slow_indexing/slow_indexing_helpers.ex @@ -108,7 +108,7 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do end end) - Sources.update_source(source, %{last_indexed_at: DateTime.utc_now()}) + update_source_after_indexing(source) DownloadingHelpers.enqueue_pending_download_tasks(source) result @@ -241,4 +241,36 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpers do [:break_on_existing, download_archive: archive_file] end + + # Updates the source after a successful indexing run. This includes: + # - Setting `last_indexed_at` to the current time + # - Advancing `download_cutoff_date` to 7 days ago if it's older (or nil) + # + # Advancing the cutoff date prevents yt-dlp from scanning through months of old videos + # on every index. We use 7 days as a buffer to ensure we don't miss any videos that + # might have been uploaded just before the cutoff. + # + # We use `run_post_commit_tasks: false` to avoid triggering side effects like + # re-indexing or metadata storage since this is an internal update. + defp update_source_after_indexing(source) do + new_cutoff_date = Date.utc_today() |> Date.add(-7) + + update_attrs = + %{last_indexed_at: DateTime.utc_now()} + |> maybe_advance_cutoff_date(source.download_cutoff_date, new_cutoff_date) + + Sources.update_source(source, update_attrs, run_post_commit_tasks: false) + end + + defp maybe_advance_cutoff_date(attrs, nil, new_cutoff_date) do + Map.put(attrs, :download_cutoff_date, new_cutoff_date) + end + + defp maybe_advance_cutoff_date(attrs, current_cutoff_date, new_cutoff_date) do + if Date.compare(current_cutoff_date, new_cutoff_date) == :lt do + Map.put(attrs, :download_cutoff_date, new_cutoff_date) + else + attrs + end + end end diff --git a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs index d37721a..e71c812 100644 --- a/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs +++ b/test/pinchflat/slow_indexing/slow_indexing_helpers_test.exs @@ -228,6 +228,37 @@ defmodule Pinchflat.SlowIndexing.SlowIndexingHelpersTest do assert DateTime.diff(DateTime.utc_now(), source.last_indexed_at) < 2 end + test "advances download_cutoff_date to 7 days ago when it's nil" do + source = source_fixture(download_cutoff_date: nil) + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + source = Repo.reload!(source) + + expected_cutoff = Date.utc_today() |> Date.add(-7) + assert source.download_cutoff_date == expected_cutoff + end + + test "advances download_cutoff_date to 7 days ago when it's older" do + old_cutoff = Date.utc_today() |> Date.add(-30) + source = source_fixture(download_cutoff_date: old_cutoff) + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + source = Repo.reload!(source) + + expected_cutoff = Date.utc_today() |> Date.add(-7) + assert source.download_cutoff_date == expected_cutoff + end + + test "does not change download_cutoff_date when it's more recent than 7 days ago" do + recent_cutoff = Date.utc_today() |> Date.add(-3) + source = source_fixture(download_cutoff_date: recent_cutoff) + + SlowIndexingHelpers.index_and_enqueue_download_for_media_items(source) + source = Repo.reload!(source) + + assert source.download_cutoff_date == recent_cutoff + end + test "enqueues a job for each pending media item" do source = source_fixture() media_item = media_item_fixture(source_id: source.id, media_filepath: nil) diff --git a/test/support/fixtures/sources_fixtures.ex b/test/support/fixtures/sources_fixtures.ex index 9d699dc..b29e65a 100644 --- a/test/support/fixtures/sources_fixtures.ex +++ b/test/support/fixtures/sources_fixtures.ex @@ -77,6 +77,13 @@ defmodule Pinchflat.SourcesFixtures do end def source_attributes_return_fixture do + # Use recent dates to ensure media items pass the download_cutoff_date filter + # (which is set to 7 days ago after indexing) + today = Date.utc_today() + date1 = Date.add(today, -1) |> Calendar.strftime("%Y%m%d") + date2 = Date.add(today, -2) |> Calendar.strftime("%Y%m%d") + date3 = Date.add(today, -3) |> Calendar.strftime("%Y%m%d") + source_attributes = [ %{ id: "video1", @@ -86,7 +93,7 @@ defmodule Pinchflat.SourcesFixtures do description: "desc1", aspect_ratio: 1.67, duration: 12.34, - upload_date: "20210101" + upload_date: date1 }, %{ id: "video2", @@ -96,7 +103,7 @@ defmodule Pinchflat.SourcesFixtures do description: "desc2", aspect_ratio: 1.67, duration: 345.67, - upload_date: "20220202" + upload_date: date2 }, %{ id: "video3", @@ -106,7 +113,7 @@ defmodule Pinchflat.SourcesFixtures do description: "desc3", aspect_ratio: 1.0, duration: 678.90, - upload_date: "20230303" + upload_date: date3 } ]