Files
minstrel/internal/gc/worker.go
T
bvandeusenandClaude Opus 5 4f9b083eec
test-go / test (push) Failing after 32s
test-go / integration (push) Successful in 4m50s
feat(discover): artist-tag cache for out-of-library candidates — #2376
Migration 0050 adds candidate_artist_tags + candidate_artist_tag_state:
folksonomy tags for artists NOT in the library, which track_tags cannot
hold because it's FK'd to tracks(id) and a Discover candidate has no local
row. Slice 6 ranks against these; this slice only fills the cache.

The reuse the task claimed is real and verified: MusicBrainz's
fetchEntityTags(ctx, "artist", mbid, scale) already existed for the #1519
recording→artist fallback, so FetchArtistTags is a thin wrapper. Two
subtleties it does NOT inherit:

  - Weight scale is 1.0, not artistTagWeightFactor (0.6). That discount
    exists because FetchTrackTags uses artist tags as a *proxy* for a
    track's; here the artist IS the subject. Applying it would make these
    weights incomparable with track_tags — exactly the comparison slice 6
    depends on. Pinned by a test.
  - fetchEntityTags reports existing-but-untagged as (empty, nil) so the
    track path can fall through. There's no next level here, so empty
    becomes the terminal ErrNotFound; otherwise the enricher would settle
    a candidate as "enriched" with zero tags.

ArtistTagProvider is the split TrackTagProvider's own doc comment
anticipated ("e.g. artist-level tags"). Last.fm gains artist.getTopTags,
which returns the same toptags envelope, so the response type and
normalizer are reused unchanged.

Rather than write the merge-and-classify loop twice, extracted it from
EnrichTrack into runChain(). The ErrNotFound-vs-transient split is the
load-bearing part — those lead to opposite persistence decisions — so it
now has direct unit tests it never had while inlined.

Bookkeeping is a separate table, not columns, because the "providers had
nothing" outcome must be recordable for a candidate with zero tag rows,
and there is no per-candidate row to hang columns off (
artist_similarity_unmatched holds many rows per candidate). Absence of a
state row means "never processed", so a transient failure writes nothing
and stays eligible.

Two capacity realities are designed for, not papered over:
  - The pool is O(library artists x neighbours) and MusicBrainz allows
    ~1 req/s, so it can never drain in one pass. The eligibility query
    returns candidates in descending summed-similarity order, so the ones
    that can actually reach a deck are enriched first.
  - candidateBatch (50) is smaller than the track batch (200): tracks are
    finite and drain to completion, candidates are effectively unbounded
    and would otherwise starve the track arm forever.

GC sweeps both tables — the similarity feed churns, and a candidate that
joins the library has its tags in track_tags now. Tags swept before state
so a mid-sweep crash leaves a valid state, not a re-fetch loop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-02 20:04:15 -04:00

115 lines
4.7 KiB
Go

