diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index b7f1248e..669f6f88 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -26,6 +26,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" "git.fabledsword.com/bvandeusen/minstrel/internal/logging" "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" + "git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition" "git.fabledsword.com/bvandeusen/minstrel/internal/recsettings" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble/listenbrainz" @@ -269,6 +270,28 @@ func run() error { lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, lidarrClientFn, logger.With("component", "lidarr"), bus) go lidarrReconciler.Run(ctx) + // Missing-file re-acquisition (milestone #290). Turns albums whose files + // have been gone longer than the grace window into Lidarr requests, on an + // exponential per-album backoff. Hourly tick — the shortest meaningful + // backoff is measured in hours, so waking more often would only re-read + // settings and find nothing due. + // + // A settings-load failure is logged, not fatal: NewSettingsService always + // returns a usable service holding the defaults, and running on defaults + // is far better than dropping the feature because the database hiccuped + // during boot. + reacqSettings, reacqErr := reacquisition.NewSettingsService( + ctx, pool, logger.With("component", "reacquisition")) + if reacqErr != nil { + logger.Warn("reacquisition: using default settings", "err", reacqErr) + } + go reacquisition.NewSweeper( + pool, + reacqSettings, + lidarrrequests.NewService(pool, lidarrCfg, lidarrClientFn, nil), + logger.With("component", "reacquisition"), + ).Run(ctx) + // library_changes compactor (#357 follow-up). Daily tick; deletes // rows older than the configured retention so the change-log table // doesn't grow unbounded. Clients that drop offline longer than diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index e4e01981..d0d4a5f1 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -381,6 +381,16 @@ type LidarrRequest struct { LidarrAddConfirmedAt pgtype.Timestamptz } +type MissingReacquisition struct { + AlbumID pgtype.UUID + Attempts int32 + LastAttemptAt pgtype.Timestamptz + LastRequestID pgtype.UUID + GaveUpAt pgtype.Timestamptz + CreatedAt pgtype.Timestamptz + UpdatedAt pgtype.Timestamptz +} + type NetworkSetting struct { ID bool TrustedProxyHops int32 @@ -463,6 +473,17 @@ type PlaylistTrack struct { PickKind *string } +type ReacquisitionSetting struct { + ID bool + Enabled bool + GraceHours int32 + BackoffBaseHours int32 + BackoffMaxHours int32 + MaxAttempts int32 + MaxPerPass int32 + AutoApprove bool +} + type RecommendationTuningAudit struct { ID int64 ChangedAt pgtype.Timestamptz diff --git a/internal/db/dbq/reacquisition.sql.go b/internal/db/dbq/reacquisition.sql.go new file mode 100644 index 00000000..335e2771 --- /dev/null +++ b/internal/db/dbq/reacquisition.sql.go @@ -0,0 +1,310 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: reacquisition.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const clearRecoveredReacquisitions = `-- name: ClearRecoveredReacquisitions :execrows +DELETE FROM missing_reacquisitions r + WHERE NOT EXISTS ( + SELECT 1 FROM tracks + WHERE tracks.album_id = r.album_id + AND tracks.missing_since IS NOT NULL + ) +` + +// Drops state for albums that no longer have any missing track — the files +// came back, or the scanner adopted them at a new path (#2528). Deleting +// rather than resetting counters means a future loss starts from a clean +// budget, which is right: it is a new problem, not a continuation. +func (q *Queries) ClearRecoveredReacquisitions(ctx context.Context) (int64, error) { + result, err := q.db.Exec(ctx, clearRecoveredReacquisitions) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const countAlbumsMissingWithoutMbid = `-- name: CountAlbumsMissingWithoutMbid :one +SELECT COUNT(DISTINCT albums.id)::bigint + FROM albums + JOIN artists ON artists.id = albums.artist_id + JOIN tracks ON tracks.album_id = albums.id + WHERE tracks.missing_since IS NOT NULL + AND (albums.mbid IS NULL OR artists.mbid IS NULL) +` + +// Albums with missing files that can never be auto-requested because nothing +// identifies them to MusicBrainz. Surfaced on the admin card so the gap is +// visible: silently doing nothing for these would read as the feature being +// broken. +func (q *Queries) CountAlbumsMissingWithoutMbid(ctx context.Context) (int64, error) { + row := q.db.QueryRow(ctx, countAlbumsMissingWithoutMbid) + var column_1 int64 + err := row.Scan(&column_1) + return column_1, err +} + +const getReacquisitionForAlbums = `-- name: GetReacquisitionForAlbums :many +SELECT album_id, attempts, last_attempt_at, last_request_id, gave_up_at, created_at, updated_at FROM missing_reacquisitions WHERE album_id = ANY($1::uuid[]) +` + +// State for the admin missing-files surface, so each directory group can say +// whether a re-acquisition is in flight, waiting, or given up. +func (q *Queries) GetReacquisitionForAlbums(ctx context.Context, albumIds []pgtype.UUID) ([]MissingReacquisition, error) { + rows, err := q.db.Query(ctx, getReacquisitionForAlbums, albumIds) + if err != nil { + return nil, err + } + defer rows.Close() + var items []MissingReacquisition + for rows.Next() { + var i MissingReacquisition + if err := rows.Scan( + &i.AlbumID, + &i.Attempts, + &i.LastAttemptAt, + &i.LastRequestID, + &i.GaveUpAt, + &i.CreatedAt, + &i.UpdatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const getReacquisitionSettings = `-- name: GetReacquisitionSettings :one + +SELECT id, enabled, grace_hours, backoff_base_hours, backoff_max_hours, max_attempts, max_per_pass, auto_approve FROM reacquisition_settings WHERE id = true +` + +// Auto re-acquisition of missing files (milestone #290). The unit is the +// album: Lidarr acquires releases, and grouping collapses "40 missing files" +// into "3 albums to ask for". +func (q *Queries) GetReacquisitionSettings(ctx context.Context) (ReacquisitionSetting, error) { + row := q.db.QueryRow(ctx, getReacquisitionSettings) + var i ReacquisitionSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.GraceHours, + &i.BackoffBaseHours, + &i.BackoffMaxHours, + &i.MaxAttempts, + &i.MaxPerPass, + &i.AutoApprove, + ) + return i, err +} + +const listAlbumsDueReacquisition = `-- name: ListAlbumsDueReacquisition :many +SELECT albums.id AS album_id, + albums.title AS album_title, + albums.mbid AS album_mbid, + artists.id AS artist_id, + artists.name AS artist_name, + artists.mbid AS artist_mbid, + COUNT(tracks.id)::bigint AS missing_track_count, + COALESCE(r.attempts, 0)::int AS attempts + FROM albums + JOIN artists ON artists.id = albums.artist_id + JOIN tracks ON tracks.album_id = albums.id + LEFT JOIN missing_reacquisitions r ON r.album_id = albums.id + WHERE tracks.missing_since IS NOT NULL + AND tracks.missing_since <= now() - make_interval(hours => $1::int) + AND albums.mbid IS NOT NULL + AND artists.mbid IS NOT NULL + AND (r.gave_up_at IS NULL) + AND ( + r.last_attempt_at IS NULL + OR r.last_attempt_at <= now() - make_interval(hours => LEAST( + ($2::int + * POWER(2, GREATEST(COALESCE(r.attempts, 0) - 1, 0)))::int, + $3::int)) + ) + GROUP BY albums.id, albums.title, albums.mbid, + artists.id, artists.name, artists.mbid, r.attempts, r.last_attempt_at + ORDER BY r.last_attempt_at NULLS FIRST, albums.sort_title + LIMIT $4 +` + +type ListAlbumsDueReacquisitionParams struct { + GraceHours int32 + BackoffBaseHours int32 + BackoffMaxHours int32 + PageLimit int32 +} + +type ListAlbumsDueReacquisitionRow struct { + AlbumID pgtype.UUID + AlbumTitle string + AlbumMbid *string + ArtistID pgtype.UUID + ArtistName string + ArtistMbid *string + MissingTrackCount int64 + Attempts int32 +} + +// The sweeper's selection. An album qualifies when: +// +// - it still has at least one track whose file has been missing longer than +// the grace window. Measured on missing_since, which reconcile never +// re-stamps (#2523), so it is a genuine "gone since" clock rather than +// "when we last noticed"; +// - MusicBrainz can name it. Both the album and its artist MBID are +// required — Create rejects an album-kind request without them, and there +// is nothing to ask Lidarr for anyway. Albums failing this are counted +// separately (CountAlbumsMissingWithoutMbid) rather than vanishing; +// - it has not spent its attempt budget (gave_up_at IS NULL); +// - its backoff has elapsed: base * 2^(attempts-1) hours since the last +// attempt, clamped to the configured maximum. First attempt (no row, or +// last_attempt_at NULL) is always due. +// +// Oldest attempt first, never-attempted first, so a large loss drains in a +// stable order across passes instead of re-picking the same head each time. +func (q *Queries) ListAlbumsDueReacquisition(ctx context.Context, arg ListAlbumsDueReacquisitionParams) ([]ListAlbumsDueReacquisitionRow, error) { + rows, err := q.db.Query(ctx, listAlbumsDueReacquisition, + arg.GraceHours, + arg.BackoffBaseHours, + arg.BackoffMaxHours, + arg.PageLimit, + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsDueReacquisitionRow + for rows.Next() { + var i ListAlbumsDueReacquisitionRow + if err := rows.Scan( + &i.AlbumID, + &i.AlbumTitle, + &i.AlbumMbid, + &i.ArtistID, + &i.ArtistName, + &i.ArtistMbid, + &i.MissingTrackCount, + &i.Attempts, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const markReacquisitionGaveUp = `-- name: MarkReacquisitionGaveUp :exec +UPDATE missing_reacquisitions + SET gave_up_at = now(), + updated_at = now() + WHERE album_id = $1 + AND gave_up_at IS NULL +` + +// Stamped when the attempt budget is spent. Stored as a timestamp rather than +// inferred from `attempts >= max_attempts` so the verdict survives an operator +// later raising the maximum, and so the admin surface can say when. +func (q *Queries) MarkReacquisitionGaveUp(ctx context.Context, albumID pgtype.UUID) error { + _, err := q.db.Exec(ctx, markReacquisitionGaveUp, albumID) + return err +} + +const recordReacquisitionAttempt = `-- name: RecordReacquisitionAttempt :one +INSERT INTO missing_reacquisitions (album_id, attempts, last_attempt_at, last_request_id) +VALUES ($1, 1, now(), $2) +ON CONFLICT (album_id) DO UPDATE + SET attempts = missing_reacquisitions.attempts + 1, + last_attempt_at = now(), + last_request_id = COALESCE(EXCLUDED.last_request_id, + missing_reacquisitions.last_request_id), + updated_at = now() +RETURNING album_id, attempts, last_attempt_at, last_request_id, gave_up_at, created_at, updated_at +` + +type RecordReacquisitionAttemptParams struct { + AlbumID pgtype.UUID + LastRequestID pgtype.UUID +} + +// Bumps the attempt counter and stamps the clock the backoff measures from. +// Upsert because the first attempt has no row yet. +func (q *Queries) RecordReacquisitionAttempt(ctx context.Context, arg RecordReacquisitionAttemptParams) (MissingReacquisition, error) { + row := q.db.QueryRow(ctx, recordReacquisitionAttempt, arg.AlbumID, arg.LastRequestID) + var i MissingReacquisition + err := row.Scan( + &i.AlbumID, + &i.Attempts, + &i.LastAttemptAt, + &i.LastRequestID, + &i.GaveUpAt, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const updateReacquisitionSettings = `-- name: UpdateReacquisitionSettings :one +UPDATE reacquisition_settings + SET enabled = $1, + grace_hours = $2, + backoff_base_hours = $3, + backoff_max_hours = $4, + max_attempts = $5, + max_per_pass = $6, + auto_approve = $7 + WHERE id = true +RETURNING id, enabled, grace_hours, backoff_base_hours, backoff_max_hours, max_attempts, max_per_pass, auto_approve +` + +type UpdateReacquisitionSettingsParams struct { + Enabled bool + GraceHours int32 + BackoffBaseHours int32 + BackoffMaxHours int32 + MaxAttempts int32 + MaxPerPass int32 + AutoApprove bool +} + +// Whole-row write from the admin card; the CHECKs in migration 0056 are the +// validation, so a bad value fails loudly rather than being clamped silently. +func (q *Queries) UpdateReacquisitionSettings(ctx context.Context, arg UpdateReacquisitionSettingsParams) (ReacquisitionSetting, error) { + row := q.db.QueryRow(ctx, updateReacquisitionSettings, + arg.Enabled, + arg.GraceHours, + arg.BackoffBaseHours, + arg.BackoffMaxHours, + arg.MaxAttempts, + arg.MaxPerPass, + arg.AutoApprove, + ) + var i ReacquisitionSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.GraceHours, + &i.BackoffBaseHours, + &i.BackoffMaxHours, + &i.MaxAttempts, + &i.MaxPerPass, + &i.AutoApprove, + ) + return i, err +} diff --git a/internal/db/dbq/users.sql.go b/internal/db/dbq/users.sql.go index fe5fea6f..3ffe3499 100644 --- a/internal/db/dbq/users.sql.go +++ b/internal/db/dbq/users.sql.go @@ -239,6 +239,43 @@ func (q *Queries) GetListenBrainzConfig(ctx context.Context, id pgtype.UUID) (Ge return i, err } +const getOldestAdmin = `-- name: GetOldestAdmin :one +SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users + WHERE is_admin = true + ORDER BY created_at, id + LIMIT 1 +` + +// The account a system-initiated action is attributed to (milestone #290). +// lidarr_requests.user_id is NOT NULL and a re-acquisition has no requesting +// human, so the row is owned by the longest-standing admin: it keeps the +// request auditable and puts it in the same admin queue as everything else, +// without inventing a synthetic principal the rest of the schema would have +// to understand. Ordered by id as a tiebreak so the choice is stable across +// calls rather than depending on scan order. +func (q *Queries) GetOldestAdmin(ctx context.Context) (User, error) { + row := q.db.QueryRow(ctx, getOldestAdmin) + var i User + err := row.Scan( + &i.ID, + &i.Username, + &i.PasswordHash, + &i.ApiToken, + &i.IsAdmin, + &i.CreatedAt, + &i.SubsonicPassword, + &i.ListenbrainzToken, + &i.ListenbrainzEnabled, + &i.DisplayName, + &i.AutoApproveRequests, + &i.Email, + &i.Timezone, + &i.TimezoneUpdatedAt, + &i.DebugModeEnabled, + ) + return i, err +} + const getUserByAPIToken = `-- name: GetUserByAPIToken :one SELECT id, username, password_hash, api_token, is_admin, created_at, subsonic_password, listenbrainz_token, listenbrainz_enabled, display_name, auto_approve_requests, email, timezone, timezone_updated_at, debug_mode_enabled FROM users WHERE api_token = $1 ` diff --git a/internal/db/migrations/0056_missing_reacquisition.down.sql b/internal/db/migrations/0056_missing_reacquisition.down.sql new file mode 100644 index 00000000..5acbc0ba --- /dev/null +++ b/internal/db/migrations/0056_missing_reacquisition.down.sql @@ -0,0 +1,2 @@ +DROP TABLE IF EXISTS reacquisition_settings; +DROP TABLE IF EXISTS missing_reacquisitions; diff --git a/internal/db/migrations/0056_missing_reacquisition.up.sql b/internal/db/migrations/0056_missing_reacquisition.up.sql new file mode 100644 index 00000000..a4e73b4b --- /dev/null +++ b/internal/db/migrations/0056_missing_reacquisition.up.sql @@ -0,0 +1,98 @@ +-- Auto re-acquisition of missing files via Lidarr (#2527 slice 3, milestone +-- #290). A file going missing has been a dead end until now: reconcile marks +-- it (#2523), every selection path skips it, the admin surface lists it, and +-- there it sits. +-- +-- The unit here is the ALBUM, not the track, and that is the design decision +-- carrying most of the safety. Lidarr acquires releases; there is no +-- meaningful "fetch me one track" operation, and a track-kind request needs a +-- recording MBID plenty of files simply don't have. Grouping by album means +-- the case that produced #2523 -- three reorganised albums, ~40 missing files +-- -- becomes three requests instead of forty. + +CREATE TABLE missing_reacquisitions ( + album_id uuid PRIMARY KEY REFERENCES albums (id) ON DELETE CASCADE, + attempts int NOT NULL DEFAULT 0, + last_attempt_at timestamptz, + -- The request this album's most recent attempt produced. SET NULL rather + -- than CASCADE: a purged request row must not erase the attempt history + -- that stops us asking again in a loop. + last_request_id uuid REFERENCES lidarr_requests (id) ON DELETE SET NULL, + -- Set when the attempt budget is spent. Distinct from "attempts = max" + -- so the reason survives a later change to the configured maximum, and so + -- the admin surface can say "gave up on the 3rd of August" rather than + -- inferring it. + gave_up_at timestamptz, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT missing_reacquisitions_attempts_nonneg CHECK (attempts >= 0) +); + +-- The sweeper's own read: albums due another attempt, oldest attempt first. +-- Partial on the not-given-up rows because a spent album is never selected +-- again and would otherwise grow the index forever. +CREATE INDEX missing_reacquisitions_due_idx + ON missing_reacquisitions (last_attempt_at NULLS FIRST) + WHERE gave_up_at IS NULL; + +-- Settings, singleton in the style of network_settings (0053). Every value an +-- operator might want to tune lives here rather than in YAML (rule #25). +CREATE TABLE reacquisition_settings ( + id boolean PRIMARY KEY DEFAULT true, + + -- Master switch. Default true: the operator asked for this to happen by + -- itself, and a feature that ships switched off is a feature nobody finds. + enabled boolean NOT NULL DEFAULT true, + + -- How long a file must have been missing before the FIRST attempt. This + -- is what separates "automatic" from "trigger-happy": a filesystem lies + -- transiently -- an unmounted volume, a container that started before its + -- media mount attached, a NAS mid-reboot -- and every one of those + -- resolves itself well inside a day at no cost. tracks.missing_since is + -- never re-stamped (#2523), so it is a true "gone since" clock to measure + -- against. + grace_hours int NOT NULL DEFAULT 24, + + -- Exponential spacing between attempts: base * 2^(attempts-1), clamped to + -- backoff_max_hours. 6h -> 12h -> 24h -> 48h by default. An album Lidarr + -- genuinely cannot find must get quieter, not keep pace. + backoff_base_hours int NOT NULL DEFAULT 6, + backoff_max_hours int NOT NULL DEFAULT 168, -- one week + + -- Attempts before giving up. Three real tries spread over days is enough + -- to ride out a transient Lidarr/indexer outage; past that the answer is + -- "this release is not obtainable" and asking again is noise. + max_attempts int NOT NULL DEFAULT 3, + + -- Ceiling on requests created per sweep. Album grouping already collapses + -- the common case, but a genuinely large loss (a whole drive slipping + -- under reconcile's 25% mark cap) should still trickle rather than dump + -- hundreds of requests into the queue at once. + max_per_pass int NOT NULL DEFAULT 20, + + -- Whether the sweeper approves what it creates. Requests are created + -- pending and nothing reaches Lidarr until approval, so with this off the + -- feature is a notification rather than an attempt -- which is why the + -- default is on. Off is the review-first posture: rows appear in the + -- admin Requests queue for a human to release. + auto_approve boolean NOT NULL DEFAULT true, + + CONSTRAINT reacquisition_settings_singleton CHECK (id = true), + -- Ranges exist to stop a typo becoming a behaviour change: a 0-hour grace + -- would fire on every transient unmount, and a 10000-per-pass cap would + -- defeat the point of having one. + CONSTRAINT reacquisition_settings_grace_range + CHECK (grace_hours >= 1 AND grace_hours <= 720), + CONSTRAINT reacquisition_settings_backoff_base_range + CHECK (backoff_base_hours >= 1 AND backoff_base_hours <= 168), + CONSTRAINT reacquisition_settings_backoff_max_range + CHECK (backoff_max_hours >= 1 AND backoff_max_hours <= 720), + CONSTRAINT reacquisition_settings_backoff_ordered + CHECK (backoff_max_hours >= backoff_base_hours), + CONSTRAINT reacquisition_settings_attempts_range + CHECK (max_attempts >= 1 AND max_attempts <= 10), + CONSTRAINT reacquisition_settings_per_pass_range + CHECK (max_per_pass >= 1 AND max_per_pass <= 200) +); + +INSERT INTO reacquisition_settings (id) VALUES (true) ON CONFLICT (id) DO NOTHING; diff --git a/internal/db/queries/reacquisition.sql b/internal/db/queries/reacquisition.sql new file mode 100644 index 00000000..5f638e3e --- /dev/null +++ b/internal/db/queries/reacquisition.sql @@ -0,0 +1,119 @@ +-- Auto re-acquisition of missing files (milestone #290). The unit is the +-- album: Lidarr acquires releases, and grouping collapses "40 missing files" +-- into "3 albums to ask for". + +-- name: GetReacquisitionSettings :one +SELECT * FROM reacquisition_settings WHERE id = true; + +-- name: UpdateReacquisitionSettings :one +-- Whole-row write from the admin card; the CHECKs in migration 0056 are the +-- validation, so a bad value fails loudly rather than being clamped silently. +UPDATE reacquisition_settings + SET enabled = sqlc.arg(enabled), + grace_hours = sqlc.arg(grace_hours), + backoff_base_hours = sqlc.arg(backoff_base_hours), + backoff_max_hours = sqlc.arg(backoff_max_hours), + max_attempts = sqlc.arg(max_attempts), + max_per_pass = sqlc.arg(max_per_pass), + auto_approve = sqlc.arg(auto_approve) + WHERE id = true +RETURNING *; + +-- name: ListAlbumsDueReacquisition :many +-- The sweeper's selection. An album qualifies when: +-- +-- * it still has at least one track whose file has been missing longer than +-- the grace window. Measured on missing_since, which reconcile never +-- re-stamps (#2523), so it is a genuine "gone since" clock rather than +-- "when we last noticed"; +-- * MusicBrainz can name it. Both the album and its artist MBID are +-- required — Create rejects an album-kind request without them, and there +-- is nothing to ask Lidarr for anyway. Albums failing this are counted +-- separately (CountAlbumsMissingWithoutMbid) rather than vanishing; +-- * it has not spent its attempt budget (gave_up_at IS NULL); +-- * its backoff has elapsed: base * 2^(attempts-1) hours since the last +-- attempt, clamped to the configured maximum. First attempt (no row, or +-- last_attempt_at NULL) is always due. +-- +-- Oldest attempt first, never-attempted first, so a large loss drains in a +-- stable order across passes instead of re-picking the same head each time. +SELECT albums.id AS album_id, + albums.title AS album_title, + albums.mbid AS album_mbid, + artists.id AS artist_id, + artists.name AS artist_name, + artists.mbid AS artist_mbid, + COUNT(tracks.id)::bigint AS missing_track_count, + COALESCE(r.attempts, 0)::int AS attempts + FROM albums + JOIN artists ON artists.id = albums.artist_id + JOIN tracks ON tracks.album_id = albums.id + LEFT JOIN missing_reacquisitions r ON r.album_id = albums.id + WHERE tracks.missing_since IS NOT NULL + AND tracks.missing_since <= now() - make_interval(hours => sqlc.arg(grace_hours)::int) + AND albums.mbid IS NOT NULL + AND artists.mbid IS NOT NULL + AND (r.gave_up_at IS NULL) + AND ( + r.last_attempt_at IS NULL + OR r.last_attempt_at <= now() - make_interval(hours => LEAST( + (sqlc.arg(backoff_base_hours)::int + * POWER(2, GREATEST(COALESCE(r.attempts, 0) - 1, 0)))::int, + sqlc.arg(backoff_max_hours)::int)) + ) + GROUP BY albums.id, albums.title, albums.mbid, + artists.id, artists.name, artists.mbid, r.attempts, r.last_attempt_at + ORDER BY r.last_attempt_at NULLS FIRST, albums.sort_title + LIMIT sqlc.arg(page_limit); + +-- name: CountAlbumsMissingWithoutMbid :one +-- Albums with missing files that can never be auto-requested because nothing +-- identifies them to MusicBrainz. Surfaced on the admin card so the gap is +-- visible: silently doing nothing for these would read as the feature being +-- broken. +SELECT COUNT(DISTINCT albums.id)::bigint + FROM albums + JOIN artists ON artists.id = albums.artist_id + JOIN tracks ON tracks.album_id = albums.id + WHERE tracks.missing_since IS NOT NULL + AND (albums.mbid IS NULL OR artists.mbid IS NULL); + +-- name: RecordReacquisitionAttempt :one +-- Bumps the attempt counter and stamps the clock the backoff measures from. +-- Upsert because the first attempt has no row yet. +INSERT INTO missing_reacquisitions (album_id, attempts, last_attempt_at, last_request_id) +VALUES (sqlc.arg(album_id), 1, now(), sqlc.narg(last_request_id)) +ON CONFLICT (album_id) DO UPDATE + SET attempts = missing_reacquisitions.attempts + 1, + last_attempt_at = now(), + last_request_id = COALESCE(EXCLUDED.last_request_id, + missing_reacquisitions.last_request_id), + updated_at = now() +RETURNING *; + +-- name: MarkReacquisitionGaveUp :exec +-- Stamped when the attempt budget is spent. Stored as a timestamp rather than +-- inferred from `attempts >= max_attempts` so the verdict survives an operator +-- later raising the maximum, and so the admin surface can say when. +UPDATE missing_reacquisitions + SET gave_up_at = now(), + updated_at = now() + WHERE album_id = sqlc.arg(album_id) + AND gave_up_at IS NULL; + +-- name: ClearRecoveredReacquisitions :execrows +-- Drops state for albums that no longer have any missing track — the files +-- came back, or the scanner adopted them at a new path (#2528). Deleting +-- rather than resetting counters means a future loss starts from a clean +-- budget, which is right: it is a new problem, not a continuation. +DELETE FROM missing_reacquisitions r + WHERE NOT EXISTS ( + SELECT 1 FROM tracks + WHERE tracks.album_id = r.album_id + AND tracks.missing_since IS NOT NULL + ); + +-- name: GetReacquisitionForAlbums :many +-- State for the admin missing-files surface, so each directory group can say +-- whether a re-acquisition is in flight, waiting, or given up. +SELECT * FROM missing_reacquisitions WHERE album_id = ANY(sqlc.arg(album_ids)::uuid[]); diff --git a/internal/db/queries/users.sql b/internal/db/queries/users.sql index 971a60b3..bbdaac3e 100644 --- a/internal/db/queries/users.sql +++ b/internal/db/queries/users.sql @@ -173,3 +173,16 @@ SELECT u.id, u.timezone FROM users u WHERE pe.user_id = u.id AND pe.started_at > now() - INTERVAL '7 days' ); + +-- name: GetOldestAdmin :one +-- The account a system-initiated action is attributed to (milestone #290). +-- lidarr_requests.user_id is NOT NULL and a re-acquisition has no requesting +-- human, so the row is owned by the longest-standing admin: it keeps the +-- request auditable and puts it in the same admin queue as everything else, +-- without inventing a synthetic principal the rest of the schema would have +-- to understand. Ordered by id as a tiebreak so the choice is stable across +-- calls rather than depending on scan order. +SELECT * FROM users + WHERE is_admin = true + ORDER BY created_at, id + LIMIT 1; diff --git a/internal/reacquisition/settings.go b/internal/reacquisition/settings.go new file mode 100644 index 00000000..455f9f36 --- /dev/null +++ b/internal/reacquisition/settings.go @@ -0,0 +1,166 @@ +// Package reacquisition turns a missing file back into a Lidarr request +// without anyone pressing anything (milestone #290). +// +// The unit of work is the ALBUM, not the track. Lidarr acquires releases; +// there is no meaningful "fetch me one track", and a track-kind request needs +// a recording MBID plenty of files lack. Grouping also does most of the +// safety work: the loss that produced #2523 — three reorganised albums, ~40 +// missing files — becomes three requests rather than forty. +package reacquisition + +import ( + "context" + "errors" + "fmt" + "log/slog" + "sync" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Settings is the operator-tunable policy, mirroring the columns and CHECK +// ranges in migration 0056. +type Settings struct { + Enabled bool + GraceHours int32 + BackoffBaseHours int32 + BackoffMaxHours int32 + MaxAttempts int32 + MaxPerPass int32 + AutoApprove bool +} + +// Defaults mirror migration 0056's column defaults. Duplicated here so a +// database that cannot be read still yields a sane policy rather than a +// zero-valued one — a zero grace window would fire on every transient +// unmount, which is the exact failure the grace period exists to prevent. +var Defaults = Settings{ + Enabled: true, + GraceHours: 24, + BackoffBaseHours: 6, + BackoffMaxHours: 168, + MaxAttempts: 3, + MaxPerPass: 20, + AutoApprove: true, +} + +// ErrOutOfRange is returned by Set for values migration 0056's CHECKs would +// reject, so the API layer answers 400 instead of surfacing a constraint +// violation. +var ErrOutOfRange = errors.New("reacquisition setting out of range") + +// SettingsService caches the settings and owns their persistence. Cached +// because the sweeper reads them every pass and the admin card reads them on +// every render; neither needs a round-trip. +type SettingsService struct { + pool *pgxpool.Pool + logger *slog.Logger + + mu sync.RWMutex + cur Settings +} + +// NewSettingsService loads once and caches. +// +// Always returns a usable service, even alongside a non-nil error: a +// boot-time database hiccup should leave the sweeper running on defaults +// rather than take it out entirely. The error is returned so the caller can +// log that the cache holds defaults rather than stored state. +func NewSettingsService(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*SettingsService, error) { + s := &SettingsService{pool: pool, logger: logger, cur: Defaults} + row, err := dbq.New(pool).GetReacquisitionSettings(ctx) + if err != nil { + return s, fmt.Errorf("reacquisition: load settings: %w", err) + } + s.cur = fromRow(row) + return s, nil +} + +// Get returns the cached settings. +func (s *SettingsService) Get() Settings { + s.mu.RLock() + defer s.mu.RUnlock() + return s.cur +} + +// Set validates, persists and re-caches. +func (s *SettingsService) Set(ctx context.Context, in Settings) (Settings, error) { + if err := validate(in); err != nil { + return Settings{}, err + } + row, err := dbq.New(s.pool).UpdateReacquisitionSettings(ctx, dbq.UpdateReacquisitionSettingsParams{ + Enabled: in.Enabled, + GraceHours: in.GraceHours, + BackoffBaseHours: in.BackoffBaseHours, + BackoffMaxHours: in.BackoffMaxHours, + MaxAttempts: in.MaxAttempts, + MaxPerPass: in.MaxPerPass, + AutoApprove: in.AutoApprove, + }) + if err != nil { + return Settings{}, fmt.Errorf("reacquisition: save settings: %w", err) + } + out := fromRow(row) + s.mu.Lock() + s.cur = out + s.mu.Unlock() + return out, nil +} + +// Backoff is how long to wait before the next attempt on an album that has +// already been tried [attempts] times: base * 2^(attempts-1), clamped to the +// configured maximum. Zero attempts means "never tried", which is always due. +// +// Exported and pure so the schedule is testable without a database, and so +// the admin surface can show the same number the sweeper will act on. +func (s Settings) Backoff(attempts int32) time.Duration { + if attempts <= 0 { + return 0 + } + hours := s.BackoffBaseHours + for i := int32(1); i < attempts; i++ { + hours *= 2 + // Clamp inside the loop as well as after: doubling from a large base + // enough times would overflow int32 before the comparison ran. + if hours >= s.BackoffMaxHours { + return time.Duration(s.BackoffMaxHours) * time.Hour + } + } + if hours > s.BackoffMaxHours { + hours = s.BackoffMaxHours + } + return time.Duration(hours) * time.Hour +} + +func validate(in Settings) error { + switch { + case in.GraceHours < 1 || in.GraceHours > 720: + return fmt.Errorf("%w: grace_hours must be 1-720", ErrOutOfRange) + case in.BackoffBaseHours < 1 || in.BackoffBaseHours > 168: + return fmt.Errorf("%w: backoff_base_hours must be 1-168", ErrOutOfRange) + case in.BackoffMaxHours < 1 || in.BackoffMaxHours > 720: + return fmt.Errorf("%w: backoff_max_hours must be 1-720", ErrOutOfRange) + case in.BackoffMaxHours < in.BackoffBaseHours: + return fmt.Errorf("%w: backoff_max_hours must be >= backoff_base_hours", ErrOutOfRange) + case in.MaxAttempts < 1 || in.MaxAttempts > 10: + return fmt.Errorf("%w: max_attempts must be 1-10", ErrOutOfRange) + case in.MaxPerPass < 1 || in.MaxPerPass > 200: + return fmt.Errorf("%w: max_per_pass must be 1-200", ErrOutOfRange) + } + return nil +} + +func fromRow(row dbq.ReacquisitionSetting) Settings { + return Settings{ + Enabled: row.Enabled, + GraceHours: row.GraceHours, + BackoffBaseHours: row.BackoffBaseHours, + BackoffMaxHours: row.BackoffMaxHours, + MaxAttempts: row.MaxAttempts, + MaxPerPass: row.MaxPerPass, + AutoApprove: row.AutoApprove, + } +} diff --git a/internal/reacquisition/settings_test.go b/internal/reacquisition/settings_test.go new file mode 100644 index 00000000..6f7ed3e2 --- /dev/null +++ b/internal/reacquisition/settings_test.go @@ -0,0 +1,126 @@ +package reacquisition + +import ( + "errors" + "testing" + "time" +) + +func TestBackoffSchedule(t *testing.T) { + s := Defaults // 6h base, 168h (one week) cap, 3 attempts + + cases := []struct { + name string + attempts int32 + want time.Duration + }{ + // Never tried is always due — the grace window, not the backoff, is + // what holds the first attempt back. + {"never attempted", 0, 0}, + {"negative is treated as never", -1, 0}, + {"after one attempt", 1, 6 * time.Hour}, + {"after two", 2, 12 * time.Hour}, + {"after three", 3, 24 * time.Hour}, + {"after four", 4, 48 * time.Hour}, + {"after five", 5, 96 * time.Hour}, + // 6h * 2^5 = 192h, past the one-week cap. + {"clamped at the cap", 6, 168 * time.Hour}, + {"still clamped far out", 20, 168 * time.Hour}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := s.Backoff(c.attempts); got != c.want { + t.Errorf("Backoff(%d) = %v, want %v", c.attempts, got, c.want) + } + }) + } +} + +// The doubling must not be able to overflow int32 before the clamp is +// consulted — a large base with a high attempt count is the case that would +// wrap negative and make a spent album look due immediately. +func TestBackoffDoesNotOverflow(t *testing.T) { + s := Settings{BackoffBaseHours: 168, BackoffMaxHours: 720} + for attempts := int32(1); attempts <= 40; attempts++ { + got := s.Backoff(attempts) + if got <= 0 { + t.Fatalf("Backoff(%d) = %v, want a positive duration", attempts, got) + } + if got > 720*time.Hour { + t.Fatalf("Backoff(%d) = %v, want <= the 720h cap", attempts, got) + } + } +} + +func TestBackoffRespectsCustomSettings(t *testing.T) { + s := Settings{BackoffBaseHours: 1, BackoffMaxHours: 4} + for attempts, want := range map[int32]time.Duration{ + 1: 1 * time.Hour, + 2: 2 * time.Hour, + 3: 4 * time.Hour, + 4: 4 * time.Hour, // clamped + } { + if got := s.Backoff(attempts); got != want { + t.Errorf("Backoff(%d) = %v, want %v", attempts, got, want) + } + } +} + +func TestValidateRejectsWhatTheCheckWouldReject(t *testing.T) { + // Each case mirrors a CHECK in migration 0056. Validating in Go as well + // means the API answers 400 with a readable message instead of surfacing + // a constraint violation. + cases := []struct { + name string + in Settings + }{ + {"zero grace would fire on every transient unmount", + mutate(func(s *Settings) { s.GraceHours = 0 })}, + {"grace beyond a month", mutate(func(s *Settings) { s.GraceHours = 721 })}, + {"zero backoff base", mutate(func(s *Settings) { s.BackoffBaseHours = 0 })}, + {"backoff base beyond a week", mutate(func(s *Settings) { s.BackoffBaseHours = 169 })}, + {"zero backoff cap", mutate(func(s *Settings) { s.BackoffMaxHours = 0 })}, + {"cap below base is incoherent", mutate(func(s *Settings) { + s.BackoffBaseHours = 48 + s.BackoffMaxHours = 24 + })}, + {"zero attempts means never try", mutate(func(s *Settings) { s.MaxAttempts = 0 })}, + {"attempts beyond ten", mutate(func(s *Settings) { s.MaxAttempts = 11 })}, + {"zero per pass means never sweep", mutate(func(s *Settings) { s.MaxPerPass = 0 })}, + {"per pass beyond the cap", mutate(func(s *Settings) { s.MaxPerPass = 201 })}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if err := validate(c.in); !errors.Is(err, ErrOutOfRange) { + t.Errorf("validate() = %v, want ErrOutOfRange", err) + } + }) + } +} + +func TestValidateAcceptsDefaults(t *testing.T) { + if err := validate(Defaults); err != nil { + t.Fatalf("the shipped defaults must be valid, got %v", err) + } +} + +// Equal base and cap is legal — it is how an operator asks for a flat retry +// interval rather than an escalating one. +func TestValidateAcceptsFlatBackoff(t *testing.T) { + s := mutate(func(s *Settings) { + s.BackoffBaseHours = 12 + s.BackoffMaxHours = 12 + }) + if err := validate(s); err != nil { + t.Fatalf("flat backoff should be allowed, got %v", err) + } + if got := s.Backoff(5); got != 12*time.Hour { + t.Errorf("flat backoff gave %v, want 12h at every attempt", got) + } +} + +func mutate(f func(*Settings)) Settings { + s := Defaults + f(&s) + return s +} diff --git a/internal/reacquisition/sweeper.go b/internal/reacquisition/sweeper.go new file mode 100644 index 00000000..e14d1820 --- /dev/null +++ b/internal/reacquisition/sweeper.go @@ -0,0 +1,228 @@ +package reacquisition + +import ( + "context" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" +) + +// requestCreator is the slice of lidarrrequests.Service the sweeper needs, +// narrowed to an interface so the pass can be tested without a Lidarr client +// or an approval path that talks to one. +type requestCreator interface { + Create(ctx context.Context, userID pgtype.UUID, p lidarrrequests.CreateParams) (dbq.LidarrRequest, error) + Approve(ctx context.Context, requestID, adminID pgtype.UUID, ov lidarrrequests.ApproveOverrides) (dbq.LidarrRequest, error) +} + +// Sweeper periodically turns albums with long-missing files into Lidarr +// requests (milestone #290). +// +// Deliberately a periodic worker rather than a hook inside the scanner's +// reconcile pass. Reconcile runs inside a scan and has no business deciding +// to talk to a third-party service; it also re-runs often, which would make +// "attempt once, then back off" awkward to express. A worker paces itself, +// survives a restart, and retries without needing another scan. +type Sweeper struct { + pool *pgxpool.Pool + settings *SettingsService + requests requestCreator + logger *slog.Logger + tick time.Duration +} + +// NewSweeper constructs a Sweeper. The tick is deliberately coarse: the +// smallest meaningful backoff is measured in hours, so waking more often than +// hourly would only re-read settings and find nothing due. +func NewSweeper( + pool *pgxpool.Pool, + settings *SettingsService, + requests requestCreator, + logger *slog.Logger, +) *Sweeper { + return &Sweeper{ + pool: pool, + settings: settings, + requests: requests, + logger: logger, + tick: 1 * time.Hour, + } +} + +// Run drives the sweep loop until ctx is cancelled. +func (s *Sweeper) Run(ctx context.Context) { + t := time.NewTicker(s.tick) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + if err := s.SweepOnce(ctx); err != nil { + s.logger.Warn("reacquisition: sweep failed", "err", err) + } + } + } +} + +// PassResult reports what a single sweep did, for logging and tests. +type PassResult struct { + Cleared int64 // albums whose files came back; state dropped + Requested int // requests created this pass + Approved int // of those, sent on to Lidarr + GaveUp int // albums that spent their attempt budget + Unnameable int64 // albums with missing files but no MBID to ask for +} + +// SweepOnce runs one pass. Exported so the admin surface can offer a "run +// now" without waiting out the tick, and so tests drive it directly. +func (s *Sweeper) SweepOnce(ctx context.Context) error { + cfg := s.settings.Get() + if !cfg.Enabled { + return nil + } + q := dbq.New(s.pool) + + // Before selecting work: drop state for albums whose files came back, or + // were adopted at a new path (#2528). Doing this first means a recovered + // album cannot be picked in the same pass that would have retried it. + cleared, err := q.ClearRecoveredReacquisitions(ctx) + if err != nil { + return fmt.Errorf("clear recovered: %w", err) + } + res := PassResult{Cleared: cleared} + + // Counted, not acted on: an album MusicBrainz cannot name is not a + // failure to retry, it is a permanent gap the operator should see. + if n, cerr := q.CountAlbumsMissingWithoutMbid(ctx); cerr == nil { + res.Unnameable = n + } + + due, err := q.ListAlbumsDueReacquisition(ctx, dbq.ListAlbumsDueReacquisitionParams{ + GraceHours: cfg.GraceHours, + BackoffBaseHours: cfg.BackoffBaseHours, + BackoffMaxHours: cfg.BackoffMaxHours, + PageLimit: cfg.MaxPerPass, + }) + if err != nil { + return fmt.Errorf("list due: %w", err) + } + if len(due) == 0 { + s.logSummary(res) + return nil + } + + // One admin lookup per pass, not per album. A library with no admin at + // all cannot own a request, so the pass stops rather than half-working. + admin, err := q.GetOldestAdmin(ctx) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + s.logger.Warn("reacquisition: no admin account to own requests; skipping pass") + return nil + } + return fmt.Errorf("owner lookup: %w", err) + } + + for _, album := range due { + if err := s.attempt(ctx, q, cfg, admin.ID, album, &res); err != nil { + // One album's failure must not abandon the rest of the pass: a + // single unmatched MBID or a transient Lidarr error says nothing + // about the next album in the list. + s.logger.Warn("reacquisition: attempt failed", + "album", album.AlbumTitle, "err", err) + } + } + s.logSummary(res) + return nil +} + +// attempt creates (and optionally approves) the request for one album, then +// records the attempt against its backoff budget. +func (s *Sweeper) attempt( + ctx context.Context, + q *dbq.Queries, + cfg Settings, + adminID pgtype.UUID, + album dbq.ListAlbumsDueReacquisitionRow, + res *PassResult, +) error { + // The query already filters these out; belt and braces, because Create + // would reject the request and burn an attempt for no reason. + if album.AlbumMbid == nil || album.ArtistMbid == nil { + return nil + } + + req, err := s.requests.Create(ctx, adminID, lidarrrequests.CreateParams{ + Kind: "album", + LidarrArtistMBID: *album.ArtistMbid, + ArtistName: album.ArtistName, + LidarrAlbumMBID: *album.AlbumMbid, + AlbumTitle: album.AlbumTitle, + }) + if err != nil { + return fmt.Errorf("create request: %w", err) + } + res.Requested++ + + // Record the attempt even when Create deduped into somebody else's + // existing request: the point of the counter is "how often have we gone + // looking for this album", and a manual request in flight is a reason to + // wait rather than to keep asking. + row, err := q.RecordReacquisitionAttempt(ctx, dbq.RecordReacquisitionAttemptParams{ + AlbumID: album.AlbumID, + LastRequestID: req.ID, + }) + if err != nil { + return fmt.Errorf("record attempt: %w", err) + } + + if cfg.AutoApprove { + _, aerr := s.requests.Approve(ctx, req.ID, adminID, lidarrrequests.ApproveOverrides{}) + switch { + case aerr == nil: + res.Approved++ + case errors.Is(aerr, lidarrrequests.ErrLidarrDisabled): + // Leave it pending rather than treating it as a failure. The + // request is still the right record of intent, and it becomes + // actionable the moment Lidarr is configured. + s.logger.Info("reacquisition: request left pending, Lidarr disabled", + "album", album.AlbumTitle) + case errors.Is(aerr, lidarrrequests.ErrNotPending): + // Deduped onto a request somebody already approved. Nothing to do + // and nothing wrong. + default: + return fmt.Errorf("approve: %w", aerr) + } + } + + if row.Attempts >= cfg.MaxAttempts { + if err := q.MarkReacquisitionGaveUp(ctx, album.AlbumID); err != nil { + return fmt.Errorf("mark gave up: %w", err) + } + res.GaveUp++ + } + return nil +} + +func (s *Sweeper) logSummary(res PassResult) { + // Silence when a pass did nothing at all — this runs hourly forever, and + // an unconditional line would bury the passes that mattered. + if res.Requested == 0 && res.Cleared == 0 && res.GaveUp == 0 { + return + } + s.logger.Info("reacquisition: sweep", + "requested", res.Requested, + "approved", res.Approved, + "gave_up", res.GaveUp, + "cleared", res.Cleared, + "unnameable", res.Unnameable, + ) +}