fix: shared MB rate limiter prevents search/indexer 429 collisions
Previously, the MusicBrainzClient had no proactive rate limiter — it relied on the musicbrainzws2 library's retry-on-429 backoff. When the background indexer was resolving artist images (hitting MB at 1.5 req/sec) and a user search fired 3+ concurrent MB calls, the combined burst triggered 429s with cascading retries up to 60s. Now a single shared RateLimiter (1 req/sec) gates all MB API calls: - MusicBrainzClient search/lookup/browse methods - ArtistImageProvider fetchMBRels (was on a separate 1.5 req/sec limiter) The limiter serializes access proactively, preventing 429s entirely. The musicbrainzws2 retry logic remains as a safety net. Also split the old shared limiter into separate lbLimiter (for ListenBrainz + CoverArt) and mbLimiter (for MusicBrainz) so the two APIs don't block each other.
This commit is contained in:
@@ -35,12 +35,13 @@ type Service struct {
|
|||||||
// client, and ListenBrainz client internally.
|
// client, and ListenBrainz client internally.
|
||||||
func NewExploreService(logger *slog.Logger, db *database.DB) *Service {
|
func NewExploreService(logger *slog.Logger, db *database.DB) *Service {
|
||||||
cache := NewCache(db, logger.WithGroup("cache"))
|
cache := NewCache(db, logger.WithGroup("cache"))
|
||||||
limiter := NewRateLimiter()
|
lbLimiter := NewRateLimiter()
|
||||||
mb := NewMusicBrainzClient(cache, logger.WithGroup("musicbrainz"))
|
mbLimiter := NewRateLimiter() // 1 req/sec, shared across all MB consumers
|
||||||
lb := NewListenBrainzClient(limiter, cache, logger.WithGroup("listenbrainz"))
|
mb := NewMusicBrainzClient(cache, mbLimiter, logger.WithGroup("musicbrainz"))
|
||||||
artProxy := NewCoverArtProxy(db, limiter)
|
lb := NewListenBrainzClient(lbLimiter, cache, logger.WithGroup("listenbrainz"))
|
||||||
|
artProxy := NewCoverArtProxy(db, lbLimiter)
|
||||||
artistImg := NewArtistImageProvider(
|
artistImg := NewArtistImageProvider(
|
||||||
db, cache, NewRateLimiterF(1.5), logger.WithGroup("artist-image"),
|
db, cache, mbLimiter, logger.WithGroup("artist-image"),
|
||||||
)
|
)
|
||||||
index := NewSearchIndex(db, lb, artistImg, logger.WithGroup("search-index"))
|
index := NewSearchIndex(db, lb, artistImg, logger.WithGroup("search-index"))
|
||||||
libMBID := NewLibraryMBIDIndex(db)
|
libMBID := NewLibraryMBIDIndex(db)
|
||||||
|
|||||||
@@ -23,18 +23,22 @@ const (
|
|||||||
// response cache. Every API call checks the cache first and stores
|
// response cache. Every API call checks the cache first and stores
|
||||||
// successful responses for future hits.
|
// successful responses for future hits.
|
||||||
//
|
//
|
||||||
// The underlying musicbrainzws2.Client handles MusicBrainz-specific
|
// A proactive rate limiter gates all outgoing requests at 1 req/sec
|
||||||
// rate limiting via retries on HTTP 429, so we do not use the
|
// to avoid triggering MusicBrainz 429 responses. The underlying
|
||||||
// RateLimiter from this package (that is reserved for ListenBrainz).
|
// musicbrainzws2.Client still retries on 429 as a safety net, but
|
||||||
|
// the limiter should prevent most rate-limit hits.
|
||||||
type MusicBrainzClient struct {
|
type MusicBrainzClient struct {
|
||||||
mb *musicbrainzws2.Client
|
mb *musicbrainzws2.Client
|
||||||
cache *Cache
|
cache *Cache
|
||||||
logger *slog.Logger
|
limiter *RateLimiter
|
||||||
|
logger *slog.Logger
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewMusicBrainzClient creates a MusicBrainz API client that caches
|
// NewMusicBrainzClient creates a MusicBrainz API client that caches
|
||||||
// responses in the given Cache.
|
// responses in the given Cache. The provided rate limiter is shared
|
||||||
func NewMusicBrainzClient(cache *Cache, logger *slog.Logger) *MusicBrainzClient {
|
// with all other MB consumers (e.g. artist image resolution) to
|
||||||
|
// prevent concurrent bursts from triggering 429s.
|
||||||
|
func NewMusicBrainzClient(cache *Cache, limiter *RateLimiter, logger *slog.Logger) *MusicBrainzClient {
|
||||||
mb := musicbrainzws2.NewClient(musicbrainzws2.AppInfo{
|
mb := musicbrainzws2.NewClient(musicbrainzws2.AppInfo{
|
||||||
Name: "YellowJacket",
|
Name: "YellowJacket",
|
||||||
Version: "dev",
|
Version: "dev",
|
||||||
@@ -42,9 +46,10 @@ func NewMusicBrainzClient(cache *Cache, logger *slog.Logger) *MusicBrainzClient
|
|||||||
})
|
})
|
||||||
|
|
||||||
return &MusicBrainzClient{
|
return &MusicBrainzClient{
|
||||||
mb: mb,
|
mb: mb,
|
||||||
cache: cache,
|
cache: cache,
|
||||||
logger: logger,
|
limiter: limiter,
|
||||||
|
logger: logger,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -71,6 +76,10 @@ func (c *MusicBrainzClient) SearchArtists(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz search artists",
|
c.logger.Info("musicbrainz search artists",
|
||||||
"query", query,
|
"query", query,
|
||||||
"limit", limit,
|
"limit", limit,
|
||||||
@@ -105,6 +114,10 @@ func (c *MusicBrainzClient) SearchReleaseGroups(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz search release groups",
|
c.logger.Info("musicbrainz search release groups",
|
||||||
"query", query,
|
"query", query,
|
||||||
"limit", limit,
|
"limit", limit,
|
||||||
@@ -139,6 +152,10 @@ func (c *MusicBrainzClient) SearchRecordings(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz search recordings",
|
c.logger.Info("musicbrainz search recordings",
|
||||||
"query", query,
|
"query", query,
|
||||||
"limit", limit,
|
"limit", limit,
|
||||||
@@ -176,6 +193,10 @@ func (c *MusicBrainzClient) LookupArtist(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz lookup artist", "mbid", mbid)
|
c.logger.Info("musicbrainz lookup artist", "mbid", mbid)
|
||||||
|
|
||||||
a, err := c.mb.LookupArtist(ctx,
|
a, err := c.mb.LookupArtist(ctx,
|
||||||
@@ -206,6 +227,10 @@ func (c *MusicBrainzClient) LookupReleaseGroup(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz lookup release group", "mbid", mbid)
|
c.logger.Info("musicbrainz lookup release group", "mbid", mbid)
|
||||||
|
|
||||||
rg, err := c.mb.LookupReleaseGroup(ctx,
|
rg, err := c.mb.LookupReleaseGroup(ctx,
|
||||||
@@ -241,6 +266,10 @@ func (c *MusicBrainzClient) BrowseReleaseGroups(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz browse release groups",
|
c.logger.Info("musicbrainz browse release groups",
|
||||||
"artistMBID", artistMBID,
|
"artistMBID", artistMBID,
|
||||||
)
|
)
|
||||||
@@ -276,6 +305,10 @@ func (c *MusicBrainzClient) BrowseReleases(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if err := c.limiter.Wait(ctx); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
c.logger.Info("musicbrainz browse releases",
|
c.logger.Info("musicbrainz browse releases",
|
||||||
"releaseGroupMBID", releaseGroupMBID,
|
"releaseGroupMBID", releaseGroupMBID,
|
||||||
)
|
)
|
||||||
|
|||||||
+2
@@ -23,6 +23,8 @@ export function GetThumbnail(arg1:string,arg2:string,arg3:string):Promise<string
|
|||||||
|
|
||||||
export function GetThumbnails(arg1:Array<explore.ThumbnailRequest>):Promise<Record<string, string>>;
|
export function GetThumbnails(arg1:Array<explore.ThumbnailRequest>):Promise<Record<string, string>>;
|
||||||
|
|
||||||
|
export function IndexNewArtists():Promise<void>;
|
||||||
|
|
||||||
export function InvalidateIndexDiscographies():Promise<void>;
|
export function InvalidateIndexDiscographies():Promise<void>;
|
||||||
|
|
||||||
export function LookupArtist(arg1:string):Promise<explore.MBArtist>;
|
export function LookupArtist(arg1:string):Promise<explore.MBArtist>;
|
||||||
|
|||||||
@@ -42,6 +42,10 @@ export function GetThumbnails(arg1) {
|
|||||||
return window['go']['explore']['Service']['GetThumbnails'](arg1);
|
return window['go']['explore']['Service']['GetThumbnails'](arg1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function IndexNewArtists() {
|
||||||
|
return window['go']['explore']['Service']['IndexNewArtists']();
|
||||||
|
}
|
||||||
|
|
||||||
export function InvalidateIndexDiscographies() {
|
export function InvalidateIndexDiscographies() {
|
||||||
return window['go']['explore']['Service']['InvalidateIndexDiscographies']();
|
return window['go']['explore']['Service']['InvalidateIndexDiscographies']();
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user