The post-scan backfills share MusicBrainz's rate limiters with every page the user can open, and both were FIFO — so a thousand-artist enrichment put an album page behind an hour of queued work. WithBackgroundLane/WithBackgroundPriority add a slower second lane: a marked wait takes no token while any interactive wait is outstanding. It is a context marker rather than a parameter because a backfill calls the same client methods a detail page does. A long backfill also has to be visible and stoppable, so jobs.KindCatalogEnrich registers both with progress and cancel — after the work is counted, since these passes are a no-op on every launch once the library is covered. What it does not fetch is the point. It ran for hours against a 900-artist library and marked nothing, because three of the four things it did per artist were work nobody asked for: similar artists, which the artist page already resolves on view, and a full GetArtistImage (fanart.tv, TheAudioDB, Wikidata, Wikipedia, ten portraits) reached only to warm the MB artist lookup EnsureArtistRels does alone. It was also serial across artists while every limiter is per-host and idle. The marks are a table rather than more explore_index columns, because artifactimport merges by column list and a flag added there is a second place to remember. BrowseReleaseGroupsAll pages to exhaustion, where the old call silently cut a prolific artist at 100 release groups. One portrait is downloaded now; the rest are remembered as URLs. resolveAllSources downloaded every candidate, up to ten, full size, while nothing reads anything but primary.jpg — 5.3 GB measured on a real cache, 4.1 GB of it unreachable. OrphanedArtistImagesJob is why that survived: it joined the bare MBID onto the images directory, but artist directories are sharded under a two-character prefix, so it named a path that never existed and deleted the rows that were the only record of the files it left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDCbcCZQepnpSQYJ6SxxZm
161 lines
4.0 KiB
Go
161 lines
4.0 KiB
Go
package explore
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func TestRateLimiterBurst(t *testing.T) {
|
|
rl := NewRateLimiter()
|
|
ctx := context.Background()
|
|
|
|
const n = 5
|
|
|
|
start := time.Now()
|
|
|
|
for i := range n {
|
|
if err := rl.Wait(ctx); err != nil {
|
|
t.Fatalf("Wait %d: %v", i, err)
|
|
}
|
|
}
|
|
|
|
elapsed := time.Since(start)
|
|
|
|
// First request is immediate; 4 more at 1/sec = ≥4s total.
|
|
if elapsed < 4*time.Second {
|
|
t.Errorf(
|
|
"elapsed %v, want ≥ 4s (rate limiter too fast)", elapsed,
|
|
)
|
|
}
|
|
|
|
// Generous upper bound to avoid CI flakes.
|
|
if elapsed > 7*time.Second {
|
|
t.Errorf(
|
|
"elapsed %v, want ≤ 7s (rate limiter too slow)", elapsed,
|
|
)
|
|
}
|
|
}
|
|
|
|
// TestRateLimiterBackgroundYields is the property the whole priority
|
|
// lane exists for: an interactive caller arriving while a backfill is
|
|
// running is not queued behind the rest of the backfill.
|
|
func TestRateLimiterBackgroundYields(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Fast rates: this asserts ordering, not pacing.
|
|
rl := NewRateLimiterBurst(50, 1).WithBackgroundLane(50)
|
|
bg := WithBackgroundPriority(context.Background())
|
|
|
|
// Hold the gate open for the length of the test by keeping one
|
|
// interactive wait outstanding.
|
|
rl.enterInteractive()
|
|
|
|
done := make(chan struct{})
|
|
|
|
go func() {
|
|
defer close(done)
|
|
|
|
_ = rl.Wait(bg)
|
|
}()
|
|
|
|
select {
|
|
case <-done:
|
|
t.Fatal("background Wait proceeded while an interactive wait was outstanding")
|
|
case <-time.After(100 * time.Millisecond):
|
|
}
|
|
|
|
rl.exitInteractive()
|
|
|
|
select {
|
|
case <-done:
|
|
case <-time.After(2 * time.Second):
|
|
t.Fatal("background Wait did not proceed after the interactive wait cleared")
|
|
}
|
|
}
|
|
|
|
// TestRateLimiterBackgroundLanePaces checks the second half: background
|
|
// callers are held to their own slower rate even with the gate clear.
|
|
func TestRateLimiterBackgroundLanePaces(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Interactive lane is effectively unlimited; the background lane is
|
|
// the only thing that can slow this down.
|
|
rl := NewRateLimiterBurst(1000, 1000).WithBackgroundLane(4)
|
|
bg := WithBackgroundPriority(context.Background())
|
|
|
|
start := time.Now()
|
|
|
|
for i := range 3 {
|
|
if err := rl.Wait(bg); err != nil {
|
|
t.Fatalf("Wait %d: %v", i, err)
|
|
}
|
|
}
|
|
|
|
// Burst of 1, then two more at 4/sec = ≥500ms.
|
|
if elapsed := time.Since(start); elapsed < 400*time.Millisecond {
|
|
t.Errorf("elapsed %v, want ≥ 400ms (background lane not pacing)", elapsed)
|
|
}
|
|
}
|
|
|
|
// TestRateLimiterBackgroundCancels guards the loop in waitBackground:
|
|
// a background caller blocked behind a permanently busy interactive
|
|
// lane must still honour its context.
|
|
func TestRateLimiterBackgroundCancels(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
rl := NewRateLimiter().WithBackgroundLane(1)
|
|
rl.enterInteractive() // never cleared
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
|
defer cancel()
|
|
|
|
err := rl.Wait(WithBackgroundPriority(ctx))
|
|
if err == nil {
|
|
t.Fatal("expected error from cancelled context, got nil")
|
|
}
|
|
|
|
if !errors.Is(err, context.DeadlineExceeded) {
|
|
t.Errorf("error = %v, want context.DeadlineExceeded", err)
|
|
}
|
|
}
|
|
|
|
// TestRateLimiterInteractiveUnmarked confirms an unmarked context is
|
|
// interactive — the default has to be the safe one, since every
|
|
// existing call site is unmarked.
|
|
func TestRateLimiterInteractiveUnmarked(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
if isBackgroundPriority(context.Background()) {
|
|
t.Error("an unmarked context reported as background priority")
|
|
}
|
|
|
|
if !isBackgroundPriority(WithBackgroundPriority(context.Background())) {
|
|
t.Error("a marked context did not report as background priority")
|
|
}
|
|
}
|
|
|
|
func TestRateLimiterContextCancel(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
rl := NewRateLimiter()
|
|
|
|
// Drain the initial token so the next Wait must block.
|
|
if err := rl.Wait(context.Background()); err != nil {
|
|
t.Fatalf("drain token: %v", err)
|
|
}
|
|
|
|
ctx, cancel := context.WithCancel(context.Background())
|
|
cancel() // cancel immediately
|
|
|
|
err := rl.Wait(ctx)
|
|
if err == nil {
|
|
t.Fatal("expected error from cancelled context, got nil")
|
|
}
|
|
|
|
if !errors.Is(err, context.Canceled) {
|
|
t.Errorf("error = %v, want context.Canceled", err)
|
|
}
|
|
}
|