internal/acquire/grab.go

@@ -6,6 +6,7 @@ import (
 	"path"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/jackc/pgx/v5/pgtype"
 
@@ -152,7 +153,25 @@ func (s *Service) Grab(ctx context.Context, want Want, chosen *Candidate, all []
 	}
 	add.SavePath = landingFor(s.Grabber.Landing, hash, chosen.Name)
 
-	if err := s.Grabber.Client.AddTorrent(ctx, add); err != nil && !errors.Is(err, downloader.ErrDuplicate) {
+	err := s.Grabber.Client.AddTorrent(ctx, add)
+	if errors.Is(err, downloader.ErrDuplicate) && s.heldElsewhere(ctx, hash) {
+		why := "Already in the download client outside Minstrel's category, where Minstrel cannot follow it (" +
+			strings.TrimPrefix(indexer.FirstLine(err.Error()), downloader.ErrDuplicate.Error()+": ") + ")"
+		if force {
+			out.Error = why + "."
+			return out
+		}
+		set(chosen, Blocked, Hard, CodeBlocked, why+".")
+		if s.Memory != nil && hash != "" {
+			_ = s.Memory.Block(ctx, dbq.BlockReleaseParams{
+				InfoHash: hash, Name: chosen.Name, Reason: BlockGone, Detail: why + ".", WantID: want.ID,
+				ExpiresAt: pgtype.Timestamptz{Time: time.Now().Add(GoneBlock), Valid: true},
+			})
+		}
+		out.Refused = true
+		return out
+	}
+	if err != nil && !errors.Is(err, downloader.ErrDuplicate) {
 		if errors.Is(err, downloader.ErrUnconfigured) {
 			out.Unconfigured, out.Missing = true, MissingClient
 		} else {
@@ -191,6 +210,19 @@ func (s *Service) Grab(ctx context.Context, want Want, chosen *Candidate, all []
 	return out
 }
 
+// heldElsewhere reports whether a release the client says it already holds is
+// one it does not list for Minstrel: held under another category, from before
+// Minstrel took the downloads over (#6176). Recorded as a grab, it would read
+// as gone half an hour later and be taken again, every pass. A client that
+// cannot answer now is given the benefit of the doubt, as the add was.
+func (s *Service) heldElsewhere(ctx context.Context, hash string) bool {
+	if hash == "" {
+		return false
+	}
+	listed, err := s.Grabber.Client.Torrents(ctx, indexer.NormalizeHash(hash))
+	return err == nil && len(listed) == 0
+}
+
 // judgeFiles judges a chosen release again on its .torrent's file list, and
 // sets it aside when the files say it is not the album, so no later search
 // offers it again. force still takes it.

internal/acquire/memory.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgtype"
@@ -24,8 +25,17 @@ const (
 	// BlockStalled: given up on because it would not finish (#5523); held
 	// back until the block's expires_at.
 	BlockStalled = "stalled"
+	// BlockGone: its download client would not let Minstrel follow it, held
+	// outside Minstrel's category or leaving the client twice (#6176); held
+	// back for GoneBlock.
+	BlockGone = "gone"
 )
 
+// GoneBlock is how long a release its client lost track of is held back:
+// long enough that a release which keeps vanishing is tried once a day rather
+// than on every pass, short enough that it is taken once the client lets go.
+const GoneBlock = 24 * time.Hour
+
 // Grab origins, matching grabs_origin_check.
 const (
 	OriginSchedule = "schedule"
@@ -106,6 +116,11 @@ func BlockSentence(reason, detail string) string {
 			return "Refused on import: " + detail
 		}
 		return "Refused on import: its files were not the album."
+	case BlockGone:
+		if detail != "" {
+			return "Held back for now: " + detail
+		}
+		return "Held back for now: its download client lost track of it."
 	case BlockStalled:
 		if detail != "" {
 			return "Given up on for now: " + detail

internal/acquire/search_test.go

@@ -3,6 +3,7 @@ package acquire
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -153,6 +154,8 @@ type fakeClient struct {
 	addErr   error
 	deleted  []string
 	progress float64
+	// elsewhere: the client holds it but does not list it for Minstrel.
+	elsewhere bool
 }
 
 func (c *fakeClient) AddTorrent(_ context.Context, a downloader.Add) error {
@@ -161,6 +164,9 @@ func (c *fakeClient) AddTorrent(_ context.Context, a downloader.Add) error {
 }
 
 func (c *fakeClient) Torrents(_ context.Context, hashes ...string) ([]downloader.Torrent, error) {
+	if c.elsewhere {
+		return nil, nil
+	}
 	return []downloader.Torrent{{Hash: hashes[0], Progress: c.progress}}, nil
 }
 
@@ -212,6 +218,28 @@ func TestGrab_AClientThatAlreadyHasItCounts(t *testing.T) {
 	}
 }
 
+// A duplicate the client does not list for Minstrel sits under another
+// category, where it would read as gone half an hour later (#6176). It is set
+// aside rather than taken; forced, it is refused with the client's reason.
+func TestGrab_ADuplicateHeldOutsideMinstrelIsNotTaken(t *testing.T) {
+	client := &fakeClient{addErr: fmt.Errorf("%w: already downloading Ohio Players (lidarr)", downloader.ErrDuplicate), elsewhere: true}
+	s := &Service{Grabber: &Downloader{Client: client, Landing: "/downloads"}}
+	c := Candidate{Verdict: Grab, InfoHash: "aaaa", Source: indexer.Result{Magnet: "magnet:?xt=urn:btih:aaaa"}}
+	took := s.Grab(context.Background(), ram, &c, nil, false, Origin{By: OriginSchedule})
+	if took.Added || !took.Refused {
+		t.Fatalf("took %+v", took)
+	}
+	if c.Verdict != Blocked || !strings.Contains(c.Reason, "(already downloading Ohio Players (lidarr))") {
+		t.Errorf("candidate %s: %q", c.Verdict, c.Reason)
+	}
+
+	forced := Candidate{Verdict: Grab, InfoHash: "aaaa", Source: indexer.Result{Magnet: "magnet:?xt=urn:btih:aaaa"}}
+	took = s.Grab(context.Background(), ram, &forced, nil, true, Origin{By: OriginOperator})
+	if took.Added || !strings.Contains(took.Error, "outside Minstrel's category") {
+		t.Errorf("forced: %+v", took)
+	}
+}
+
 func TestBetter_QualityThenSeedersThenYear(t *testing.T) {
 	flac := Candidate{Verdict: Grab, Tier: release.TierLossless, Seeders: 2}
 	mp3 := Candidate{Verdict: Grab, Tier: release.TierLossyHigh, Seeders: 90}

internal/db/dbq/lifecycle.sql.go

@@ -11,6 +11,25 @@ import (
 	"github.com/jackc/pgx/v5/pgtype"
 )
 
+const countGoneGrabs = `-- name: CountGoneGrabs :one
+SELECT count(*) FROM grabs
+ WHERE info_hash = $1 AND end_reason = 'gone' AND ended_at > $2
+`
+
+type CountGoneGrabsParams struct {
+	InfoHash string
+	Since    pgtype.Timestamptz
+}
+
+// How many of a release's grabs ended gone since a time: a release that keeps
+// leaving its client is held back rather than taken on every pass (#6176).
+func (q *Queries) CountGoneGrabs(ctx context.Context, arg CountGoneGrabsParams) (int64, error) {
+	row := q.db.QueryRow(ctx, countGoneGrabs, arg.InfoHash, arg.Since)
+	var count int64
+	err := row.Scan(&count)
+	return count, err
+}
+
 const countRemediation = `-- name: CountRemediation :one
 UPDATE download_interventions
    SET remediations = remediations + 1,

internal/db/migrations/0098_release_blocks_gone.down.sql

@@ -0,0 +1,4 @@
+DELETE FROM release_blocks WHERE reason = 'gone';
+ALTER TABLE release_blocks DROP CONSTRAINT release_blocks_reason_check;
+ALTER TABLE release_blocks ADD CONSTRAINT release_blocks_reason_check
+    CHECK (reason IN ('operator', 'files', 'import', 'stalled'));

internal/db/migrations/0098_release_blocks_gone.up.sql

@@ -0,0 +1,6 @@
+-- #6176: gone, a release its download client would not let Minstrel follow:
+-- held outside Minstrel's category, or leaving the client twice. Held back
+-- until expires_at, as stalled is.
+ALTER TABLE release_blocks DROP CONSTRAINT release_blocks_reason_check;
+ALTER TABLE release_blocks ADD CONSTRAINT release_blocks_reason_check
+    CHECK (reason IN ('operator', 'files', 'import', 'stalled', 'gone'));

internal/db/queries/lifecycle.sql

@@ -77,6 +77,12 @@ UPDATE grabs
  WHERE id = sqlc.arg(id) AND ended_at IS NULL AND import_state = ANY(sqlc.arg(states)::text[])
 RETURNING *;
 
+-- name: CountGoneGrabs :one
+-- How many of a release's grabs ended gone since a time: a release that keeps
+-- leaving its client is held back rather than taken on every pass (#6176).
+SELECT count(*) FROM grabs
+ WHERE info_hash = sqlc.arg(info_hash) AND end_reason = 'gone' AND ended_at > sqlc.arg(since);
+
 -- name: ListSeedingGrabs :many
 -- Imported downloads still sharing, oldest import first.
 SELECT id, client, info_hash, name, imported_at FROM grabs

internal/lifecycle/lifecycle.go

@@ -75,6 +75,8 @@ const (
 	// goneAfter: a client restarting can answer without a download for a
 	// while; half an hour missing is gone.
 	goneAfter = 30 * time.Minute
+	// goneWindow: a release gone twice within it is held back (#6176).
+	goneWindow = 7 * 24 * time.Hour
 )
 
 // running are the import states of a grab still downloading: the lifecycle
@@ -441,12 +443,35 @@ func (s *Service) giveUp(ctx context.Context, q *dbq.Queries, d downloader.Drive
 }
 
 // endGone ends a grab whose download left the client, and searches again.
-// The release is not held back: nothing was found wrong with it.
+// The first time, the release is not held back: nothing was found wrong with
+// it. A release that leaves its client again within goneWindow is held back for
+// acquire.GoneBlock (#6176): whatever keeps the client from listing it, taking
+// it again on every pass found 38 grabs of one release in a day.
 func (s *Service) endGone(ctx context.Context, q *dbq.Queries, g dbq.ListLiveGrabsRow, rep *Report) {
 	detail := "The download client no longer lists it: removed outside Minstrel. The album is searched for again."
+	before, err := q.CountGoneGrabs(ctx, dbq.CountGoneGrabsParams{
+		InfoHash: g.InfoHash, Since: pgtype.Timestamptz{Time: s.now().Add(-goneWindow), Valid: true}})
+	if err != nil {
+		s.logger.Warn("lifecycle: count earlier gone grabs", "grab", g.Name, "err", err)
+	}
+	again := before > 0
+	until := s.now().Add(acquire.GoneBlock)
+	if again {
+		detail = fmt.Sprintf("The download client no longer lists it, again: held back until %s; the album is searched for again.",
+			until.Format("2 Jan 15:04"))
+	}
 	if _, err := q.GiveUpGrab(ctx, dbq.GiveUpGrabParams{ID: g.ID, EndReason: EndGone, EndDetail: detail, States: running}); err != nil {
 		return
 	}
+	if again {
+		if err := acquire.NewMemory(s.pool).Block(ctx, dbq.BlockReleaseParams{
+			InfoHash: g.InfoHash, Name: g.Name, Reason: acquire.BlockGone,
+			Detail: "It left its download client twice without finishing.",
+			WantID: g.WantID, ExpiresAt: pgtype.Timestamptz{Time: until, Valid: true},
+		}); err != nil {
+			s.logger.Warn("lifecycle: hold the release back", "grab", g.Name, "err", err)
+		}
+	}
 	if g.WantID.Valid {
 		if err := q.SearchWantAgain(ctx, g.WantID); err != nil {
 			s.logger.Warn("lifecycle: search again", "grab", g.Name, "err", err)

internal/lifecycle/lifecycle_test.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"log/slog"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -277,6 +278,52 @@ func TestLifecycle_ADownloadGoneFromTheClientEndsAfterHalfAnHour_Integration(t *
 	}
 }
 
+// A release that leaves its client a second time is held back, whatever the
+// cause (#6176): one release was taken 40 times in a day, each ending gone.
+func TestLifecycle_AReleaseGoneTwiceIsHeldBack_Integration(t *testing.T) {
+	st := newStack(t, downloader.Torrent{Name: "Daft Punk - RAM [FLAC]", Progress: 0.3, State: "downloading", NumSeeds: 4})
+	ctx := context.Background()
+	delete(st.client.held, hash)
+	st.client.held[other] = downloader.Torrent{Hash: other, Name: "Someone else's", Progress: 1, State: "uploading"}
+
+	st.s.Pass(ctx)
+	st.clock = st.clock.Add(31 * time.Minute)
+	if rep := st.s.Pass(ctx); rep.Gone != 1 {
+		t.Fatalf("first gone: %+v", rep)
+	}
+	if blocks, _ := st.q.ListReleaseBlocks(ctx); len(blocks) != 0 {
+		t.Fatalf("held back after one gone: %+v", blocks)
+	}
+
+	again, err := acquire.NewMemory(st.s.pool).Grabbed(ctx, dbq.InsertGrabParams{WantID: st.want.ID, InfoHash: hash,
+		Name: "Daft Punk - RAM [FLAC]", Key: hash, Client: "qbittorrent", SavePath: "/downloads", Origin: acquire.OriginSchedule},
+		pgtype.UUID{}, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// first_seen is the database's clock and the test's runs ahead of it, so
+	// the second grab may end on either pass; count both.
+	gone := st.s.Pass(ctx).Gone
+	st.clock = st.clock.Add(31 * time.Minute)
+	if gone += st.s.Pass(ctx).Gone; gone != 1 {
+		t.Fatalf("second grab ended gone %d times, want 1", gone)
+	}
+	blocks, err := st.q.ListReleaseBlocks(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(blocks) != 1 || blocks[0].Reason != acquire.BlockGone || !blocks[0].ExpiresAt.Valid {
+		t.Fatalf("blocks = %+v, want one timed gone block", blocks)
+	}
+	g, err := st.q.GetGrab(ctx, again.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if g.EndReason != EndGone || !strings.Contains(g.EndDetail, "held back until") {
+		t.Errorf("second grab %q: %s", g.EndReason, g.EndDetail)
+	}
+}
+
 func TestLifecycle_AnEmptyListIsNotAMeasurement_Integration(t *testing.T) {
 	st := newStack(t, downloader.Torrent{Name: "Daft Punk - RAM [FLAC]", Progress: 0.3, State: "downloading"})
 	ctx := context.Background()

web/src/lib/api/types.ts

@@ -783,7 +783,7 @@ export type DownloadsPage = {
 export type ReleaseBlock = {
   info_hash: string;
   name: string;
-  reason: 'operator' | 'files' | 'import' | 'stalled';
+  reason: 'operator' | 'files' | 'import' | 'stalled' | 'gone';
   detail: string;
   blocked_at: string;
   // When a release given up on may be taken again; null is for good.

web/src/lib/components/wanted/BlockList.svelte

@@ -37,6 +37,7 @@
     if (b.reason === 'operator') return 'Set aside by you';
     if (b.reason === 'files') return 'Set aside for its files';
     if (b.reason === 'stalled') return 'Given up on: it would not finish';
+    if (b.reason === 'gone') return 'Held back: its download client lost track of it';
     return 'Refused on import';
   }