Files
yellowjacket/backend/explore/ratelimiter_test.go
T
yonluandClaude Opus 5 20fbf28f2a perf(explore): make the owned-artist backfill yield, mark, and stop
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
2026-08-14 13:33:54 -04:00

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)
}
}