VACUUM snapshot metadata through the sqlite driver, not a CLI (closes #120)
check / check (pull_request) Failing after 1s
check / check (pull_request) Failing after 1s
snapshot create vacuumed the exported metadata database by shelling out to a sqlite3 binary, so a backup failed at the very end on any host without that CLI. vacuumDatabase now opens the database with the modernc.org/sqlite driver and runs VACUUM through it, outside any transaction; internal/snapshot no longer imports os/exec. The database opens in WAL mode, so the close after VACUUM checkpoints the rewrite into the main file that is compressed and uploaded. A new test deletes marked rows, vacuums, and asserts the file shrank and no longer holds the deleted bytes. script/bootstrap and the Dockerfile stop installing the CLI in both the test-build and the shipped runtime stage, so the image no longer carries the dependency this change removes. Model: opus-4-8
This commit is contained in:
@@ -44,7 +44,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -669,14 +668,31 @@ func (sm *SnapshotManager) collectCleanupStats(
|
||||
|
||||
// vacuumDatabase runs VACUUM on the database to remove deleted data and compact
|
||||
// This is critical for security - ensures no stale/deleted data pages are uploaded
|
||||
//
|
||||
// VACUUM runs through the modernc.org/sqlite driver, on a freshly opened
|
||||
// connection with no transaction in flight (VACUUM cannot run inside one).
|
||||
// The database opens in WAL mode, so VACUUM's rewrite lands in the WAL; the
|
||||
// checkpoint on Close flushes it into the main file, which is the file we
|
||||
// then compress and upload.
|
||||
func (sm *SnapshotManager) vacuumDatabase(ctx context.Context, dbPath string) error {
|
||||
log.Debug("Running VACUUM on database", "path", dbPath)
|
||||
//nolint:gosec // G204: fixed argv; dbPath is our own temp file path
|
||||
cmd := exec.CommandContext(ctx, "sqlite3", dbPath, "VACUUM;")
|
||||
|
||||
output, err := cmd.CombinedOutput()
|
||||
db, err := database.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("running VACUUM: %w (output: %s)", err, string(output))
|
||||
return fmt.Errorf("opening database for VACUUM: %w", err)
|
||||
}
|
||||
|
||||
defer func() {
|
||||
cerr := db.Close()
|
||||
if cerr != nil {
|
||||
log.Debug("Failed to close database after VACUUM",
|
||||
"path", dbPath, "error", cerr)
|
||||
}
|
||||
}()
|
||||
|
||||
_, err = db.ExecWithLog(ctx, "VACUUM")
|
||||
if err != nil {
|
||||
return fmt.Errorf("running VACUUM: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package snapshot
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"io"
|
||||
@@ -96,6 +97,97 @@ func verifyCleanedDB(
|
||||
}
|
||||
}
|
||||
|
||||
// TestVacuumDatabaseRemovesDeletedData proves the export path uploads a
|
||||
// compacted database: after rows carrying a recognizable marker are deleted
|
||||
// and vacuumDatabase runs, no page holding that marker survives in the file
|
||||
// on disk (the file compressFile later reads for upload).
|
||||
func TestVacuumDatabaseRemovesDeletedData(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
t.Parallel()
|
||||
|
||||
ctx := context.Background()
|
||||
fs := afero.NewOsFs()
|
||||
|
||||
tempDir := t.TempDir()
|
||||
dbPath := filepath.Join(tempDir, "snapshot.db")
|
||||
|
||||
db, err := database.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create database: %v", err)
|
||||
}
|
||||
|
||||
// A marker distinctive enough that its presence in the raw file can only
|
||||
// come from the rows inserted below.
|
||||
marker := []byte("VACUUM_PROBE_DEADBEEF_DELETED_ROW")
|
||||
payload := bytes.Repeat(marker, 128) // ~4 KiB per row
|
||||
|
||||
_, err = db.Conn().ExecContext(ctx,
|
||||
"CREATE TABLE vacuum_probe (id INTEGER PRIMARY KEY, payload BLOB)")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create probe table: %v", err)
|
||||
}
|
||||
|
||||
for range 512 {
|
||||
_, err = db.Conn().ExecContext(ctx,
|
||||
"INSERT INTO vacuum_probe (payload) VALUES (?)", payload)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to insert probe row: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
_, err = db.Conn().ExecContext(ctx, "DELETE FROM vacuum_probe")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to delete probe rows: %v", err)
|
||||
}
|
||||
|
||||
// Close so the deletes reach the main file, mirroring the state
|
||||
// prepareExportDB hands to vacuumDatabase.
|
||||
err = db.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to close database: %v", err)
|
||||
}
|
||||
|
||||
beforeInfo, err := fs.Stat(dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to stat database before vacuum: %v", err)
|
||||
}
|
||||
|
||||
beforeBytes, err := afero.ReadFile(fs, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read database before vacuum: %v", err)
|
||||
}
|
||||
|
||||
if !bytes.Contains(beforeBytes, marker) {
|
||||
t.Fatalf("expected deleted-row data to linger before vacuum")
|
||||
}
|
||||
|
||||
sm := &SnapshotManager{fs: fs}
|
||||
|
||||
err = sm.vacuumDatabase(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("vacuumDatabase failed: %v", err)
|
||||
}
|
||||
|
||||
afterBytes, err := afero.ReadFile(fs, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read database after vacuum: %v", err)
|
||||
}
|
||||
|
||||
if bytes.Contains(afterBytes, marker) {
|
||||
t.Fatalf("deleted-row data survived vacuum in the uploaded file")
|
||||
}
|
||||
|
||||
afterInfo, err := fs.Stat(dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to stat database after vacuum: %v", err)
|
||||
}
|
||||
|
||||
if afterInfo.Size() >= beforeInfo.Size() {
|
||||
t.Fatalf("expected vacuum to shrink the file: before=%d after=%d",
|
||||
beforeInfo.Size(), afterInfo.Size())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanSnapshotDBEmptySnapshot(t *testing.T) {
|
||||
// Initialize logger
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
Reference in New Issue
Block a user