perf(persistence): make album list pages index-served

The two album list requests reported in discussion #5929 (a 110k-album,
two-library instance) were dominated by work that scaled with the whole
library instead of the page:

- Every native API list request also runs a count for the X-Total-Count
  header. album had no index covering library_id, so that count scanned the
  entire table on every request, dragging each row's JSON blobs through the
  page cache. count(distinct id) additionally built a temp b-tree over the
  full result set even when the query had no join that could duplicate rows.
- A random sort cannot be served by an index, so SQLite evaluated SEEDEDRAND
  for every matching row and pushed each one, with its library and annotation
  joins and full column projection, through a sorter that kept only 36 rows.

Add an album(id, library_id) index so library-filtered counts are served by a
covering index scan, and make the shared count() helper use count(*) when the
rendered query has no join, keeping count(distinct id) wherever a join could
fan out rows. id leads the index deliberately: with library_id leading, the
planner drives sorted list queries off it and sorts an entire library rather
than walking a sort-satisfying index, and fresh statistics do not change that
choice.

Resolve random pages by id before hydrating them, so the sort pass runs on
that covering index with no joins and only the page itself is materialized.
Pages larger than 1000 keep the single-query path. The seeded shuffle is
unchanged: the id query carries the caller's options, so SEEDEDRAND, explicit
seeds, and the reseed-only-at-offset-0 rule all still apply, and the page is
reordered in Go to preserve it.

Also make the album name sort a total order by appending album.id, and widen
album_order_album_name to (order_album_name, order_album_artist_name, id) to
serve it. Without a tiebreaker, equal names made page order plan-dependent,
which can skip or duplicate rows across paginated requests.
This commit is contained in:
Deluan 2026-08-10 15:41:22 -04:00
parent 7736bbb545
commit 1c83f36ddf
5 changed files with 155 additions and 2 deletions

View File

@ -0,0 +1,16 @@
-- +goose Up
-- id leads so library_id cannot drive a seek: the index covers library-filtered counts without
-- tempting the planner to sort a whole library instead of walking a sort-satisfying index.
create index if not exists album_id_library_id
on album(id, library_id);
drop index if exists album_order_album_name;
create index album_order_album_name
on album(order_album_name, order_album_artist_name, id);
-- +goose Down
drop index if exists album_id_library_id;
drop index if exists album_order_album_name;
create index album_order_album_name
on album(order_album_name);

View File

