2025-12-16 16:08:32 -05:00
|
|
|
package cmd
|
|
|
|
|
|
|
|
|
|
import (
|
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* perf(db): keep query planner statistics trustworthy with full ANALYZE
PRAGMA optimize's internal ANALYZE runs with a limited analysis budget
(~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality
indexes: on a 96K-track library it claimed (missing, library_id) narrows
to ~2000 rows when it matches the whole table. The planner then prefers
that index over the sort index and falls back to a full-table temp
B-tree sort per request, turning paginated song listings into
multi-second queries (reproduced at 5.5s on real hardware; ~90x slower
than with correct stats). Every index-creating migration re-triggered
the poisoning via the post-migration optimize, and the daily optimizer
could re-trigger it on large library changes. Setting analysis_limit on
the connection does not help: optimize ignores it.
Run a plain full ANALYZE instead: after migrations with schema changes,
and in db.Optimize (daily schedule and scan-end). Stats are stored in
the database file, so one connection suffices and the per-connection
pool loop is gone. The Optimize call at shutdown is removed: stats are
maintained at migration/scan/daily points, and an ANALYZE during
shutdown only delays it and races container stop timeouts.
* perf(db): drop startup PRAGMA optimize that re-poisons planner stats
The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal
ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for
low-cardinality indexes -- the exact statistics-poisoning this PR set out to
eliminate. Because DevOptimizeDB defaults to true, a restart with no pending
migrations would re-poison the planner until the next scan or daily Optimize.
Remove it: statistics are already refreshed with a full ANALYZE after
schema-changing migrations (Init) and via Optimize at scan-end and on the daily
schedule, so nothing on the startup path needs to touch them. Also clarify that
Optimize is a no-op unless DevOptimizeDB is enabled.
* chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans
The flag only gated the optimize/ANALYZE maintenance calls and there is
no reason to leave planner statistics unmaintained; the guards are gone
along with the flag. The scan-end Optimize now runs only after full
scans — quick scans barely move the statistics, and the daily schedule
covers drift.
* style(scanner): drop redundant comment in runOptimize
* chore(persistence): drop the no-op PRAGMA optimize from ScanEnd
Mask 0x10000 only selects candidate tables by size change; without the
0x02 action bit optimize does nothing (verified: sqlite_stat1 stays
stale after a 100x table growth). The scan-end statistics refresh is
db.Optimize's full ANALYZE, and the expression-collation-index concern
the old comment guarded against no longer applies.
* fix(scanner): run the post-scan ANALYZE in the server process
With the external scanner (the default), the scan pipeline runs in a
subprocess, so its ANALYZE was invisible to the server: SQLite loads
sqlite_stat1 into the process's shared schema cache, and an ANALYZE from
another process does not refresh it — verified with the production DSN
that even brand-new pool connections keep planning with the old
statistics until the server restarts. An in-process ANALYZE, by
contrast, is immediately visible to every pooled connection through the
same shared cache.
Move the full-scan Optimize from the scanner pipeline to the scan
controller, which always runs in the server process.
* fix(scanner): honor promoted full scans in the optimize gate
A quick scan resuming an interrupted full scan is promoted inside the
scanner (possibly in a subprocess); mirror the promotion in the
controller so the post-scan ANALYZE isn't skipped.
* refactor: apply cleanup review findings
- drop forceFullRescan's inline ANALYZE: Init already runs a full
ANALYZE after any migration batch with schema changes, so upgrades
including a full-rescan migration analyzed the whole DB twice
- resumingFullScan uses a filtered CountAll instead of fetching and
scanning all libraries
- document why CallScan (CLI) deliberately skips the post-scan Optimize
* perf(db): make planner analysis maintenance resilient
Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans.
* perf(db): avoid analyzing routine quick-scan changes
Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check.
* fix(scan): analyze resumed full scans in CLI
* fix(db): back off failed analysis retries
* feat(db): allow disabling scheduled analysis
* test(db): remove redundant analysis coverage
* refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites
- move query-planner statistics code from db.go to its own optimize.go
(and matching optimize_test.go)
- log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of
repeating the timing block at every call site
- drop the LastDBAnalyzeAttemptAt write on success: it is only read
while failures >= 1, and every failure rewrites it first
- extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner)
helpers
2026-07-13 12:04:29 -04:00
|
|
|
"context"
|
2025-12-16 16:08:32 -05:00
|
|
|
"os"
|
|
|
|
|
"path/filepath"
|
|
|
|
|
|
|
|
|
|
"github.com/navidrome/navidrome/model"
|
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* perf(db): keep query planner statistics trustworthy with full ANALYZE
PRAGMA optimize's internal ANALYZE runs with a limited analysis budget
(~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality
indexes: on a 96K-track library it claimed (missing, library_id) narrows
to ~2000 rows when it matches the whole table. The planner then prefers
that index over the sort index and falls back to a full-table temp
B-tree sort per request, turning paginated song listings into
multi-second queries (reproduced at 5.5s on real hardware; ~90x slower
than with correct stats). Every index-creating migration re-triggered
the poisoning via the post-migration optimize, and the daily optimizer
could re-trigger it on large library changes. Setting analysis_limit on
the connection does not help: optimize ignores it.
Run a plain full ANALYZE instead: after migrations with schema changes,
and in db.Optimize (daily schedule and scan-end). Stats are stored in
the database file, so one connection suffices and the per-connection
pool loop is gone. The Optimize call at shutdown is removed: stats are
maintained at migration/scan/daily points, and an ANALYZE during
shutdown only delays it and races container stop timeouts.
* perf(db): drop startup PRAGMA optimize that re-poisons planner stats
The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal
ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for
low-cardinality indexes -- the exact statistics-poisoning this PR set out to
eliminate. Because DevOptimizeDB defaults to true, a restart with no pending
migrations would re-poison the planner until the next scan or daily Optimize.
Remove it: statistics are already refreshed with a full ANALYZE after
schema-changing migrations (Init) and via Optimize at scan-end and on the daily
schedule, so nothing on the startup path needs to touch them. Also clarify that
Optimize is a no-op unless DevOptimizeDB is enabled.
* chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans
The flag only gated the optimize/ANALYZE maintenance calls and there is
no reason to leave planner statistics unmaintained; the guards are gone
along with the flag. The scan-end Optimize now runs only after full
scans — quick scans barely move the statistics, and the daily schedule
covers drift.
* style(scanner): drop redundant comment in runOptimize
* chore(persistence): drop the no-op PRAGMA optimize from ScanEnd
Mask 0x10000 only selects candidate tables by size change; without the
0x02 action bit optimize does nothing (verified: sqlite_stat1 stays
stale after a 100x table growth). The scan-end statistics refresh is
db.Optimize's full ANALYZE, and the expression-collation-index concern
the old comment guarded against no longer applies.
* fix(scanner): run the post-scan ANALYZE in the server process
With the external scanner (the default), the scan pipeline runs in a
subprocess, so its ANALYZE was invisible to the server: SQLite loads
sqlite_stat1 into the process's shared schema cache, and an ANALYZE from
another process does not refresh it — verified with the production DSN
that even brand-new pool connections keep planning with the old
statistics until the server restarts. An in-process ANALYZE, by
contrast, is immediately visible to every pooled connection through the
same shared cache.
Move the full-scan Optimize from the scanner pipeline to the scan
controller, which always runs in the server process.
* fix(scanner): honor promoted full scans in the optimize gate
A quick scan resuming an interrupted full scan is promoted inside the
scanner (possibly in a subprocess); mirror the promotion in the
controller so the post-scan ANALYZE isn't skipped.
* refactor: apply cleanup review findings
- drop forceFullRescan's inline ANALYZE: Init already runs a full
ANALYZE after any migration batch with schema changes, so upgrades
including a full-rescan migration analyzed the whole DB twice
- resumingFullScan uses a filtered CountAll instead of fetching and
scanning all libraries
- document why CallScan (CLI) deliberately skips the post-scan Optimize
* perf(db): make planner analysis maintenance resilient
Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans.
* perf(db): avoid analyzing routine quick-scan changes
Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check.
* fix(scan): analyze resumed full scans in CLI
* fix(db): back off failed analysis retries
* feat(db): allow disabling scheduled analysis
* test(db): remove redundant analysis coverage
* refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites
- move query-planner statistics code from db.go to its own optimize.go
(and matching optimize_test.go)
- log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of
repeating the timing block at every call site
- drop the LastDBAnalyzeAttemptAt write on success: it is only read
while failures >= 1, and every failure rewrites it first
- extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner)
helpers
2026-07-13 12:04:29 -04:00
|
|
|
"github.com/navidrome/navidrome/scanner"
|
2025-12-16 16:08:32 -05:00
|
|
|
. "github.com/onsi/ginkgo/v2"
|
|
|
|
|
. "github.com/onsi/gomega"
|
|
|
|
|
)
|
|
|
|
|
|
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* perf(db): keep query planner statistics trustworthy with full ANALYZE
PRAGMA optimize's internal ANALYZE runs with a limited analysis budget
(~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality
indexes: on a 96K-track library it claimed (missing, library_id) narrows
to ~2000 rows when it matches the whole table. The planner then prefers
that index over the sort index and falls back to a full-table temp
B-tree sort per request, turning paginated song listings into
multi-second queries (reproduced at 5.5s on real hardware; ~90x slower
than with correct stats). Every index-creating migration re-triggered
the poisoning via the post-migration optimize, and the daily optimizer
could re-trigger it on large library changes. Setting analysis_limit on
the connection does not help: optimize ignores it.
Run a plain full ANALYZE instead: after migrations with schema changes,
and in db.Optimize (daily schedule and scan-end). Stats are stored in
the database file, so one connection suffices and the per-connection
pool loop is gone. The Optimize call at shutdown is removed: stats are
maintained at migration/scan/daily points, and an ANALYZE during
shutdown only delays it and races container stop timeouts.
* perf(db): drop startup PRAGMA optimize that re-poisons planner stats
The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal
ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for
low-cardinality indexes -- the exact statistics-poisoning this PR set out to
eliminate. Because DevOptimizeDB defaults to true, a restart with no pending
migrations would re-poison the planner until the next scan or daily Optimize.
Remove it: statistics are already refreshed with a full ANALYZE after
schema-changing migrations (Init) and via Optimize at scan-end and on the daily
schedule, so nothing on the startup path needs to touch them. Also clarify that
Optimize is a no-op unless DevOptimizeDB is enabled.
* chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans
The flag only gated the optimize/ANALYZE maintenance calls and there is
no reason to leave planner statistics unmaintained; the guards are gone
along with the flag. The scan-end Optimize now runs only after full
scans — quick scans barely move the statistics, and the daily schedule
covers drift.
* style(scanner): drop redundant comment in runOptimize
* chore(persistence): drop the no-op PRAGMA optimize from ScanEnd
Mask 0x10000 only selects candidate tables by size change; without the
0x02 action bit optimize does nothing (verified: sqlite_stat1 stays
stale after a 100x table growth). The scan-end statistics refresh is
db.Optimize's full ANALYZE, and the expression-collation-index concern
the old comment guarded against no longer applies.
* fix(scanner): run the post-scan ANALYZE in the server process
With the external scanner (the default), the scan pipeline runs in a
subprocess, so its ANALYZE was invisible to the server: SQLite loads
sqlite_stat1 into the process's shared schema cache, and an ANALYZE from
another process does not refresh it — verified with the production DSN
that even brand-new pool connections keep planning with the old
statistics until the server restarts. An in-process ANALYZE, by
contrast, is immediately visible to every pooled connection through the
same shared cache.
Move the full-scan Optimize from the scanner pipeline to the scan
controller, which always runs in the server process.
* fix(scanner): honor promoted full scans in the optimize gate
A quick scan resuming an interrupted full scan is promoted inside the
scanner (possibly in a subprocess); mirror the promotion in the
controller so the post-scan ANALYZE isn't skipped.
* refactor: apply cleanup review findings
- drop forceFullRescan's inline ANALYZE: Init already runs a full
ANALYZE after any migration batch with schema changes, so upgrades
including a full-rescan migration analyzed the whole DB twice
- resumingFullScan uses a filtered CountAll instead of fetching and
scanning all libraries
- document why CallScan (CLI) deliberately skips the post-scan Optimize
* perf(db): make planner analysis maintenance resilient
Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans.
* perf(db): avoid analyzing routine quick-scan changes
Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check.
* fix(scan): analyze resumed full scans in CLI
* fix(db): back off failed analysis retries
* feat(db): allow disabling scheduled analysis
* test(db): remove redundant analysis coverage
* refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites
- move query-planner statistics code from db.go to its own optimize.go
(and matching optimize_test.go)
- log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of
repeating the timing block at every call site
- drop the LastDBAnalyzeAttemptAt write on success: it is only read
while failures >= 1, and every failure rewrites it first
- extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner)
helpers
2026-07-13 12:04:29 -04:00
|
|
|
var _ = Describe("trackScanInteractively", func() {
|
|
|
|
|
It("reports changes and scan errors", func() {
|
|
|
|
|
progress := make(chan *scanner.ProgressInfo, 2)
|
|
|
|
|
progress <- &scanner.ProgressInfo{ChangesDetected: true}
|
|
|
|
|
progress <- &scanner.ProgressInfo{Error: "scan failed"}
|
|
|
|
|
close(progress)
|
|
|
|
|
|
|
|
|
|
changesDetected, err := trackScanInteractively(context.Background(), progress)
|
|
|
|
|
Expect(changesDetected).To(BeTrue())
|
|
|
|
|
Expect(err).To(MatchError("scan failed"))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
2025-12-16 16:08:32 -05:00
|
|
|
var _ = Describe("readTargetsFromFile", func() {
|
|
|
|
|
var tempDir string
|
|
|
|
|
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
var err error
|
|
|
|
|
tempDir, err = os.MkdirTemp("", "navidrome-test-")
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
AfterEach(func() {
|
|
|
|
|
os.RemoveAll(tempDir)
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("reads valid targets from file", func() {
|
|
|
|
|
filePath := filepath.Join(tempDir, "targets.txt")
|
|
|
|
|
content := "1:Music/Rock\n2:Music/Jazz\n3:Classical\n"
|
|
|
|
|
err := os.WriteFile(filePath, []byte(content), 0600)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
targets, err := readTargetsFromFile(filePath)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(targets).To(HaveLen(3))
|
|
|
|
|
Expect(targets[0]).To(Equal(model.ScanTarget{LibraryID: 1, FolderPath: "Music/Rock"}))
|
|
|
|
|
Expect(targets[1]).To(Equal(model.ScanTarget{LibraryID: 2, FolderPath: "Music/Jazz"}))
|
|
|
|
|
Expect(targets[2]).To(Equal(model.ScanTarget{LibraryID: 3, FolderPath: "Classical"}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("skips empty lines", func() {
|
|
|
|
|
filePath := filepath.Join(tempDir, "targets.txt")
|
|
|
|
|
content := "1:Music/Rock\n\n2:Music/Jazz\n\n"
|
|
|
|
|
err := os.WriteFile(filePath, []byte(content), 0600)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
targets, err := readTargetsFromFile(filePath)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(targets).To(HaveLen(2))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("trims whitespace", func() {
|
|
|
|
|
filePath := filepath.Join(tempDir, "targets.txt")
|
|
|
|
|
content := " 1:Music/Rock \n\t2:Music/Jazz\t\n"
|
|
|
|
|
err := os.WriteFile(filePath, []byte(content), 0600)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
targets, err := readTargetsFromFile(filePath)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(targets).To(HaveLen(2))
|
|
|
|
|
Expect(targets[0].FolderPath).To(Equal("Music/Rock"))
|
|
|
|
|
Expect(targets[1].FolderPath).To(Equal("Music/Jazz"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns error for non-existent file", func() {
|
|
|
|
|
_, err := readTargetsFromFile("/nonexistent/file.txt")
|
|
|
|
|
Expect(err).To(HaveOccurred())
|
|
|
|
|
Expect(err.Error()).To(ContainSubstring("failed to open target file"))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns error for invalid target format", func() {
|
|
|
|
|
filePath := filepath.Join(tempDir, "targets.txt")
|
|
|
|
|
content := "invalid-format\n"
|
|
|
|
|
err := os.WriteFile(filePath, []byte(content), 0600)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
_, err = readTargetsFromFile(filePath)
|
|
|
|
|
Expect(err).To(HaveOccurred())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles mixed valid and empty lines", func() {
|
|
|
|
|
filePath := filepath.Join(tempDir, "targets.txt")
|
|
|
|
|
content := "\n1:Music/Rock\n\n\n2:Music/Jazz\n\n"
|
|
|
|
|
err := os.WriteFile(filePath, []byte(content), 0600)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
targets, err := readTargetsFromFile(filePath)
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(targets).To(HaveLen(2))
|
|
|
|
|
})
|
|
|
|
|
})
|