diff --git a/internal/daemon/compact.go b/internal/daemon/compact.go index d8add997..1f4190d5 100644 --- a/internal/daemon/compact.go +++ b/internal/daemon/compact.go @@ -19,6 +19,9 @@ const ( compactEvery = 3 * time.Hour // compactPace is a pause between snapshots, as for the daily check. compactPace = 50 * time.Millisecond + // dbCompactFirst is when the database file first gives its unused space + // back (see daemon.go), once the start's own work is done. + dbCompactFirst = 2 * time.Minute ) // CompactSnapshots has every snapshot that can share its large files with diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index dde8a586..1948c955 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -386,6 +386,37 @@ func (d *Daemon) Start() error { } }) + // The database file gives back the space of what was deleted from it + // (store/compact.go): shortly after start, out of the way of everything a + // start does, and hourly after that. A database from an older build is + // switched over first, once: a rewrite of the whole file, which in + // store.Open would have held up the first start after updating. + d.P2P.GoSync(func(ctx context.Context) { + select { + case <-ctx.Done(): + return + case <-time.After(dbCompactFirst): + } + began := time.Now() + if converted, err := d.Store.EnableIncrementalVacuum(); err != nil { + d.Log.Log("warn", fmt.Sprintf("the database could not be set to give unused space back to the disk: %v", err)) + } else if converted { + d.Log.Log("info", fmt.Sprintf("the database now gives unused space back to the disk (a one-time rewrite, %s)", + time.Since(began).Round(100*time.Millisecond))) + } + d.Store.Compact() + ticker := time.NewTicker(time.Hour) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + d.Store.Compact() + } + } + }) + // Look for newly installed games: a few minutes after start, then every // hour. See newgames.go. d.P2P.GoSync(func(ctx context.Context) { diff --git a/internal/p2p/syncengine/hold.go b/internal/p2p/syncengine/hold.go index 43a63a87..ebcf4ba9 100644 --- a/internal/p2p/syncengine/hold.go +++ b/internal/p2p/syncengine/hold.go @@ -416,8 +416,8 @@ func (e *Engine) PutBack(gameID string) (fetching int, err error) { } for root, files := range hold.Files() { var missing []string + _ = e.Store.ClearDeletedFiles(gameID, root, files) for _, f := range files { - _ = e.Store.ClearDeletedFile(gameID, root, f) if _, ok := x.present[root][f]; !ok { missing = append(missing, f) } diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index 694a6ab7..6ae96fdb 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -1243,14 +1243,16 @@ func (m *Manager) recordDeletionsSince(gameID, branch, snapshotID string, captur if err != nil || len(records) == 0 { continue } + var back []string for _, f := range captured { if f.Root != root { continue } if _, remembered := records[f.Path]; remembered { - _ = m.Store.ClearDeletedFile(gameID, root, f.Path) + back = append(back, f.Path) } } + _ = m.Store.ClearDeletedFiles(gameID, root, back) } } diff --git a/internal/store/compact.go b/internal/store/compact.go new file mode 100644 index 00000000..c5a48cf6 --- /dev/null +++ b/internal/store/compact.go @@ -0,0 +1,73 @@ +package store + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/opensave/opensave/internal/fsx" +) + +// Giving the space of deleted rows back to the disk. +// +// SQLite keeps the pages rows were deleted from, for rows to come; the file +// never shrinks on its own. Snapshots are merged, pruned and replaced all the +// time, and each held a row per file of its save — a quarter of a million +// for one game — so the file grew by what every snapshot ever deleted had +// held: 663 MB on one device, 262 MB of it unused. +// +// With auto_vacuum set to INCREMENTAL the free pages can be handed back a +// few at a time (Compact), never holding the database for long. A database +// being created gets the setting for free: it takes effect only on an empty +// file, so openDB asks for it before anything else. One created by an older +// build needs a full VACUUM to take it (EnableIncrementalVacuum), which the +// daemon runs once in the background after it has started, not in Open: on a +// large database that is a rewrite of some seconds, and the first start after +// updating would wait for it. + +// freePagesWorthCompacting is how many free pages are given back at a time, +// and fewer are not worth the bother: 4 MB at the default page size. +const freePagesWorthCompacting = 1024 + +func (s *Store) incrementalVacuum() bool { + var mode int + return s.db.Get(&mode, `PRAGMA auto_vacuum`) == nil && mode == 2 // 2 = INCREMENTAL +} + +// EnableIncrementalVacuum switches a database created by an older build to +// incremental auto-vacuum, with the one full VACUUM that takes. It reports +// whether it did; on a database that already has it, it does nothing. +// +// The VACUUM rewrites the whole file: it holds the database while it runs and +// needs about the file's size free on the same disk for the copy. Without +// that much free it is not attempted, and the error says so. +func (s *Store) EnableIncrementalVacuum() (bool, error) { + if s.incrementalVacuum() { + return false, nil + } + if s.path != "" { + if info, err := os.Stat(s.path); err == nil { + if free, ok := fsx.FreeBytes(filepath.Dir(s.path)); ok && free < uint64(info.Size()) { + return false, fmt.Errorf("giving unused space back needs about %d MB free next to the database, and %d MB are", + info.Size()>>20, free>>20) + } + } + } + if _, err := s.db.Exec(`PRAGMA auto_vacuum = INCREMENTAL`); err != nil { + return false, err + } + if _, err := s.db.Exec(`VACUUM`); err != nil { + return false, err + } + return true, nil +} + +// Compact hands the database file's unused pages back to the disk, when +// there are enough of them to matter. +func (s *Store) Compact() { + var free int + if err := s.db.Get(&free, `PRAGMA freelist_count`); err != nil || free < freePagesWorthCompacting { + return + } + _, _ = s.db.Exec(`PRAGMA incremental_vacuum`) +} diff --git a/internal/store/compact_test.go b/internal/store/compact_test.go new file mode 100644 index 00000000..f334b628 --- /dev/null +++ b/internal/store/compact_test.go @@ -0,0 +1,137 @@ +package store + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/jmoiron/sqlx" +) + +// Deleted rows give their space back to the disk: the file shrinks, rather +// than holding everything it ever held. +func TestCompactGivesSpaceBack(t *testing.T) { + path := filepath.Join(t.TempDir(), "opensave.db") + s, err := Open(path) + if err != nil { + t.Fatal(err) + } + var mode int + _ = s.db.Get(&mode, `PRAGMA auto_vacuum`) + if mode != 2 { + t.Fatalf("auto_vacuum = %d, want incremental (2)", mode) + } + // A table of the test's own, so the test depends on no other. + if _, err := s.db.Exec(`CREATE TABLE padding (k TEXT PRIMARY KEY, v TEXT NOT NULL)`); err != nil { + t.Fatal(err) + } + pad := strings.Repeat("x", 2000) + tx := s.db.MustBegin() + for i := 0; i < 4000; i++ { + tx.MustExec(`INSERT INTO padding (k, v) VALUES (?, ?)`, fmt.Sprint("k", i), pad) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + _, _ = s.db.Exec(`PRAGMA wal_checkpoint(TRUNCATE)`) + full, _ := os.Stat(path) + if _, err := s.db.Exec(`DELETE FROM padding`); err != nil { + t.Fatal(err) + } + s.Compact() + _, _ = s.db.Exec(`PRAGMA wal_checkpoint(TRUNCATE)`) + after, _ := os.Stat(path) + if after.Size() >= full.Size()/2 { + t.Errorf("file %d bytes after deleting everything, %d before: nothing given back", after.Size(), full.Size()) + } + s.Close() +} + +// Clearing a large save's deletion records is one transaction, not one write +// to disk per file. +func TestClearDeletedFilesIsOneTransaction(t *testing.T) { + s, err := Open(filepath.Join(t.TempDir(), "opensave.db")) + if err != nil { + t.Fatal(err) + } + defer s.Close() + if err := s.CreateGame(Game{ID: "g", Name: "g", SavePath: t.TempDir(), ActiveBranch: "main"}); err != nil { + t.Fatal(err) + } + var files []DeletedFile + var paths []string + for i := 0; i < 50000; i++ { + p := fmt.Sprintf("map/%d/%d.bin", i/100, i) + files = append(files, DeletedFile{Path: p, Hash: "h"}) + paths = append(paths, p) + } + if err := s.RecordDeletedFiles("g", files); err != nil { + t.Fatal(err) + } + start := time.Now() + if err := s.ClearDeletedFiles("g", "", paths); err != nil { + t.Fatal(err) + } + took := time.Since(start) + left, _ := s.DeletedFiles("g", "") + if len(left) != 0 { + t.Errorf("%d records left", len(left)) + } + // Generous: CI runs this under the race detector on shared runners, where + // one statement per file took 10s; what this guards against took minutes. + if took > 30*time.Second { + t.Errorf("clearing 50,000 records took %v", took) + } + t.Logf("cleared 50,000 deletion records in %v", took) +} + +// The database writes through a write-ahead log, not a rollback journal +// created and deleted for every write. +func TestDatabaseUsesWAL(t *testing.T) { + s, err := Open(filepath.Join(t.TempDir(), "opensave.db")) + if err != nil { + t.Fatal(err) + } + defer s.Close() + var mode string + if err := s.db.Get(&mode, `PRAGMA journal_mode`); err != nil || mode != "wal" { + t.Errorf("journal_mode = %q (%v), want wal", mode, err) + } +} + +// A database created by an older build, without incremental auto-vacuum, is +// not rewritten when it is opened: the start stays fast. It is switched over +// once, by EnableIncrementalVacuum, and after that gives space back as usual. +func TestOlderDatabaseIsSwitchedOverAfterOpenNotDuringIt(t *testing.T) { + path := filepath.Join(t.TempDir(), "opensave.db") + old, err := sqlx.Open("sqlite", "file:"+path) // as an older build left it: no auto-vacuum + if err != nil { + t.Fatal(err) + } + if _, err := old.Exec(`CREATE TABLE older_build (k TEXT)`); err != nil { + t.Fatal(err) + } + old.Close() + + s, err := Open(path) + if err != nil { + t.Fatal(err) + } + defer s.Close() + if s.incrementalVacuum() { + t.Fatal("Open rewrote an older database; that is for after the start") + } + converted, err := s.EnableIncrementalVacuum() + if err != nil || !converted { + t.Fatalf("EnableIncrementalVacuum = %v, %v; want it switched over", converted, err) + } + if !s.incrementalVacuum() { + t.Fatal("auto_vacuum is still not incremental") + } + if converted, err := s.EnableIncrementalVacuum(); err != nil || converted { + t.Errorf("a second EnableIncrementalVacuum = %v, %v; want nothing to do", converted, err) + } +} diff --git a/internal/store/db.go b/internal/store/db.go index bcdbfd2f..71407db6 100644 --- a/internal/store/db.go +++ b/internal/store/db.go @@ -23,7 +23,8 @@ var migrationsFS embed.FS // methods (see settings.go, games.go, branches.go, snapshots.go, peers.go, // cloudtokens.go). type Store struct { - db *sqlx.DB + db *sqlx.DB + path string // the database file, for EnableIncrementalVacuum's space check } // Open creates (if needed) and opens the SQLite database at path, applying @@ -34,7 +35,7 @@ func Open(path string) (*Store, error) { return nil, err } - s := &Store{db: db} + s := &Store{db: db, path: path} if err := s.migrate(); err != nil { db.Close() return nil, err @@ -55,7 +56,18 @@ func openDB(path string) (*sqlx.DB, error) { // _pragma params ensure foreign keys are enforced (SQLite defaults them // off per-connection) and busy_timeout avoids spurious SQLITE_BUSY // errors from the watcher/api/p2p goroutines all touching the DB. - dsn := fmt.Sprintf("file:%s?_pragma=foreign_keys(1)&_pragma=busy_timeout(5000)", path) + // + // WAL: a write is appended to one log and the database file is updated + // from it later, rather than every write creating, flushing and deleting a + // rollback journal of its own — which made a quarter of a million small + // writes take most of an hour. synchronous NORMAL is what WAL is meant to + // run with: the database stays whole whatever happens; a power cut can + // only lose the last moments' writes, never corrupt it. + // + // auto_vacuum INCREMENTAL comes first: it takes effect only while the + // file is still empty, before WAL writes its header (see compact.go). On + // an existing database it changes nothing. + dsn := fmt.Sprintf("file:%s?_pragma=auto_vacuum(2)&_pragma=foreign_keys(1)&_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)&_pragma=synchronous(NORMAL)", path) db, err := sqlx.Open("sqlite", dsn) if err != nil { return nil, fmt.Errorf("open sqlite database: %w", err) diff --git a/internal/store/deleted_files.go b/internal/store/deleted_files.go index 1cda3429..0a4a0e0e 100644 --- a/internal/store/deleted_files.go +++ b/internal/store/deleted_files.go @@ -107,6 +107,41 @@ func (s *Store) ClearDeletedFile(gameID, root, path string) error { return nil } +// ClearDeletedFiles forgets the deletions recorded for paths of one save +// location, in one transaction: one at a time, each its own write to disk, +// the quarter of a million of a large save took a quarter of an hour. Paths go +// clearDeletedChunk at a time, so that is a few hundred statements, not one +// per file. +func (s *Store) ClearDeletedFiles(gameID, root string, paths []string) error { + if len(paths) == 0 { + return nil + } + tx, err := s.db.Beginx() + if err != nil { + return err + } + defer tx.Rollback() + for len(paths) > 0 { + n := min(len(paths), clearDeletedChunk) + args := make([]any, 0, n+2) + args = append(args, gameID, root) + for _, p := range paths[:n] { + args = append(args, p) + } + q := `DELETE FROM deleted_files WHERE game_id = ? AND root = ? AND path IN (?` + + strings.Repeat(",?", n-1) + `)` + if _, err := tx.Exec(q, args...); err != nil { + return fmt.Errorf("clear deletion records: %w", err) + } + paths = paths[n:] + } + return tx.Commit() +} + +// clearDeletedChunk is how many paths one statement of ClearDeletedFiles +// names: well under SQLite's limit on bound parameters. +const clearDeletedChunk = 500 + // ClearDeletedFilesForGame forgets every deletion recorded for a game, for use // when it is untracked — the records describe a folder this device no longer // has an opinion about.