@ -1,6 +1,7 @@
package persistence
import (
"cmp"
"context"
"encoding/json"
"errors"
@ -109,7 +110,7 @@ func NewAlbumRepository(ctx context.Context, db dbx.Builder) model.AlbumReposito
r.tableName = "album"
r.registerModel(&model.Album{}, albumFilters())
r.setSortMappings(map[string]string{
"name": "order_album_name, order_album_artist_name",
"name": "order_album_name, order_album_artist_name, album.id",
"artist": "compilation, order_album_artist_name, order_album_name",
"album_artist": "compilation, order_album_artist_name, order_album_name",
// TODO Rename this to just year (or date)
@ -239,7 +240,14 @@ func (r *albumRepository) Get(id string) (*model.Album, error) {
return &res[0], nil
}
// Above this page size the id round-trip stops paying for itself and the IN list gets unwieldy.
const maxRandomPageIDs = 1000
func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, error) {
if len(options) > 0 && options[0].Sort == "random" &&
options[0].Max > 0 && options[0].Max <= maxRandomPageIDs {
return r.getRandomPage(options[0])
}
sq := r.selectAlbum(options...)
var res dbAlbums
err := r.queryAll(sq, &res)
@ -251,6 +259,28 @@ func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, e
return albums, nil
}
// No index can serve a random sort, so the sort pass visits every matching row. Resolving ids
// first keeps that pass on a covering index instead of dragging full rows and joins through it.
func (r *albumRepository) getRandomPage(options model.QueryOptions) (model.Albums, error) {
ids, err := r.GetAllIDs(options)
if err != nil {
return nil, err
}
if len(ids) == 0 {
return model.Albums{}, nil
}
albums, err := r.GetAll(model.QueryOptions{Filters: Eq{"album.id": ids}})
if err != nil {
return nil, err
}
pos := make(map[string]int, len(ids))
for i, albumID := range ids {
pos[albumID] = i
}
slices.SortFunc(albums, func(a, b model.Album) int { return cmp.Compare(pos[a.ID], pos[b.ID]) })
return albums, nil
}
func (r *albumRepository) hydrateArtwork(albums model.Albums) {
hydrateItems(r.ctx, r.db, model.KindAlbumArtwork, albums,
func(a *model.Album) (string, *model.ItemImage) { return a.ID, &a.ItemImage })

View File

@ -109,6 +109,84 @@ var _ = Describe("AlbumRepository", func() {
Expect(GetAll()).To(Equal(testAlbums))
})
// The REST layer sends library_id as a string. SQLite only coerces it to the column's
// integer affinity while the term stays a plain column reference, so a filter built on an
// expression instead would silently match nothing (#5929).
DescribeTable("library_id filter from the REST layer",
func(sort string) {
res, err := albumRepo.ReadAll(rest.QueryOptions{
Sort: sort, Max: 10, Filters: map[string]any{"library_id": "1"},
})
Expect(err).ToNot(HaveOccurred())
Expect(res.(model.Albums)).ToNot(BeEmpty())
},
Entry("name sort", "name"),
Entry("random sort", "random"),
Entry("no sort", ""),
)
Context("random sort", func() {
It("returns the page in the same order the id query produced", func() {
opts := model.QueryOptions{Sort: "random", Max: 4, Seed: "a-seed"}
ids, err := albumRepo.GetAllIDs(opts)
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(HaveLen(4))
albums, err := GetAll(opts)
Expect(err).ToNot(HaveOccurred())
Expect(slice.Map(albums, func(a model.Album) string { return a.ID })).To(Equal(ids))
})
It("keeps paging on one shuffle for a given seed", func() {
ids := func(albums model.Albums) []string {
return slice.Map(albums, func(a model.Album) string { return a.ID })
}
firstTwoPages, err := GetAll(model.QueryOptions{Sort: "random", Max: 3, Seed: "s"})
Expect(err).ToNot(HaveOccurred())
secondPage, err := GetAll(model.QueryOptions{Sort: "random", Max: 3, Offset: 3, Seed: "s"})
Expect(err).ToNot(HaveOccurred())
whole, err := GetAll(model.QueryOptions{Sort: "random", Max: 6, Seed: "s"})
Expect(err).ToNot(HaveOccurred())
Expect(append(ids(firstTwoPages), ids(secondPage)...)).To(Equal(ids(whole)))
})
// Without an explicit seed, only Offset == 0 reseeds, so scrolling never repeats an album.
It("does not repeat albums across pages of one scroll", func() {
page1, err := GetAll(model.QueryOptions{Sort: "random", Max: 3})
Expect(err).ToNot(HaveOccurred())
page2, err := GetAll(model.QueryOptions{Sort: "random", Max: 3, Offset: 3})
Expect(err).ToNot(HaveOccurred())
Expect(page1).To(HaveLen(3))
Expect(page2).To(HaveLen(3))
for _, a := range page2 {
Expect(page1).ToNot(ContainElement(a))
}
})
It("returns every record when no limit is given", func() {
albums, err := GetAll(model.QueryOptions{Sort: "random"})
Expect(err).ToNot(HaveOccurred())
Expect(albums).To(ConsistOf(testAlbums))
})
It("applies filters", func() {
albums, err := GetAll(model.QueryOptions{
Sort: "random", Max: 10, Filters: squirrel.Eq{"album.name": albumSgtPeppers.Name},
})
Expect(err).ToNot(HaveOccurred())
Expect(albums).To(HaveLen(1))
Expect(albums[0].ID).To(Equal(albumSgtPeppers.ID))
})
It("hydrates the same fields as a non-random page", func() {
albums, err := GetAll(model.QueryOptions{
Sort: "random", Max: 1, Filters: squirrel.Eq{"album.id": albumSgtPeppers.ID},
})
Expect(err).ToNot(HaveOccurred())
Expect(albums).To(Equal(model.Albums{albumSgtPeppers}))
})
})
It("returns all records sorted", func() {
Expect(GetAll(model.QueryOptions{Sort: "name"})).To(Equal(model.Albums{
albumAbbeyRoad,

View File

@ -511,13 +511,26 @@ func (r sqlRepository) classifyOwnedWriteMiss(id string) error {
return rest.ErrNotFound
}
var joinRegex = regexp.MustCompile(`\bJOIN\b`)
// countExpression returns count(*) for join-free queries; joins can fan out rows per id and
// need the much more expensive count(distinct id) (temp b-tree over the whole result set).
func (r sqlRepository) countExpression(query SelectBuilder) string {
sql, _, err := query.Columns("1").ToSql()
if err != nil || joinRegex.MatchString(sql) {
return "count(distinct " + r.tableName + ".id) as count"
}
return "count(*) as count"
}
func (r sqlRepository) count(countQuery SelectBuilder, options ...model.QueryOptions) (int64, error) {
countQuery = countQuery.
RemoveColumns().Columns("count(distinct " + r.tableName + ".id) as count").
RemoveColumns().
RemoveOffset().RemoveLimit().
OrderBy(r.tableName + ".id"). // To remove any ORDER BY clause that could slow down the query
From(r.tableName)
countQuery = r.applyFilters(countQuery, options...)
countQuery = countQuery.Columns(r.countExpression(countQuery))
var res struct{ Count int64 }
err := r.queryOne(countQuery, &res)
return res.Count, err

View File

@ -18,6 +18,22 @@ var _ = Describe("sqlRepository", func() {
r.tableName = "table"
})
Describe("countExpression", func() {
It("uses a plain count when the query has no joins", func() {
sq := squirrel.Select().From("table").Where(squirrel.Eq{"library_id": []int{1, 4}})
Expect(r.countExpression(sq)).To(Equal("count(*) as count"))
})
It("uses a distinct count when the query has a join", func() {
sq := squirrel.Select().From("table").
LeftJoin("annotation on annotation.item_id = table.id")
Expect(r.countExpression(sq)).To(Equal("count(distinct table.id) as count"))
})
It("is not fooled by parametrized values containing the word join", func() {
sq := squirrel.Select().From("table").Where(squirrel.Eq{"name": "the join band"})
Expect(r.countExpression(sq)).To(Equal("count(*) as count"))
})
})
Describe("applyOptions", func() {
var sq squirrel.SelectBuilder
BeforeEach(func() {