mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
perf(artwork): fetch only IDs for backfill enumeration
Backfill enumerated every album, artist, playlist and radio via GetAll and mapped out just the ID. GetAll materializes full entities (library joins, participant/stats/tags JSON, annotation, artwork hydration), so on a large library it loaded tens of thousands of heavy structs only to read one field each — spiking transient RSS to ~1GB during the one-time upgrade backfill, a memory risk on small NAS/Pi hardware. Add GetAllIDs to the album, artist, playlist and radio repositories: it reuses each repo's base row-set filter (library visibility, artist content join, playlist userFilter) but projects only id, skipping the heavy columns and post-processing. A per-repo parity test asserts GetAllIDs returns exactly the same id set as GetAll. Verified on a 727MB / 29k-artist production DB copy: peak RSS during backfill dropped from ~1012MB to ~89MB, file descriptors flat, same 36,138 items enqueued.
This commit is contained in:
parent
b172ce4296
commit
9ce51cf575
@ -12,7 +12,6 @@ import (
|
||||
"github.com/navidrome/navidrome/core/auth"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
||||
// FingerprintPropertyKey is the model.PropertyRepository key Backfill compares against
|
||||
@ -54,22 +53,10 @@ func Backfill(ctx context.Context, ds model.DataStore) (bool, error) {
|
||||
kind string
|
||||
fetch func() ([]string, error)
|
||||
}{
|
||||
{"ar", func() ([]string, error) {
|
||||
as, err := ds.Artist(ctx).GetAll()
|
||||
return slice.Map(as, func(a model.Artist) string { return a.ID }), err
|
||||
}},
|
||||
{"al", func() ([]string, error) {
|
||||
as, err := ds.Album(ctx).GetAll()
|
||||
return slice.Map(as, func(a model.Album) string { return a.ID }), err
|
||||
}},
|
||||
{"pl", func() ([]string, error) {
|
||||
ps, err := ds.Playlist(ctx).GetAll()
|
||||
return slice.Map(ps, func(p model.Playlist) string { return p.ID }), err
|
||||
}},
|
||||
{"ra", func() ([]string, error) {
|
||||
rs, err := ds.Radio(ctx).GetAll()
|
||||
return slice.Map(rs, func(r model.Radio) string { return r.ID }), err
|
||||
}},
|
||||
{"ar", func() ([]string, error) { return ds.Artist(ctx).GetAllIDs() }},
|
||||
{"al", func() ([]string, error) { return ds.Album(ctx).GetAllIDs() }},
|
||||
{"pl", func() ([]string, error) { return ds.Playlist(ctx).GetAllIDs() }},
|
||||
{"ra", func() ([]string, error) { return ds.Radio(ctx).GetAllIDs() }},
|
||||
}
|
||||
for _, k := range kinds {
|
||||
ids, err := k.fetch()
|
||||
|
||||
@ -142,6 +142,7 @@ type AlbumRepository interface {
|
||||
UpdateExternalInfo(*Album) error
|
||||
Get(id string) (*Album, error)
|
||||
GetAll(...QueryOptions) (Albums, error)
|
||||
GetAllIDs(...QueryOptions) ([]string, error)
|
||||
GetCursor(...QueryOptions) (AlbumCursor, error)
|
||||
GetYears(libraryIDs ...int) ([]int, error)
|
||||
|
||||
|
||||
@ -89,6 +89,7 @@ type ArtistRepository interface {
|
||||
UpdateExternalInfo(a *Artist) error
|
||||
Get(id string) (*Artist, error)
|
||||
GetAll(options ...QueryOptions) (Artists, error)
|
||||
GetAllIDs(options ...QueryOptions) ([]string, error)
|
||||
GetCursor(options ...QueryOptions) (ArtistCursor, error)
|
||||
GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error)
|
||||
|
||||
|
||||
@ -143,6 +143,7 @@ type PlaylistRepository interface {
|
||||
Get(id string) (*Playlist, error)
|
||||
GetWithTracks(id string, refreshSmartPlaylist, includeMissing bool) (*Playlist, error)
|
||||
GetAll(options ...QueryOptions) (Playlists, error)
|
||||
GetAllIDs(options ...QueryOptions) ([]string, error)
|
||||
GetCursor(options ...QueryOptions) (PlaylistCursor, error)
|
||||
FindByPath(path string) (*Playlist, error)
|
||||
Delete(id string) error
|
||||
|
||||
@ -32,5 +32,6 @@ type RadioRepository interface {
|
||||
Delete(id string) error
|
||||
Get(id string) (*Radio, error)
|
||||
GetAll(options ...QueryOptions) (Radios, error)
|
||||
GetAllIDs(options ...QueryOptions) ([]string, error)
|
||||
Put(u *Radio, colsToUpdate ...string) error
|
||||
}
|
||||
|
||||
@ -254,6 +254,15 @@ func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, e
|
||||
return res.toModels(), nil
|
||||
}
|
||||
|
||||
// GetAllIDs returns just the album IDs for the same row set as GetAll, skipping the
|
||||
// heavy column projection and JSON post-processing. Used by bulk enumeration (artwork backfill).
|
||||
func (r *albumRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("album.id"))
|
||||
ids := []string{}
|
||||
err := r.queryAllSlice(sq, &ids)
|
||||
return ids, err
|
||||
}
|
||||
|
||||
func (r *albumRepository) GetCursor(options ...model.QueryOptions) (model.AlbumCursor, error) {
|
||||
sq := r.selectAlbum(options...)
|
||||
cursor, err := queryWithStableResults[dbAlbum](r.sqlRepository, sq)
|
||||
|
||||
@ -84,6 +84,21 @@ var _ = Describe("AlbumRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetAllIDs", func() {
|
||||
It("returns the same id set as GetAll", func() {
|
||||
want, err := albumRepo.GetAll()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
wantIDs := make([]string, 0, len(want))
|
||||
for _, a := range want {
|
||||
wantIDs = append(wantIDs, a.ID)
|
||||
}
|
||||
ids, err := albumRepo.GetAllIDs()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(ids).To(ConsistOf(wantIDs))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetAll", func() {
|
||||
var GetAll = func(opts ...model.QueryOptions) (model.Albums, error) {
|
||||
albums, err := albumRepo.GetAll(opts...)
|
||||
|
||||
@ -264,6 +264,15 @@ func (r *artistRepository) GetAll(options ...model.QueryOptions) (model.Artists,
|
||||
return res, err
|
||||
}
|
||||
|
||||
// GetAllIDs returns just the artist IDs for the same row set as GetAll, skipping the
|
||||
// heavy stats/annotation columns and JSON post-processing. Used by bulk enumeration (artwork backfill).
|
||||
func (r *artistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
sq := r.applyLibraryFilterToArtistQuery(r.newSelect(options...).Columns("artist.id")).GroupBy("artist.id")
|
||||
ids := []string{}
|
||||
err := r.queryAllSlice(sq, &ids)
|
||||
return ids, err
|
||||
}
|
||||
|
||||
func (r *artistRepository) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
|
||||
sel := r.selectArtist(options...)
|
||||
cursor, err := queryWithStableResults[dbArtist](r.sqlRepository, sel)
|
||||
|
||||
@ -284,6 +284,21 @@ var _ = Describe("ArtistRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetAllIDs", func() {
|
||||
It("returns the same id set as GetAll", func() {
|
||||
want, err := repo.GetAll()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
wantIDs := make([]string, 0, len(want))
|
||||
for _, a := range want {
|
||||
wantIDs = append(wantIDs, a.ID)
|
||||
}
|
||||
ids, err := repo.GetAllIDs()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(ids).To(ConsistOf(wantIDs))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Basic Operations", func() {
|
||||
Describe("Count", func() {
|
||||
It("returns the number of artists in the DB", func() {
|
||||
|
||||
@ -189,6 +189,16 @@ func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playli
|
||||
return playlists, err
|
||||
}
|
||||
|
||||
// GetAllIDs returns just the playlist IDs for the same row set as GetAll (honoring userFilter),
|
||||
// skipping the owner-name join columns and annotation. Used by bulk enumeration (artwork backfill).
|
||||
func (r *playlistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
sq := r.newSelect(options...).Columns("playlist.id").
|
||||
Join("user on user.id = owner_id").Where(r.userFilter())
|
||||
ids := []string{}
|
||||
err := r.queryAllSlice(sq, &ids)
|
||||
return ids, err
|
||||
}
|
||||
|
||||
func (r *playlistRepository) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
|
||||
// Same userFilter as GetAll: a cursor must not widen visibility beyond public/owned playlists.
|
||||
sel := r.selectPlaylist(options...).Where(r.userFilter())
|
||||
|
||||
@ -37,6 +37,21 @@ var _ = Describe("PlaylistRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetAllIDs", func() {
|
||||
It("returns the same id set as GetAll", func() {
|
||||
want, err := repo.GetAll()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
wantIDs := make([]string, 0, len(want))
|
||||
for _, p := range want {
|
||||
wantIDs = append(wantIDs, p.ID)
|
||||
}
|
||||
ids, err := repo.GetAllIDs()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(ids).To(ConsistOf(wantIDs))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Exists", func() {
|
||||
It("returns true for an existing playlist", func() {
|
||||
Expect(repo.Exists(plsCool.ID)).To(BeTrue())
|
||||
|
||||
@ -59,6 +59,14 @@ func (r *radioRepository) GetAll(options ...model.QueryOptions) (model.Radios, e
|
||||
return res, err
|
||||
}
|
||||
|
||||
// GetAllIDs returns just the radio IDs. Used by bulk enumeration (artwork backfill).
|
||||
func (r *radioRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
sel := r.newSelect(options...).Columns("id")
|
||||
ids := []string{}
|
||||
err := r.queryAllSlice(sel, &ids)
|
||||
return ids, err
|
||||
}
|
||||
|
||||
func (r *radioRepository) Put(radio *model.Radio, colsToUpdate ...string) error {
|
||||
if !r.isPermitted() {
|
||||
return rest.ErrPermissionDenied
|
||||
|
||||
@ -78,6 +78,21 @@ var _ = Describe("RadioRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetAllIDs", func() {
|
||||
It("returns the same id set as GetAll", func() {
|
||||
want, err := repo.GetAll()
|
||||
Expect(err).To(BeNil())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
wantIDs := make([]string, 0, len(want))
|
||||
for _, r := range want {
|
||||
wantIDs = append(wantIDs, r.ID)
|
||||
}
|
||||
ids, err := repo.GetAllIDs()
|
||||
Expect(err).To(BeNil())
|
||||
Expect(ids).To(ConsistOf(wantIDs))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Put", func() {
|
||||
It("successfully updates item", func() {
|
||||
err := repo.Put(&model.Radio{
|
||||
|
||||
@ -76,6 +76,18 @@ func (m *MockAlbumRepo) GetAll(qo ...model.QueryOptions) (model.Albums, error) {
|
||||
return m.All, nil
|
||||
}
|
||||
|
||||
func (m *MockAlbumRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
|
||||
all, err := m.GetAll(qo...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ids := make([]string, len(all))
|
||||
for i, a := range all {
|
||||
ids[i] = a.ID
|
||||
}
|
||||
return ids, nil
|
||||
}
|
||||
|
||||
func (m *MockAlbumRepo) GetCursor(qo ...model.QueryOptions) (model.AlbumCursor, error) {
|
||||
res, err := m.GetAll(qo...)
|
||||
if err != nil {
|
||||
|
||||
@ -113,6 +113,18 @@ func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
|
||||
return allArtists, nil
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
all, err := m.GetAll(options...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ids := make([]string, len(all))
|
||||
for i, a := range all {
|
||||
ids[i] = a.ID
|
||||
}
|
||||
return ids, nil
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
|
||||
res, err := m.GetAll(options...)
|
||||
if err != nil {
|
||||
|
||||
@ -52,6 +52,18 @@ func (m *MockPlaylistRepo) GetAll(options ...model.QueryOptions) (model.Playlist
|
||||
return m.All, nil
|
||||
}
|
||||
|
||||
func (m *MockPlaylistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
all, err := m.GetAll(options...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ids := make([]string, len(all))
|
||||
for i, p := range all {
|
||||
ids[i] = p.ID
|
||||
}
|
||||
return ids, nil
|
||||
}
|
||||
|
||||
func (m *MockPlaylistRepo) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
|
||||
res, err := m.GetAll(options...)
|
||||
if err != nil {
|
||||
|
||||
@ -73,6 +73,18 @@ func (m *MockedRadioRepo) GetAll(qo ...model.QueryOptions) (model.Radios, error)
|
||||
return m.All, nil
|
||||
}
|
||||
|
||||
func (m *MockedRadioRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
|
||||
all, err := m.GetAll(qo...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ids := make([]string, len(all))
|
||||
for i, r := range all {
|
||||
ids[i] = r.ID
|
||||
}
|
||||
return ids, nil
|
||||
}
|
||||
|
||||
func (m *MockedRadioRepo) Put(radio *model.Radio, _ ...string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user