// Package gc runs periodic garbage-collection / lifecycle sweeps
// against tables that have NO writer-side close path or NO retention
// policy. Each sweep addresses a drift finding from the 2026-06-02
// audit (Scribe parent #552) and is idempotent — re-running it on
// already-clean rows is a no-op.
//
// One Worker handles all sweeps so a single long-tick goroutine
// amortises the per-tick fixed cost. Each individual sweep is small
// (single UPDATE / DELETE with a time-bounded WHERE) and emits a
// log line with the affected-row count so the sweep cadence is
// visible in the application log without an explicit metrics layer.
//
// Sweeps:
// - GcCloseStalePlayEvents (#566)
// - GcClosePlaySessionsWithNoRecentEvents (#565)
// - GcExpireScrobbleQueueFailedRows (#567)
// - GcResetStuckSystemPlaylistRuns (#574)
// - GcDeleteExpiredPasswordResets (#575)
// - GcPruneDiagnostics (M9 — diagnostics 30d retention)
// - GcDeleteExpiredSuggestionSnoozes (#2374 — snoozes expire, then go)
// - GcDeleteOrphanedCandidateArtistTags(+State) (#2376 — the similarity
// feed churns, so cached candidate tags outlive their candidates)
package gc
import (
"context"
"log/slog"
"time"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// defaultTick is the production sweep cadence. 1 hour is generous
// since each sweep's WHERE clause uses a multi-hour staleness
// threshold; the worst-case delay between a row becoming sweepable
// and the worker noticing is bounded by tick + threshold.
const defaultTick = 1 * time.Hour
// Worker holds the pool + logger + tick interval. Construct with
// [NewWorker]; pass the returned Worker to a goroutine that calls
// [Worker.Run] with a context that's cancelled on shutdown.
type Worker struct {
pool *pgxpool.Pool
logger *slog.Logger
tick time.Duration
}
// NewWorker builds a Worker with the production tick (1h). Tests can
// reach into the Worker after construction to override `tick` for
// faster iteration.
func NewWorker(pool *pgxpool.Pool, logger *slog.Logger) *Worker {
return &Worker{pool: pool, logger: logger, tick: defaultTick}
}
// Run blocks until ctx is cancelled, running every sweep on each
// tick. Sweeps fire in fixed order; an error in one does NOT abort
// the rest (the panic-vs-just-failed distinction matters here — a
// pgx transient error from one query shouldn't prevent the others
// from running).
func (w *Worker) Run(ctx context.Context) {
// Fire once at start so a freshly-deployed server doesn't wait a
// full tick before doing the initial sweep. Matches the scrobble
// + similarity workers' "sweep then tick" pattern.
w.tickOnce(ctx)
t := time.NewTicker(w.tick)
defer t.Stop()
for {
select {
case <-ctx.Done():
return
case <-t.C:
w.tickOnce(ctx)
}
}
}
// tickOnce runs each sweep once, logging the affected-row count.
// Errors are logged per-sweep but do NOT abort the remaining ones —
// each sweep is independent.
func (w *Worker) tickOnce(ctx context.Context) {
q := dbq.New(w.pool)
w.runSweep(ctx, "close_stale_play_events", q.GcCloseStalePlayEvents)
w.runSweep(ctx, "close_play_sessions", q.GcClosePlaySessionsWithNoRecentEvents)
w.runSweep(ctx, "expire_scrobble_failed", q.GcExpireScrobbleQueueFailedRows)
w.runSweep(ctx, "reset_stuck_system_runs", q.GcResetStuckSystemPlaylistRuns)
w.runSweep(ctx, "delete_expired_password_resets", q.GcDeleteExpiredPasswordResets)
w.runSweep(ctx, "prune_diagnostics", q.GcPruneDiagnostics)
w.runSweep(ctx, "delete_expired_suggestion_snoozes", q.GcDeleteExpiredSuggestionSnoozes)
// Tags before state: if the process dies between the two, a candidate left
// with a state row and no tags simply reads as "settled, nothing found",
// which is already a valid state. The reverse order could leave tags with
// no state row, which the drainer would treat as never-processed and
// re-fetch on top of rows that are already there.
w.runSweep(ctx, "orphaned_candidate_artist_tags", q.GcDeleteOrphanedCandidateArtistTags)
w.runSweep(ctx, "orphaned_candidate_artist_tag_state", q.GcDeleteOrphanedCandidateArtistTagState)
}
// runSweep is a small adapter so each sweep call site is a one-liner
// in tickOnce. Logs at info on rows>0 and debug on rows=0 to keep
// the normal-case (nothing-to-do) noise out of operator logs.
func (w *Worker) runSweep(ctx context.Context, name string, fn func(context.Context) (int64, error)) {
rows, err := fn(ctx)
if err != nil {
w.logger.Error("gc sweep failed", "sweep", name, "err", err)
return
}
if rows > 0 {
w.logger.Info("gc sweep", "sweep", name, "rows_affected", rows)
} else {
w.logger.Debug("gc sweep", "sweep", name, "rows_affected", 0)
}
}