From c7549bbe48f75bf6c0dbb977bfb1766d41c3fb77 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:13:27 -0400 Subject: [PATCH] perf(api): collapse N+1 in /api/artists/{id} + 1 round-trip in /api/albums/{id} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new sqlc queries replace three sequential per-album round trips that were dominating detail-screen latency. GetAlbumWithArtist: handleGetAlbum was doing GetAlbumByID then GetArtistByID — separate round trips for one logical lookup. The new query joins albums + artists with sqlc.embed and returns both in one SELECT. Detail-page DB cost: 3 trips → 2. ListAlbumsByArtistWithTrackCount: handleGetArtist was loading the artist's album list, then issuing one CountTracksByAlbum per album to populate track_count. On a 30-album artist that's 32 sequential queries — each ~5ms over a local DB, ~30ms over a remote one. The new query embeds the album row + a correlated count(*) subquery, so every album's track count comes back in one SELECT regardless of album count. Detail-page DB cost: 1 + N → 1 + 1. Together these account for the bulk of cold-cache navigation latency on the Flutter client. Combined with the existing SWR + nav hydration on the client side, detail screens should render their header instantly and the body within one round trip instead of N+constant. --- internal/api/library.go | 47 +++++++++++-------- internal/db/dbq/albums.sql.go | 85 ++++++++++++++++++++++++++++++++++ internal/db/queries/albums.sql | 20 ++++++++ 3 files changed, 132 insertions(+), 20 deletions(-) diff --git a/internal/api/library.go b/internal/api/library.go index 3d94da35..206c5549 100644 --- a/internal/api/library.go +++ b/internal/api/library.go @@ -40,19 +40,28 @@ func (h *handlers) handleGetTrack(w http.ResponseWriter, r *http.Request) { // handleGetAlbum implements GET /api/albums/{id}. Returns the album plus its // tracks (ordered by disc/track number via the underlying query) with // duration summed from the track list — keeps one source of truth. +// +// Two DB round trips: the album+artist join and the per-user tracks +// list. Down from three (separate album, artist, tracks) before +// GetAlbumWithArtist landed. func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { q := dbq.New(h.pool) - album, apiErr := resolveByID(r, "id", q.GetAlbumByID, "album") - if apiErr != nil { - writeErr(w, apiErr) + id, ok := requireURLUUID(w, r, "id") + if !ok { return } - artist, err := q.GetArtistByID(r.Context(), album.ArtistID) + row, err := q.GetAlbumWithArtist(r.Context(), id) if err != nil { - h.logger.Error("api: get album artist failed", "err", err) + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, apierror.NotFound("album")) + return + } + h.logger.Error("api: get album+artist failed", "err", err) writeErr(w, apierror.InternalMsg("lookup failed", err)) return } + album := row.Album + artistName := row.ArtistName var userID pgtype.UUID if user, ok := auth.UserFromContext(r.Context()); ok { userID = user.ID @@ -68,20 +77,24 @@ func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { refs := make([]TrackRef, 0, len(tracks)) durSec := 0 for _, t := range tracks { - ref := trackRefFrom(t, album.Title, artist.Name) + ref := trackRefFrom(t, album.Title, artistName) refs = append(refs, ref) durSec += ref.DurationSec } detail := AlbumDetail{ - AlbumRef: albumRefFrom(album, artist.Name, len(tracks), durSec), + AlbumRef: albumRefFrom(album, artistName, len(tracks), durSec), Tracks: refs, } writeJSON(w, http.StatusOK, detail) } // handleGetArtist implements GET /api/artists/{id}. Returns artist + albums; -// each album carries its own track_count (one count query per album, same -// pattern as subsonic). At M1 sizes (albums-per-artist << 100) this is fine. +// each album carries its own track_count. +// +// Down from 1 + 1 + N queries (artist, albums, per-album CountTracksByAlbum) +// to 1 + 1 (artist, albums-with-track-count via correlated subquery). +// On a 30-album artist that's ~32 round trips collapsed to 2 — the +// difference between "feels slow" and "feels instant" on detail nav. func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { q := dbq.New(h.pool) artist, apiErr := resolveByID(r, "id", q.GetArtistByID, "artist") @@ -89,25 +102,19 @@ func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { writeErr(w, apiErr) return } - albums, err := q.ListAlbumsByArtist(r.Context(), artist.ID) + rows, err := q.ListAlbumsByArtistWithTrackCount(r.Context(), artist.ID) if err != nil { h.logger.Error("api: list albums by artist failed", "err", err) writeErr(w, apierror.InternalMsg("lookup failed", err)) return } - refs := make([]AlbumRef, 0, len(albums)) - for _, a := range albums { - count, cerr := q.CountTracksByAlbum(r.Context(), a.ID) - if cerr != nil { - h.logger.Error("api: count tracks failed", "err", cerr) - writeErr(w, apierror.InternalMsg("lookup failed", cerr)) - return - } + refs := make([]AlbumRef, 0, len(rows)) + for _, row := range rows { // durationSec=0: not aggregated for nested album lists per spec data flow. - refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0)) + refs = append(refs, albumRefFrom(row.Album, artist.Name, int(row.TrackCount), 0)) } detail := ArtistDetail{ - ArtistRef: artistRefFrom(artist, len(albums)), + ArtistRef: artistRefFrom(artist, len(rows)), Albums: refs, } writeJSON(w, http.StatusOK, detail) diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 5168f686..ca990688 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -168,6 +168,41 @@ func (q *Queries) GetAlbumCoverageRollup(ctx context.Context) (GetAlbumCoverageR return i, err } +const getAlbumWithArtist = `-- name: GetAlbumWithArtist :one +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +FROM albums +JOIN artists ON artists.id = albums.artist_id +WHERE albums.id = $1 +` + +type GetAlbumWithArtistRow struct { + Album Album + ArtistName string +} + +// Combined fetch for /api/albums/{id}: returns the album row + the +// joined artist name in a single round trip. Replaces a sequential +// GetAlbumByID + GetArtistByID pair on the hot detail-page path. +func (q *Queries) GetAlbumWithArtist(ctx context.Context, id pgtype.UUID) (GetAlbumWithArtistRow, error) { + row := q.db.QueryRow(ctx, getAlbumWithArtist, id) + var i GetAlbumWithArtistRow + err := row.Scan( + &i.Album.ID, + &i.Album.Title, + &i.Album.SortTitle, + &i.Album.ArtistID, + &i.Album.ReleaseDate, + &i.Album.Mbid, + &i.Album.CoverArtPath, + &i.Album.CreatedAt, + &i.Album.UpdatedAt, + &i.Album.CoverArtSource, + &i.Album.CoverArtSourcesVersion, + &i.ArtistName, + ) + return i, err +} + const getAlbumsByIDs = `-- name: GetAlbumsByIDs :many SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE id = ANY($1::uuid[]) ` @@ -389,6 +424,56 @@ func (q *Queries) ListAlbumsByArtist(ctx context.Context, artistID pgtype.UUID) return items, nil } +const listAlbumsByArtistWithTrackCount = `-- name: ListAlbumsByArtistWithTrackCount :many +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, + (SELECT count(*) FROM tracks t WHERE t.album_id = albums.id)::bigint + AS track_count +FROM albums +WHERE albums.artist_id = $1 +ORDER BY release_date NULLS LAST, sort_title +` + +type ListAlbumsByArtistWithTrackCountRow struct { + Album Album + TrackCount int64 +} + +// Replaces the N+1 pattern in handleGetArtist (1 + N CountTracksByAlbum +// per album). Returns each album joined with its track count via a +// correlated subquery — single round trip regardless of album count. +func (q *Queries) ListAlbumsByArtistWithTrackCount(ctx context.Context, artistID pgtype.UUID) ([]ListAlbumsByArtistWithTrackCountRow, error) { + rows, err := q.db.Query(ctx, listAlbumsByArtistWithTrackCount, artistID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsByArtistWithTrackCountRow + for rows.Next() { + var i ListAlbumsByArtistWithTrackCountRow + if err := rows.Scan( + &i.Album.ID, + &i.Album.Title, + &i.Album.SortTitle, + &i.Album.ArtistID, + &i.Album.ReleaseDate, + &i.Album.Mbid, + &i.Album.CoverArtPath, + &i.Album.CreatedAt, + &i.Album.UpdatedAt, + &i.Album.CoverArtSource, + &i.Album.CoverArtSourcesVersion, + &i.TrackCount, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listAlbumsByGenre = `-- name: ListAlbumsByGenre :many SELECT DISTINCT ON (albums.id) albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version FROM albums diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 38f3c34a..9811a083 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -14,6 +14,26 @@ RETURNING *; -- name: GetAlbumByID :one SELECT * FROM albums WHERE id = $1; +-- name: GetAlbumWithArtist :one +-- Combined fetch for /api/albums/{id}: returns the album row + the +-- joined artist name in a single round trip. Replaces a sequential +-- GetAlbumByID + GetArtistByID pair on the hot detail-page path. +SELECT sqlc.embed(albums), artists.name AS artist_name +FROM albums +JOIN artists ON artists.id = albums.artist_id +WHERE albums.id = $1; + +-- name: ListAlbumsByArtistWithTrackCount :many +-- Replaces the N+1 pattern in handleGetArtist (1 + N CountTracksByAlbum +-- per album). Returns each album joined with its track count via a +-- correlated subquery — single round trip regardless of album count. +SELECT sqlc.embed(albums), + (SELECT count(*) FROM tracks t WHERE t.album_id = albums.id)::bigint + AS track_count +FROM albums +WHERE albums.artist_id = $1 +ORDER BY release_date NULLS LAST, sort_title; + -- name: GetAlbumByArtistAndTitle :one -- Scanner uses this for the no-mbid dedupe path: resolve-or-create. SELECT * FROM albums WHERE artist_id = $1 AND title = $2 LIMIT 1;