mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
perf(playlists): evaluate smart playlist criteria before taking the write lock
Smart playlists were refreshed with a single INSERT ... SELECT, so SQLite held the write lock while the criteria query ran. On a Raspberry Pi 4 with 300k songs and a rule matching 200k of them, that blocked every other writer for the whole 6 to 8 s evaluation, pushing the worst writer wait past the 15 s busy timeout at the end of a scan. The criteria query now runs first as a plain read, which WAL lets run alongside writers. The matching ids are then written in one short transaction: delete the old tracks, insert the new ones in order through json_each, and update the counters and evaluated_at. On the same Pi the evaluation no longer raises the worst writer wait above the scan's own baseline; the write itself holds the lock for about 3 s instead of the full evaluation. This also applies when a playlist is refreshed on read, and the scanner no longer wraps Evaluate in a transaction, so the read runs outside any lock.
This commit is contained in:
parent
210fd55021
commit
e510f34604
3 changed files with 51 additions and 30 deletions
|
|
@ -260,6 +260,17 @@ func (r *playlistRepository) selectPlaylist(ctx context.Context, options ...mode
|
|||
return r.withAnnotation(ctx, sel, r.tableName+".id")
|
||||
}
|
||||
|
||||
// inTx runs fn in a transaction, joining the caller's if one is already open.
|
||||
func (r *playlistRepository) inTx(fn func(tx *playlistRepository) error) error {
|
||||
conn, ok := r.db.(*dbx.DB)
|
||||
if !ok {
|
||||
return fn(r)
|
||||
}
|
||||
return conn.Transactional(func(tx *dbx.Tx) error {
|
||||
return fn(NewPlaylistRepository(tx).(*playlistRepository))
|
||||
})
|
||||
}
|
||||
|
||||
func (r *playlistRepository) updateTracks(ctx context.Context, id string, tracks model.MediaFiles) error {
|
||||
ids := make([]string, len(tracks))
|
||||
for i := range tracks {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ package persistence
|
|||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"slices"
|
||||
"time"
|
||||
|
|
@ -57,12 +59,6 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls *
|
|||
log.Debug(ctx, "Refreshing smart playlist", "playlist", pls.Name, "id", pls.ID)
|
||||
start := time.Now()
|
||||
|
||||
del := Delete("playlist_tracks").Where(Eq{"playlist_id": pls.ID})
|
||||
if _, err := r.executeSQL(ctx, del); err != nil {
|
||||
log.Error(ctx, "Error deleting old smart playlist tracks", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
|
||||
rulesSQL := newSmartPlaylistCriteria(*pls.NormalizedRules(), withSmartPlaylistOwner(*usr))
|
||||
|
||||
if !r.refreshChildPlaylists(ctx, pls, rulesSQL, visited) {
|
||||
|
|
@ -73,37 +69,56 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls *
|
|||
return false
|
||||
}
|
||||
|
||||
sq := r.buildSmartPlaylistQuery(ctx, pls, rulesSQL, usr.ID)
|
||||
sq, err := r.addCriteria(sq, rulesSQL)
|
||||
sq, err := r.addCriteria(r.buildSmartPlaylistQuery(ctx, rulesSQL, usr.ID), rulesSQL)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error building smart playlist criteria", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
|
||||
insSql := Insert("playlist_tracks").Columns("id", "playlist_id", "media_file_id").Select(sq)
|
||||
if _, err = r.executeSQL(ctx, insSql); err != nil {
|
||||
// Evaluate the criteria before writing, so the write lock is only held for the short replace below
|
||||
var ids []string
|
||||
if err = r.queryAllSlice(ctx, sq, &ids); err != nil && !errors.Is(err, model.ErrNotFound) {
|
||||
log.Error(ctx, "Error evaluating smart playlist criteria", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
|
||||
err = r.inTx(func(tx *playlistRepository) error { return tx.replaceSmartPlaylistTracks(ctx, pls, ids) })
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error refreshing smart playlist tracks", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
|
||||
if err = r.refreshCounters(ctx, pls); err != nil {
|
||||
log.Error(ctx, "Error updating smart playlist stats", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
|
||||
// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
|
||||
now := pls.UpdatedAt
|
||||
updSql := Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})
|
||||
if _, err = r.executeSQL(ctx, updSql); err != nil {
|
||||
log.Error(ctx, "Error updating smart playlist", "playlist", pls.Name, "id", pls.ID, err)
|
||||
return false
|
||||
}
|
||||
pls.EvaluatedAt = &now
|
||||
|
||||
log.Debug(ctx, "Refreshed playlist", "playlist", pls.Name, "id", pls.ID, "numTracks", pls.SongCount, "elapsed", time.Since(start))
|
||||
return true
|
||||
}
|
||||
|
||||
func (r *playlistRepository) replaceSmartPlaylistTracks(ctx context.Context, pls *model.Playlist, ids []string) error {
|
||||
if _, err := r.executeSQL(ctx, Delete("playlist_tracks").Where(Eq{"playlist_id": pls.ID})); err != nil {
|
||||
return err
|
||||
}
|
||||
if len(ids) > 0 {
|
||||
idsJSON, err := json.Marshal(ids)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
ins := Expr("INSERT INTO playlist_tracks (id, playlist_id, media_file_id) SELECT key + 1, ?, value FROM json_each(?)",
|
||||
pls.ID, string(idsJSON))
|
||||
if _, err = r.executeSQL(ctx, ins); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if err := r.refreshCounters(ctx, pls); err != nil {
|
||||
return err
|
||||
}
|
||||
// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
|
||||
now := pls.UpdatedAt
|
||||
if _, err := r.executeSQL(ctx, Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})); err != nil {
|
||||
return err
|
||||
}
|
||||
pls.EvaluatedAt = &now
|
||||
return nil
|
||||
}
|
||||
|
||||
// shouldRefreshSmartPlaylist determines if a smart playlist needs to be refreshed based on its type, last evaluated
|
||||
// time, and ownership.
|
||||
func (r *playlistRepository) shouldRefreshSmartPlaylist(ctx context.Context, pls *model.Playlist, usr *model.User) bool {
|
||||
|
|
@ -194,12 +209,10 @@ func (r *playlistRepository) resolvePercentageLimit(ctx context.Context, pls *mo
|
|||
return nil
|
||||
}
|
||||
|
||||
// buildSmartPlaylistQuery constructs the SQL query to select media files matching the smart playlist criteria,
|
||||
// including the joins its fields require and library filtering.
|
||||
func (r *playlistRepository) buildSmartPlaylistQuery(ctx context.Context, pls *model.Playlist, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
|
||||
orderBy := rulesSQL.orderBy()
|
||||
sq := Select("row_number() over (order by "+orderBy+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id").
|
||||
From("media_file")
|
||||
// buildSmartPlaylistQuery constructs the SQL query to select the ids of media files matching the smart playlist
|
||||
// criteria, including the joins its fields require and library filtering.
|
||||
func (r *playlistRepository) buildSmartPlaylistQuery(ctx context.Context, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
|
||||
sq := Select("media_file.id").From("media_file")
|
||||
sq = rulesSQL.applyRequiredJoins(sq, userID)
|
||||
sq = r.applyLibraryFilter(ctx, sq, "media_file")
|
||||
return sq
|
||||
|
|
|
|||
|
|
@ -356,10 +356,7 @@ func (s *scannerImpl) runEvaluateSmartPlaylists(ctx context.Context, state *scan
|
|||
return func() error {
|
||||
for _, id := range state.smartPlaylistsToEvaluate() {
|
||||
start := time.Now()
|
||||
err := s.ds.WithTxRetry(ctx, func(ctx context.Context, tx model.DataStore) error {
|
||||
return tx.Playlist().Evaluate(ctx, id)
|
||||
}, "scanner: evaluate smart playlist")
|
||||
if err != nil {
|
||||
if err := s.ds.Playlist().Evaluate(ctx, id); err != nil {
|
||||
log.Warn(ctx, "Scanner: Could not evaluate smart playlist", "id", id, err)
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue