mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-09 17:18:45 +00:00
perf(db): index media_file album/artist sort orders (#5706)
Some checks are pending
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
Some checks are pending
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
* perf(db): add composite indexes for song list album/artist sorts
The media_file sort mappings for album, artist and albumArtist expand to
multi-column ORDER BY clauses that no existing index could satisfy, so SQLite
fell back to a full table scan plus a temp B-tree sort of every row (including
the large lyrics/tags/full_text columns) even for a single 15-item page. On a
96K-track library this made /api/song?_sort=album take 3.6s on a cold cache.
Add composite indexes matching the three sort mappings, allowing the query to
walk the index and stop at the page size, in both directions. Drop the now
redundant single-column order_album_name/order_artist_name indexes (strict
prefixes of the new composites) and three indexes with no query path:
birth_time is only read in Go code, and artist/album_artist text column
lookups go through the media_file_artists table instead.
* fix(ui): make composer and track number columns non-sortable in song list
Clicking the Composer header was a silent no-op: composer is not a media_file
column, so the native API's sanitizeSort drops the sort and returns rows in
table order. Track number sorting across the whole library is not meaningful
and cannot use an index (the existing index leads with disc_number). Mark both
columns sortable={false}, like quality and mood.
* test(persistence): add sort index coverage test for large tables
Guard against sort options silently losing index support: every sort mapping
on media_file, album and artist is now verified with EXPLAIN QUERY PLAN to be
satisfiable by an index (both directions), so adding a mapping or dropping an
index that reintroduces a full-table temp B-tree sort fails the test. Sorts
that genuinely cannot use an index (random, annotation-join columns, JSON
expressions) must be declared in an exceptions list with the reason, keeping
the trade-off visible in review.
To make the sort mappings the complete declared sort surface, add identity
mappings for the media_file columns the UI sorts by without a mapping (year,
genre, duration, channels, bpm, path, comment, play_count, play_date, rating).
These are behaviorally no-ops: the same ORDER BY was previously produced by
the field whitelist fallback.
* perf(db): drop PreferSortTags expression indexes from media_file
The media_file sort_title/sort_artist_name/sort_album_name expression indexes
are only usable when PreferSortTags is enabled - a config reported by ~0.1% of
installations (insights, week of 2026-06-22) - yet every install pays their
storage (~8.6MB on a 96K-track library) and scanner write overhead. Drop them:
PreferSortTags installs fall back to a full sort for title/artist/album orders,
everyone else gets smaller DBs and cheaper writes. The order_album_name and
order_artist_name collation checks remain valid, now satisfied by the composite
sort indexes.
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
ae9b8a5fe6
commit
d4387c5502
5 changed files with 238 additions and 5 deletions
|
|
@ -0,0 +1,56 @@
|
|||
-- +goose Up
|
||||
-- +goose StatementBegin
|
||||
|
||||
-- Composite indexes matching the media_file sort mappings for album, artist and
|
||||
-- albumArtist. Without them, SQLite cannot satisfy the multi-column ORDER BY and
|
||||
-- falls back to a full scan + temp B-tree sort of the whole table (including all
|
||||
-- its large columns) even for a small LIMIT.
|
||||
create index if not exists media_file_album_sort
|
||||
on media_file(order_album_name, album_id, disc_number, track_number, order_artist_name, title);
|
||||
create index if not exists media_file_artist_sort
|
||||
on media_file(order_artist_name, order_album_name, release_date, disc_number, track_number);
|
||||
create index if not exists media_file_album_artist_sort
|
||||
on media_file(order_album_artist_name, order_album_name, release_date, disc_number, track_number);
|
||||
|
||||
-- These two are strict prefixes of the composites above, so they are redundant now.
|
||||
drop index if exists media_file_order_album_name;
|
||||
drop index if exists media_file_order_artist_name;
|
||||
|
||||
-- No query filters or sorts on these columns: birth_time is only read in Go code;
|
||||
-- artist/album_artist conditions go through the media_file_artists table.
|
||||
drop index if exists media_file_birth_time;
|
||||
drop index if exists media_file_artist;
|
||||
drop index if exists media_file_album_artist;
|
||||
|
||||
-- These expression indexes are only usable when PreferSortTags is enabled, a
|
||||
-- config used by ~0.1% of installations (per insights), yet they are maintained
|
||||
-- on every write of every install. Dropping them means those installs fall back
|
||||
-- to a full sort; everyone else saves the space and the scanner write overhead.
|
||||
drop index if exists media_file_sort_title;
|
||||
drop index if exists media_file_sort_artist_name;
|
||||
drop index if exists media_file_sort_album_name;
|
||||
-- +goose StatementEnd
|
||||
|
||||
-- +goose Down
|
||||
-- +goose StatementBegin
|
||||
drop index if exists media_file_album_sort;
|
||||
drop index if exists media_file_artist_sort;
|
||||
drop index if exists media_file_album_artist_sort;
|
||||
|
||||
create index if not exists media_file_order_album_name
|
||||
on media_file(order_album_name);
|
||||
create index if not exists media_file_order_artist_name
|
||||
on media_file(order_artist_name);
|
||||
create index if not exists media_file_birth_time
|
||||
on media_file(birth_time);
|
||||
create index if not exists media_file_artist
|
||||
on media_file(artist);
|
||||
create index if not exists media_file_album_artist
|
||||
on media_file(album_artist);
|
||||
create index if not exists media_file_sort_title
|
||||
on media_file (coalesce(nullif(sort_title,''),order_title) collate NOCASE);
|
||||
create index if not exists media_file_sort_artist_name
|
||||
on media_file (coalesce(nullif(sort_artist_name,''),order_artist_name) collate NOCASE);
|
||||
create index if not exists media_file_sort_album_name
|
||||
on media_file (coalesce(nullif(sort_album_name,''),order_album_name) collate NOCASE);
|
||||
-- +goose StatementEnd
|
||||
|
|
@ -50,9 +50,6 @@ var _ = Describe("Collation", func() {
|
|||
Entry("media_file.order_title", "media_file", "order_title collate nocase"),
|
||||
Entry("media_file.order_album_name", "media_file", "order_album_name collate nocase"),
|
||||
Entry("media_file.order_artist_name", "media_file", "order_artist_name collate nocase"),
|
||||
Entry("media_file.sort_title", "media_file", "coalesce(nullif(sort_title,''),order_title) collate nocase"),
|
||||
Entry("media_file.sort_album_name", "media_file", "coalesce(nullif(sort_album_name,''),order_album_name) collate nocase"),
|
||||
Entry("media_file.sort_artist_name", "media_file", "coalesce(nullif(sort_artist_name,''),order_artist_name) collate nocase"),
|
||||
Entry("media_file.path", "media_file", "path collate nocase"),
|
||||
Entry("playlist.name", "playlist", "name collate nocase"),
|
||||
Entry("radio.name", "radio", "name collate nocase"),
|
||||
|
|
|
|||
|
|
@ -91,6 +91,16 @@ func NewMediaFileRepository(ctx context.Context, db dbx.Builder) model.MediaFile
|
|||
"recently_added": mediaFileRecentlyAddedSort(),
|
||||
"starred_at": "starred, starred_at",
|
||||
"rated_at": "rating, rated_at",
|
||||
"year": "year",
|
||||
"genre": "genre",
|
||||
"duration": "duration",
|
||||
"channels": "channels",
|
||||
"bpm": "bpm",
|
||||
"path": "path",
|
||||
"comment": "comment",
|
||||
"play_count": "play_count",
|
||||
"play_date": "play_date",
|
||||
"rating": "rating",
|
||||
})
|
||||
return r
|
||||
}
|
||||
|
|
|
|||
168
persistence/sort_index_coverage_test.go
Normal file
168
persistence/sort_index_coverage_test.go
Normal file
|
|
@ -0,0 +1,168 @@
|
|||
package persistence
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"maps"
|
||||
"regexp"
|
||||
"slices"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/db"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
// These tests guard against sort options silently losing index support: adding or
|
||||
// changing a sort mapping, or dropping/renaming an index in a migration, must not
|
||||
// reintroduce full-table temp B-tree sorts on the large tables. Those are
|
||||
// catastrophic on big libraries but invisible on dev-sized ones, which is how the
|
||||
// unindexed album/artist song sorts went unnoticed for years.
|
||||
//
|
||||
// Every sort mapping is checked automatically: the real ORDER BY is built via
|
||||
// buildSortOrder (both directions) and verified with EXPLAIN QUERY PLAN against
|
||||
// the migrated test schema. The planner's choice is deterministic even on an
|
||||
// empty table. A sort passes when the plan has no full "USE TEMP B-TREE FOR
|
||||
// ORDER BY" step; an incremental sort of tie groups ("... FOR LAST TERM OF ORDER
|
||||
// BY") is fine, as it only sorts rows with equal leading columns.
|
||||
//
|
||||
// A new sort mapping therefore fails this test until a matching index is created.
|
||||
// The only escape hatch is exceptions, for sorts that genuinely cannot be
|
||||
// served by a table index (random, annotation-join columns, JSON expressions):
|
||||
// declaring one requires writing down the reason, making the trade-off visible in
|
||||
// review. The checks run with the default config: PreferSortTags=true rewrites
|
||||
// mappings to coalesce expressions with no matching indexes (used by ~0.1% of
|
||||
// installations, per insights), and is out of scope here.
|
||||
var _ = Describe("Sort index coverage", func() {
|
||||
conn := db.Db()
|
||||
|
||||
type repoCase struct {
|
||||
table string
|
||||
newRepo func(ctx context.Context) *sqlRepository
|
||||
// sort mapping -> reason it cannot be served by an index
|
||||
exceptions map[string]string
|
||||
}
|
||||
|
||||
cases := []repoCase{
|
||||
{
|
||||
table: "media_file",
|
||||
newRepo: func(ctx context.Context) *sqlRepository {
|
||||
return &NewMediaFileRepository(ctx, GetDBXBuilder()).(*mediaFileRepository).sqlRepository
|
||||
},
|
||||
exceptions: map[string]string{
|
||||
"random": "not a column sort",
|
||||
"starred_at": "sorts on annotation join columns",
|
||||
"rated_at": "sorts on annotation join columns",
|
||||
"play_count": "sorts on annotation join columns",
|
||||
"play_date": "sorts on annotation join columns",
|
||||
"rating": "sorts on annotation join columns",
|
||||
"comment": "UI-sortable but rarely used; not worth an index",
|
||||
},
|
||||
},
|
||||
{
|
||||
table: "album",
|
||||
newRepo: func(ctx context.Context) *sqlRepository {
|
||||
return &NewAlbumRepository(ctx, GetDBXBuilder()).(*albumRepository).sqlRepository
|
||||
},
|
||||
exceptions: map[string]string{
|
||||
"random": "not a column sort",
|
||||
"starred_at": "sorts on annotation join columns",
|
||||
"rated_at": "sorts on annotation join columns",
|
||||
"max_year": "coalesce expression over original_date/max_year, no expression index",
|
||||
},
|
||||
},
|
||||
{
|
||||
table: "artist",
|
||||
newRepo: func(ctx context.Context) *sqlRepository {
|
||||
return &NewArtistRepository(ctx, GetDBXBuilder()).(*artistRepository).sqlRepository
|
||||
},
|
||||
exceptions: map[string]string{ //nolint:gosec // G101 false positive, same as the artist sortMappings
|
||||
"starred_at": "sorts on annotation join columns",
|
||||
"rated_at": "sorts on annotation join columns",
|
||||
"song_count": "JSON expression over stats column",
|
||||
"album_count": "JSON expression over stats column",
|
||||
"size": "JSON expression over stats column",
|
||||
"maincredit_song_count": "aggregate over JSON stats",
|
||||
"maincredit_album_count": "aggregate over JSON stats",
|
||||
"maincredit_size": "aggregate over JSON stats",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
newCtx := func() context.Context {
|
||||
ctx := log.NewContext(GinkgoT().Context())
|
||||
return request.WithUser(ctx, model.User{ID: "userid"})
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
It(fmt.Sprintf("uses an index for every sort mapping on %s", c.table), func() {
|
||||
r := c.newRepo(newCtx())
|
||||
for _, sort := range slices.Sorted(maps.Keys(r.sortMappings)) {
|
||||
if _, ok := c.exceptions[sort]; ok {
|
||||
continue
|
||||
}
|
||||
for _, dir := range []string{"asc", "desc"} {
|
||||
orderBy := r.buildSortOrder(sort, dir)
|
||||
Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(),
|
||||
"sort %q (%s) on table %q needs an index. Create one matching its ORDER BY, or, if it cannot be served by an index, add it to exceptions with the reason",
|
||||
sort, dir, c.table)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
It(fmt.Sprintf("has no stale exceptions entries for %s", c.table), func() {
|
||||
r := c.newRepo(newCtx())
|
||||
for _, sort := range slices.Sorted(maps.Keys(c.exceptions)) {
|
||||
Expect(r.sortMappings).To(HaveKey(sort),
|
||||
"exceptions entry %q on table %q does not match any sort mapping - remove it", sort, c.table)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
It("uses an index for recently_added when RecentlyAddedByModTime is enabled", func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
conf.Server.RecentlyAddedByModTime = true
|
||||
for _, c := range cases[:2] { // media_file and album
|
||||
r := c.newRepo(newCtx())
|
||||
for _, dir := range []string{"asc", "desc"} {
|
||||
orderBy := r.buildSortOrder("recently_added", dir)
|
||||
Expect(checkSortUsesIndex(conn, c.table, orderBy)).To(Succeed(),
|
||||
"sort recently_added (%s) on table %q", dir, c.table)
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
// Matches the full-sort step only: incremental tie-group sorts are reported as
|
||||
// "USE TEMP B-TREE FOR LAST TERM OF ORDER BY" (or "LAST N TERMS") and are allowed.
|
||||
var fullTempBTreeSort = regexp.MustCompile(`USE TEMP B-TREE FOR ORDER BY`)
|
||||
|
||||
func checkSortUsesIndex(conn *sql.DB, table, orderBy string) error {
|
||||
rows, err := conn.Query(fmt.Sprintf("explain query plan select * from %s order by %s limit 15", table, orderBy))
|
||||
if err != nil {
|
||||
return fmt.Errorf("explain query plan failed for order by %q: %w", orderBy, err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
var details []string
|
||||
for rows.Next() {
|
||||
var id, parent, notUsed int
|
||||
var detail string
|
||||
if err := rows.Scan(&id, &parent, ¬Used, &detail); err != nil {
|
||||
return err
|
||||
}
|
||||
details = append(details, detail)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
if slices.ContainsFunc(details, fullTempBTreeSort.MatchString) {
|
||||
return fmt.Errorf("no index satisfies ORDER BY %s - plan: %v", orderBy, details)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
@ -143,9 +143,11 @@ const SongList = (props) => {
|
|||
return {
|
||||
album: isDesktop && <AlbumLinkField source="album" sortByOrder={'ASC'} />,
|
||||
artist: <ArtistLinkField source="artist" />,
|
||||
composer: <ArtistLinkField source="composer" />,
|
||||
composer: <ArtistLinkField source="composer" sortable={false} />,
|
||||
albumArtist: <ArtistLinkField source="albumArtist" />,
|
||||
trackNumber: isDesktop && <NumberField source="trackNumber" />,
|
||||
trackNumber: isDesktop && (
|
||||
<NumberField source="trackNumber" sortable={false} />
|
||||
),
|
||||
playCount: isDesktop && (
|
||||
<NumberField source="playCount" sortByOrder={'DESC'} />
|
||||
),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue