fix(crossseed): queue completion searches and retry rate-limit waits

[s0up4200]

Fixes completion-triggered cross-seed bursts by processing completion searches one-at-a-time per instance and automatically retrying when Torznab/Prowlarr returns rate-limit wait errors.

Summary by CodeRabbit

• Bug Fixes

  • Completion searches now retry on rate-limit conditions and are serialized per instance to prevent concurrent conflicts and lost attempts.
  • Scheduler now surfaces rate-limit-wait outcomes when all indexers are skipped.

• Improvements

  • Added per-instance coordination and retry/backoff handling plus metrics to better track rate-limit waits and retries.

• Tests

  • New tests cover per-instance queuing, retry behavior on rate limits, retry-delay parsing, and scheduler behavior when all indexers are skipped.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds per-instance completion lanes and retryable, rate-limit-aware completion searches to cross-seed, with an injectable completionSearchInvoker; jackett adds per-indexer completion coordination via a waitgroup and surfaces all-indexers-rate-limit-skip errors. New tests cover queuing, retries, and scheduler rate-limit behavior.

Changes

Cohort / File(s)	Summary
Cross-seed Core internal/services/crossseed/service.go	Add completionLane type, completionLanes map + mutex, getCompletionLane, executeCompletionSearchWithRetry, invokeCompletionSearch, completionRetryDelay, completionSearchInvoker hook, rate-limit token list and retry constants; replace direct completion call with lane-guarded, retry-enabled flow in HandleTorrentCompletion.
Cross-seed Tests (formatting) internal/services/crossseed/service_completion_gazelle_test.go	Minor SQL indentation/formatting changes only; no behavioral changes.
Cross-seed Queue Tests internal/services/crossseed/service_completion_queue_test.go	New tests for per-instance queuing order, retry-on-rate-limit behavior, and fallback rate-limit message parsing; includes in-memory SQLite test setup and mocked completionSearchInvoker.
Jackett Core internal/services/jackett/service.go	Add completionWG waitgroup to coordinate per-indexer OnComplete callbacks, track waitSkips/lastWaitErr, wait before finalizing, and propagate a rate-limit-wait error when all indexers are skipped.
Jackett Tests internal/services/jackett/service_test.go	New test verifying scheduler behavior when all indexers are skipped due to rate limiting and ensuring the final callback returns an appropriate error.
Module go.mod	Small module metadata update accompanying service changes.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CrossSeed as Cross-seed Service
    participant Lane as Completion Lane
    participant Search as Completion Search

    Client->>CrossSeed: HandleTorrentCompletion(torrent)
    CrossSeed->>Lane: Lock per-instance lane
    rect rgba(100,150,200,0.5)
        Note over CrossSeed,Search: Retry loop up to maxCompletionSearchAttempts
        CrossSeed->>Search: invokeCompletionSearch(...)
        alt Rate-limit detected
            Search-->>CrossSeed: RateLimitWaitError / token-match
            CrossSeed->>CrossSeed: completionRetryDelay(err) -> sleep
            CrossSeed->>Search: Retry invokeCompletionSearch(...)
        else Success
            Search-->>CrossSeed: Results
        end
    end
    CrossSeed->>Lane: Unlock per-instance lane
    CrossSeed-->>Client: Complete

Loading

sequenceDiagram
    participant Scheduler
    participant Jackett as Jackett Service
    participant IndexerA
    participant IndexerB
    participant ResultCB as Result Callback

    Scheduler->>Jackett: searchIndexersWithScheduler()
    par Per-indexer execution
        IndexerA->>Jackett: OnComplete(callback)
        Jackett->>Jackett: completionWG.Done()
    and
        IndexerB->>Jackett: OnComplete(callback)
        Jackett->>Jackett: completionWG.Done()
    end
    Scheduler->>Jackett: OnJobDone()
    Jackett->>Jackett: completionWG.Wait()
    rect rgba(200,100,100,0.5)
        alt All indexers skipped due to rate-limit
            Jackett->>ResultCB: Return rate-limit wait error
        else
            Jackett->>ResultCB: Return aggregated results & coverage
        end
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix(qbittorrent): stop reboot torrent_completed spam #1515: Modifies HandleTorrentCompletion in internal/services/crossseed/service.go — closely related changes to completion invocation semantics.
• feat(torrents): add cross-seeding and search #553: Prior cross-seed completion/search changes that this PR extends with per-instance sequencing and retry behavior.
• fix(crossseed): fix batch completion searches and remove legacy settings #744: Related cross-seed completion handling and per-instance queuing/retry logic.

