mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
* perf(persistence): use *_artists join tables for artist participant filters The artist_id/artists_id, role_<role>_id and role_total_id filters, plus the AlbumsByArtistID/AlbumsByContributingArtistID/SongsByArtistID helpers, scanned every album or media_file row through json_tree(participants, ...), which no index can serve — the cause of multi-second artist pages on large libraries (discussion #5929). Rewrite them to semi-join the album_artists and media_file_artists tables via a shared ParticipantIDFilter helper. The join tables are written in the same transaction as the participants JSON, so results are unchanged. album_artists' unique constraint led with album_id, so artist-driven lookups had no usable index. Rebuild the table with the constraint reordered to (artist_id, album_id, role, sub_role), mirroring media_file_artists, instead of adding a fourth index: measured within 4% of a dedicated covering index (geomean -92.5% vs json_tree on a 96k-track production copy) while saving ~7MiB and per-scan write amplification. Album-side consumers (participant rewrites, FK cascades, markMissing) keep using album_artists_album_id, and updateParticipants' ON CONFLICT target already names artist_id first. The rebuild is linear work: 1.1s on a 113k-row production copy. * chore(gitignore): add temp benchmark files to ignore list * fix(persistence): clear album_artists when an album is saved without participants albumRepository.Put skipped updateParticipants when the Participants map was empty, so a hypothetical save with no participants would write {} to the JSON column but leave stale album_artists rows behind, now visible through the semi-join filters. No current caller can hit this (albums built by MediaFiles.ToAlbum always have participants), but make Put unconditional anyway, matching mediaFileRepository.Put, so the join table always moves with the JSON. Raised by Codex review on #5930.
146 lines
5.0 KiB
Go
146 lines
5.0 KiB
Go
package persistence
|
|
|
|
import (
|
|
"encoding/json"
|
|
"fmt"
|
|
|
|
. "github.com/Masterminds/squirrel"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/utils/slice"
|
|
)
|
|
|
|
type participant struct {
|
|
ID string `json:"id"`
|
|
Name string `json:"name"`
|
|
SubRole string `json:"subRole,omitempty"`
|
|
}
|
|
|
|
// flatParticipant represents a flattened participant structure for SQL processing
|
|
type flatParticipant struct {
|
|
ArtistID string `json:"artist_id"`
|
|
Role string `json:"role"`
|
|
SubRole string `json:"sub_role,omitempty"`
|
|
}
|
|
|
|
// ParticipantIDFilter matches rows of table where the artist participates in any of the given roles
|
|
// (any role when empty). Semi-joins <table>_artists; json_tree over the JSON is far slower at scale.
|
|
func ParticipantIDFilter(table string, artistID any, roles ...model.Role) Sqlizer {
|
|
return participantIDFilter(table, artistID, false, roles)
|
|
}
|
|
|
|
// NotParticipantIDFilter is the negation of ParticipantIDFilter.
|
|
func NotParticipantIDFilter(table string, artistID any, roles ...model.Role) Sqlizer {
|
|
return participantIDFilter(table, artistID, true, roles)
|
|
}
|
|
|
|
func participantIDFilter(table string, artistID any, negate bool, roles []model.Role) Sqlizer {
|
|
sel := Select(table + "_id").From(table + "_artists").Where(Eq{"artist_id": artistID})
|
|
if len(roles) > 0 {
|
|
sel = sel.Where(Eq{"role": slice.Map(roles, func(r model.Role) string { return r.String() })})
|
|
}
|
|
sql, args, _ := sel.ToSql()
|
|
op := " IN ("
|
|
if negate {
|
|
op = " NOT IN ("
|
|
}
|
|
return Expr(table+".id"+op+sql+")", args...)
|
|
}
|
|
|
|
func marshalParticipants(participants model.Participants) string {
|
|
dbParticipants := make(map[model.Role][]participant)
|
|
for role, artists := range participants {
|
|
for _, artist := range artists {
|
|
dbParticipants[role] = append(dbParticipants[role], participant{ID: artist.ID, SubRole: artist.SubRole, Name: artist.Name})
|
|
}
|
|
}
|
|
res, _ := json.Marshal(dbParticipants)
|
|
return string(res)
|
|
}
|
|
|
|
func unmarshalParticipants(data string) (model.Participants, error) {
|
|
var dbParticipants map[model.Role][]participant
|
|
err := json.Unmarshal([]byte(data), &dbParticipants)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("parsing participants: %w", err)
|
|
}
|
|
|
|
participants := make(model.Participants, len(dbParticipants))
|
|
for role, participantList := range dbParticipants {
|
|
artists := slice.Map(participantList, func(p participant) model.Participant {
|
|
return model.Participant{Artist: model.Artist{ID: p.ID, Name: p.Name}, SubRole: p.SubRole}
|
|
})
|
|
participants[role] = artists
|
|
}
|
|
return participants, nil
|
|
}
|
|
|
|
func (r sqlRepository) updateParticipants(itemID string, participants model.Participants) error {
|
|
// Delete all existing participant entries for this item.
|
|
// This ensures stale role associations are removed when an artist's role changes
|
|
// (e.g., an artist was both albumartist and composer, but is now only composer).
|
|
sqd := Delete(r.tableName + "_artists").Where(Eq{r.tableName + "_id": itemID})
|
|
_, err := r.executeSQL(sqd)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if len(participants) == 0 {
|
|
return nil
|
|
}
|
|
|
|
var flatParticipants []flatParticipant
|
|
for role, artists := range participants {
|
|
for _, artist := range artists {
|
|
flatParticipants = append(flatParticipants, flatParticipant{
|
|
ArtistID: artist.ID,
|
|
Role: role.String(),
|
|
SubRole: artist.SubRole,
|
|
})
|
|
}
|
|
}
|
|
|
|
participantsJSON, err := json.Marshal(flatParticipants)
|
|
if err != nil {
|
|
return fmt.Errorf("marshaling participants: %w", err)
|
|
}
|
|
|
|
// Build the INSERT query using json_each and INNER JOIN to artist table
|
|
// to automatically filter out non-existent artist IDs
|
|
query := fmt.Sprintf(`
|
|
INSERT INTO %[1]s_artists (%[1]s_id, artist_id, role, sub_role)
|
|
SELECT ?,
|
|
json_extract(value, '$.artist_id') as artist_id,
|
|
json_extract(value, '$.role') as role,
|
|
COALESCE(json_extract(value, '$.sub_role'), '') as sub_role
|
|
-- Parse the flat JSON array: [{"artist_id": "id", "role": "role", "sub_role": "subRole"}]
|
|
FROM json_each(?) -- Iterate through each array element
|
|
-- CRITICAL: Only insert records for artists that actually exist in the database
|
|
JOIN artist ON artist.id = json_extract(value, '$.artist_id') -- Filter out non-existent artist IDs via INNER JOIN
|
|
-- Handle duplicate insertions gracefully (e.g., if called multiple times)
|
|
ON CONFLICT (artist_id, %[1]s_id, role, sub_role) DO NOTHING -- Ignore duplicates
|
|
`, r.tableName)
|
|
|
|
_, err = r.executeSQL(Expr(query, itemID, string(participantsJSON)))
|
|
return err
|
|
}
|
|
|
|
func (r *sqlRepository) getParticipants(m *model.MediaFile) (model.Participants, error) {
|
|
ar := NewArtistRepository(r.ctx, r.db)
|
|
ids := m.Participants.AllIDs()
|
|
artists, err := ar.GetAll(model.QueryOptions{Filters: Eq{"artist.id": ids}})
|
|
if err != nil {
|
|
return nil, fmt.Errorf("getting participants: %w", err)
|
|
}
|
|
artistMap := slice.ToMap(artists, func(a model.Artist) (string, model.Artist) {
|
|
return a.ID, a
|
|
})
|
|
p := m.Participants
|
|
for role, artistList := range p {
|
|
for idx, artist := range artistList {
|
|
if a, ok := artistMap[artist.ID]; ok {
|
|
p[role][idx].Artist = a
|
|
}
|
|
}
|
|
}
|
|
return p, nil
|
|
}
|