fix: a new server takes a backup after its first scan, and restarts to load the restored settings (#6786)
release / govulncheck (push) Successful in 16s
release / go (push) Failing after 49s
release / web (push) Successful in 1m9s
release / integration (push) Failing after 4m14s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 4m38s
release / Build signed APK (push) Canceled after 4m38s
release / govulncheck (push) Successful in 16s
release / go (push) Failing after 49s
release / web (push) Successful in 1m9s
release / integration (push) Failing after 4m14s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 4m38s
release / Build signed APK (push) Canceled after 4m38s
A one-off end-to-end run of the :dev image (scratch Postgres, generated FLAC files, no outbound network) found that the web restore, done the way docs/hosting.md describes, was refused with not_empty: a new server's startup scan fills the library before anyone signs up, and Restore counted tracks as occupied. The command-line restore passed the same run. - Restore now asks only that the instance has no accounts. The library the first scan found is replaced by the backup's (every foreign key into artists/albums/tracks cascades or sets null), and the restore locks the library tables so a running scan waits rather than adding to them. - Settings are read once at boot by about ten services, the library folders among them, so after a web restore the server ran on its defaults until restarted. The restore endpoint now answers, then asks the process to restart in place (syscall.Exec of the same binary, arguments and environment), which needs no container restart policy. The startup scan of the restarted server replaces the scan the endpoint used to start. - The restore form says Minstrel is restarting; the docs say a scanned new server still takes a backup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -189,7 +189,16 @@ func TestARestoredInstanceHoldsWhatTheBackupHeld_Integration(t *testing.T) {
|
||||
t.Fatalf("restore from a newer schema: err = %v, want ErrNewerSchema", err)
|
||||
}
|
||||
|
||||
mustExec(t, pool, "TRUNCATE users, artists, library_changes CASCADE")
|
||||
// A new server scans its music before anyone signs up, so it already has
|
||||
// a library, here with a track the backup doesn't know. It still takes
|
||||
// the backup: the backup's library replaces what the scan found, which the
|
||||
// next scan finds again under the backup's ids.
|
||||
mustExec(t, pool, "TRUNCATE users CASCADE")
|
||||
scannedArtist := mustID(t, pool, `INSERT INTO artists (name, sort_name) VALUES ('Scanned', 'scanned') RETURNING id::text`)
|
||||
scannedAlbum := mustID(t, pool, `INSERT INTO albums (title, sort_title, artist_id) VALUES ('Scanned', 'scanned', $1) RETURNING id::text`, scannedArtist)
|
||||
scanned := mustID(t, pool, `INSERT INTO tracks (title, album_id, artist_id, duration_ms, file_path, file_size, file_format)
|
||||
VALUES ('Scanned', $1, $2, 1000, '/music/b.flac', 10, 'flac') RETURNING id::text`, scannedAlbum, scannedArtist)
|
||||
mustExec(t, pool, `INSERT INTO library_changes (entity_type, entity_id, op) VALUES ('track', $1, 'upsert')`, scanned)
|
||||
if err := backup.Restore(ctx, pool, first); err != nil {
|
||||
t.Fatalf("restore: %v", err)
|
||||
}
|
||||
|
||||
+14
-13
@@ -13,9 +13,8 @@ import (
|
||||
)
|
||||
|
||||
var (
|
||||
// ErrNotEmpty is a restore into an instance that already has accounts or
|
||||
// a library.
|
||||
ErrNotEmpty = errors.New("backup: this Minstrel already has accounts or a library; a backup restores into an empty one")
|
||||
// ErrNotEmpty is a restore into an instance that already has accounts.
|
||||
ErrNotEmpty = errors.New("backup: this Minstrel already has accounts; a backup restores into a new one")
|
||||
|
||||
// ErrNewerSchema is a backup written by a newer Minstrel than this one.
|
||||
ErrNewerSchema = errors.New("backup: the backup was written by a newer version of Minstrel; upgrade this one first")
|
||||
@@ -30,10 +29,12 @@ const batchRows = 500
|
||||
// Restore loads a backup into an empty instance, in one transaction: either
|
||||
// all of it lands or none of it does.
|
||||
//
|
||||
// The instance must have no accounts and no tracks. Its other rows in the
|
||||
// tables the backup holds are replaced, which is what a fresh instance's
|
||||
// seeded settings need. A table the backup doesn't hold, because it was
|
||||
// written before that table existed, keeps what the instance has.
|
||||
// The instance must have no accounts. Its rows in the tables the backup holds
|
||||
// are replaced: a fresh instance's seeded settings, and the library its
|
||||
// startup scan found before anyone signed up, which the scan after the
|
||||
// restore finds again under the backup's ids. A table the backup doesn't
|
||||
// hold, because it was written before that table existed, keeps what the
|
||||
// instance has.
|
||||
//
|
||||
// A row's columns are those the backup and the table both have: an older
|
||||
// backup's rows take the defaults of columns added since, and a column dropped
|
||||
@@ -52,15 +53,15 @@ func Restore(ctx context.Context, pool *pgxpool.Pool, c Contents) error {
|
||||
if c.Schema > schema {
|
||||
return ErrNewerSchema
|
||||
}
|
||||
// Taken before the emptiness check, so a first account or a scan starting
|
||||
// at the same moment waits for the restore, then finds the data.
|
||||
if _, err := tx.Exec(ctx, "LOCK TABLE users, tracks IN EXCLUSIVE MODE"); err != nil {
|
||||
// Taken before the emptiness check, so a first account made at the same
|
||||
// moment waits for the restore, then finds the data. A scan still running
|
||||
// waits too, rather than adding to the library while it is replaced.
|
||||
if _, err := tx.Exec(ctx, "LOCK TABLE users, artists, albums, tracks IN EXCLUSIVE MODE"); err != nil {
|
||||
return fmt.Errorf("backup: lock: %w", err)
|
||||
}
|
||||
var occupied bool
|
||||
if err := tx.QueryRow(ctx,
|
||||
"SELECT EXISTS (SELECT 1 FROM users) OR EXISTS (SELECT 1 FROM tracks)").Scan(&occupied); err != nil {
|
||||
return fmt.Errorf("backup: check the instance is empty: %w", err)
|
||||
if err := tx.QueryRow(ctx, "SELECT EXISTS (SELECT 1 FROM users)").Scan(&occupied); err != nil {
|
||||
return fmt.Errorf("backup: check the instance is new: %w", err)
|
||||
}
|
||||
if occupied {
|
||||
return ErrNotEmpty
|
||||
|
||||
Reference in New Issue
Block a user