Suggested labels

cross-seed, bugfix, tests

Suggested reviewers

• Barcode-eng
• Audionut

Poem

🐰 I hop in lanes, one tunnel at a time,
Retries keep nimble when servers decline,
Waitgroups gather each indexer's call,
Rate limits may pause, but we handle them all,
Hooray — orderly cross-seeds for all!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main changes: queuing completion searches and retrying on rate-limit waits, which directly aligns with the PR's primary objectives and the substantial changes made to the crossseed service.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/crossseed-completion-queue-retry

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/services/crossseed/service_completion_queue_test.go (1)

136-165: Consider atomic access for attempts to ensure -race safety.

The attempts variable is captured and mutated by the invoker closure. While the retry logic is likely synchronous, using atomic operations would make the test robust under -race regardless of future implementation changes and aligns with the coding guideline to run tests with -race.

Also, the timing assertion on line 165 (>= 8ms for a 10ms wait) provides reasonable tolerance but could be flaky on slow CI systems.

♻️ Suggested improvement using atomic

+import "sync/atomic"
+
 func TestHandleTorrentCompletion_RetriesOnRateLimitError(t *testing.T) {
 	completionStore := setupCompletionStoreForQueueTests(t)
 
-	attempts := 0
+	var attempts atomic.Int32
 	svc := &Service{
 		completionStore: completionStore,
 		automationSettingsLoader: func(context.Context) (*models.CrossSeedAutomationSettings, error) {
 			return models.DefaultCrossSeedAutomationSettings(), nil
 		},
 		completionSearchInvoker: func(context.Context, int, *qbt.Torrent, *models.CrossSeedAutomationSettings, *models.InstanceCrossSeedCompletionSettings) error {
-			attempts++
-			if attempts == 1 {
+			n := attempts.Add(1)
+			if n == 1 {
 				return &jackett.RateLimitWaitError{
 					IndexerID:   1,
 					IndexerName: "test",
 					Wait:        10 * time.Millisecond,
 					MaxWait:     30 * time.Second,
 					Priority:    jackett.RateLimitPriorityCompletion,
 				}
 			}
 			return nil
 		},
 	}
 
 	started := time.Now()
 	svc.HandleTorrentCompletion(context.Background(), 1, qbt.Torrent{
 		Hash:     "cccccccccccccccccccccccccccccccccccccccc",
 		Name:     "retry-me",
 		Progress: 1.0,
 	})
 
-	assert.Equal(t, 2, attempts)
+	assert.Equal(t, int32(2), attempts.Load())
 	assert.GreaterOrEqual(t, time.Since(started), 8*time.Millisecond)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/services/crossseed/service_completion_queue_test.go` around lines
136 - 165, Replace the non-atomic attempts counter with an atomic integer to be
race-safe: change the test's attempts variable to an atomic-backed type and
increment/read it inside the completionSearchInvoker closure using atomic
operations (e.g., atomic.AddInt32/atomic.LoadInt32) and update the assertions to
use the atomic-loaded value when checking attempts; also relax or remove the
strict timing assertion (currently comparing time.Since(started) to 8ms) — lower
the threshold (e.g., 5ms) or remove the timing check to avoid CI flakiness.
Ensure changes touch the attempts variable, the completionSearchInvoker closure,
and the assertions that reference attempts and timing, while leaving
Service.HandleTorrentCompletion usage unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/services/crossseed/service.go`:
- Around line 244-245: The fallback rate-limit token list
completionRateLimitTokens is missing the strings emitted by
wrapCrossSeedSearchError ("rate-limited" and "cooldown"), so non-typed
rate-limit errors bypass retries; update completionRateLimitTokens to include
"rate-limited" and "cooldown" (and ensure matching remains consistent, e.g.,
case-insensitive where used) so wrapCrossSeedSearchError-produced messages are
caught by the retry fallback.

---

Nitpick comments:
In `@internal/services/crossseed/service_completion_queue_test.go`:
- Around line 136-165: Replace the non-atomic attempts counter with an atomic
integer to be race-safe: change the test's attempts variable to an atomic-backed
type and increment/read it inside the completionSearchInvoker closure using
atomic operations (e.g., atomic.AddInt32/atomic.LoadInt32) and update the
assertions to use the atomic-loaded value when checking attempts; also relax or
remove the strict timing assertion (currently comparing time.Since(started) to
8ms) — lower the threshold (e.g., 5ms) or remove the timing check to avoid CI
flakiness. Ensure changes touch the attempts variable, the
completionSearchInvoker closure, and the assertions that reference attempts and
timing, while leaving Service.HandleTorrentCompletion usage unchanged.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 579ea8d and 8eb5fd4.

📒 Files selected for processing (5)

• internal/services/crossseed/service.go
• internal/services/crossseed/service_completion_gazelle_test.go
• internal/services/crossseed/service_completion_queue_test.go
• internal/services/jackett/service.go
• internal/services/jackett/service_test.go

[coderabbitai]

🧹 Nitpick comments (1)

internal/services/crossseed/service.go (1)

338-342: Consider lifecycle cleanup for completionLanes.

completionLanes grows monotonically by instance ID. It’s likely small in practice, but adding cleanup when instances are removed/disabled would prevent stale lane accumulation over long runtimes.

Also applies to: 1473-1487

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/services/crossseed/service.go` around lines 338 - 342,
completionLanes (protected by completionLaneMu) is never pruned and can
accumulate stale entries; add lifecycle cleanup to remove a lane when its
instance is removed/disabled. Implement a thread-safe
RemoveCompletionLane(instanceID int) (or integrate into existing
instance/remove/disable handlers) that locks completionLaneMu, closes or signals
the associated *completionLane to stop any goroutines, deletes
completionLanes[instanceID], and ensures any future Get-or-create logic (where
completionLanes is accessed) can recreate a fresh lane. Also audit and add
similar cleanup at the other referenced area (around the code at lines
~1473-1487) to ensure symmetric removal on instance teardown.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/services/crossseed/service.go`:
- Around line 338-342: completionLanes (protected by completionLaneMu) is never
pruned and can accumulate stale entries; add lifecycle cleanup to remove a lane
when its instance is removed/disabled. Implement a thread-safe
RemoveCompletionLane(instanceID int) (or integrate into existing
instance/remove/disable handlers) that locks completionLaneMu, closes or signals
the associated *completionLane to stop any goroutines, deletes
completionLanes[instanceID], and ensures any future Get-or-create logic (where
completionLanes is accessed) can recreate a fresh lane. Also audit and add
similar cleanup at the other referenced area (around the code at lines
~1473-1487) to ensure symmetric removal on instance teardown.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb5fd4 and e4ca59b.

📒 Files selected for processing (2)

• internal/services/crossseed/service.go
• internal/services/crossseed/service_completion_queue_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/services/crossseed/service_completion_queue_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/services/crossseed/service.go`:
- Around line 1458-1463: Currently the code holds the per-instance lane mutex
(getCompletionLane(...).mu) across the long-running call to
executeCompletionSearchWithRetry, which can cause permanent stalls; change the
flow to only hold lane.mu to read/update short-lived lane state (e.g., mark lane
as in-progress or increment a counter) then release the mutex before calling
executeCompletionSearchWithRetry, and finally reacquire the mutex to clear the
in-progress marker and perform any cleanup; update references to
getCompletionLane, lane.mu.Lock()/Unlock(), and executeCompletionSearchWithRetry
to implement this acquire-mark-release -> long-running call -> reacquire-clear
pattern so the lane mutex is never held while the search executes.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4ca59b and e151710.

📒 Files selected for processing (2)

• internal/services/crossseed/service.go
• internal/services/crossseed/service_completion_gazelle_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/services/crossseed/service_completion_gazelle_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/services/crossseed/service_completion_queue_test.go`:
- Line 96: The file has a gofmt violation around the anonymous goroutine start
"go func() {"—run gofmt (gofmt -w) on the test file or reformat the block so the
anonymous goroutine and surrounding lines conform to gofmt rules (proper
indentation, spacing and tabs), then re-run go vet/lint; ensure the "go func()
{" block is correctly formatted and the file is gofmt-clean.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1419c31 and 99d7433.

📒 Files selected for processing (1)

• internal/services/crossseed/service_completion_queue_test.go