VACUUM snapshot metadata through the sqlite driver, not a CLI (closes #120)
snapshot create compacted the metadata database by running a sqlite3 command-line binary, after every blob had already been uploaded. On a host without that binary, which includes anyone who installed with go install, the backup failed at the last step, and two tests failed the same way. VACUUM now runs through the Go sqlite driver the program already uses, and its error is returned to the caller. The runtime Docker image no longer installs the sqlite package, since nothing in the binary calls it. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
This commit was merged in pull request #136.
This commit is contained in:
+5
-3
@@ -22,8 +22,10 @@ FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f
|
|||||||
|
|
||||||
ARG VERSION=dev
|
ARG VERSION=dev
|
||||||
|
|
||||||
# Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests)
|
# Build tooling: make, plus a C toolchain because `go test -race` needs cgo.
|
||||||
RUN apk add --no-cache make build-base sqlite
|
# The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or
|
||||||
|
# CLI is required.
|
||||||
|
RUN apk add --no-cache make build-base
|
||||||
|
|
||||||
WORKDIR /src
|
WORKDIR /src
|
||||||
|
|
||||||
@@ -71,7 +73,7 @@ RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/global
|
|||||||
# alpine:3.21, 2026-02-25
|
# alpine:3.21, 2026-02-25
|
||||||
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
|
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
|
||||||
|
|
||||||
RUN apk add --no-cache ca-certificates sqlite
|
RUN apk add --no-cache ca-certificates
|
||||||
|
|
||||||
# Copy binary from builder
|
# Copy binary from builder
|
||||||
COPY --from=builder /vaultik /usr/local/bin/vaultik
|
COPY --from=builder /vaultik /usr/local/bin/vaultik
|
||||||
|
|||||||
@@ -613,7 +613,6 @@ regardless of color setting (emoji are not color).
|
|||||||
and the pre-commit hook both run it. A `golangci-lint` installed on
|
and the pre-commit hook both run it. A `golangci-lint` installed on
|
||||||
`PATH` is not a substitute and is never used on a host, whatever its
|
`PATH` is not a substitute and is never used on a host, whatever its
|
||||||
version.
|
version.
|
||||||
* `sqlite3` CLI, which the test suite shells out to
|
|
||||||
* S3-compatible object storage (or local filesystem, or rclone remote)
|
* S3-compatible object storage (or local filesystem, or rclone remote)
|
||||||
|
|
||||||
## development workflow
|
## development workflow
|
||||||
@@ -644,8 +643,8 @@ standard: normalized scripts in `script/` are the entrypoints for the
|
|||||||
development workflow, and the Makefile targets are thin shims that call
|
development workflow, and the Makefile targets are thin shims that call
|
||||||
them. We provide:
|
them. We provide:
|
||||||
|
|
||||||
* `script/bootstrap` — install all development dependencies (go, sqlite3,
|
* `script/bootstrap` — install all development dependencies (go, Go
|
||||||
Go module download). It deliberately does not install `golangci-lint`;
|
module download). It deliberately does not install `golangci-lint`;
|
||||||
see `script/lint` below.
|
see `script/lint` below.
|
||||||
* `script/setup` — make a fresh clone ready for development: runs
|
* `script/setup` — make a fresh clone ready for development: runs
|
||||||
`script/bootstrap`, then `script/install-precommit`
|
`script/bootstrap`, then `script/install-precommit`
|
||||||
|
|||||||
@@ -25,6 +25,15 @@ release" is exactly the contradiction
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata
|
||||||
|
database through the `modernc.org/sqlite` driver instead of shelling
|
||||||
|
out to the external `sqlite` command-line binary (issue #120). A
|
||||||
|
backup no longer needs that binary on `PATH`, so `make check` passes
|
||||||
|
on a stock `go install` host; `script/bootstrap` and the `Dockerfile`
|
||||||
|
(both the test-build and the shipped runtime stage) no longer install
|
||||||
|
it, and a new test asserts the uploaded database keeps no pages from
|
||||||
|
deleted rows. Dropped the now-false note on the 2026-08-07 entry below
|
||||||
|
that said bootstrap installs it.
|
||||||
- 2026-09-21: Made `.gitea/workflows/check.yml` run on pushes to `main`
|
- 2026-09-21: Made `.gitea/workflows/check.yml` run on pushes to `main`
|
||||||
and `next` and on pull requests against either, so unit PRs (whose
|
and `next` and on pull requests against either, so unit PRs (whose
|
||||||
base is `next`) and `next` itself get a CI run instead of relying on a
|
base is `next`) and `next` itself get a CI run instead of relying on a
|
||||||
@@ -529,7 +538,7 @@ release" is exactly the contradiction
|
|||||||
was green was wrong.
|
was green was wrong.
|
||||||
- 2026-08-07: Added the standard `.golangci.yml` and `.editorconfig`
|
- 2026-08-07: Added the standard `.golangci.yml` and `.editorconfig`
|
||||||
(issue #59); lint findings under the new config are tracked in issue
|
(issue #59); lint findings under the new config are tracked in issue
|
||||||
#61. `script/bootstrap` now installs sqlite3 (needed by tests).
|
#61.
|
||||||
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
|
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
|
||||||
Makefile shims, README Entrypoints section
|
Makefile shims, README Entrypoints section
|
||||||
- 2026-07-02: Consolidated CLI verbs, retired overlapping commands; bound
|
- 2026-07-02: Consolidated CLI verbs, retired overlapping commands; bound
|
||||||
|
|||||||
+1
-1
@@ -192,7 +192,7 @@ Tracks blob upload metrics.
|
|||||||
After a snapshot is completed:
|
After a snapshot is completed:
|
||||||
1. Copy database to temporary file
|
1. Copy database to temporary file
|
||||||
2. Clean temporary database to contain only current snapshot data
|
2. Clean temporary database to contain only current snapshot data
|
||||||
3. Export to SQL dump using sqlite3
|
3. VACUUM the trimmed database so deleted rows leave no pages behind
|
||||||
4. Compress with zstd and encrypt with age
|
4. Compress with zstd and encrypt with age
|
||||||
5. Upload to S3 as `metadata/{remote-key}/db.zst.age`
|
5. Upload to S3 as `metadata/{remote-key}/db.zst.age`
|
||||||
6. Generate blob manifest and upload as `metadata/{remote-key}/manifest.json.zst`
|
6. Generate blob manifest and upload as `metadata/{remote-key}/manifest.json.zst`
|
||||||
|
|||||||
@@ -44,7 +44,6 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"os/exec"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -669,14 +668,31 @@ func (sm *SnapshotManager) collectCleanupStats(
|
|||||||
|
|
||||||
// vacuumDatabase runs VACUUM on the database to remove deleted data and compact
|
// 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
|
// 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 {
|
func (sm *SnapshotManager) vacuumDatabase(ctx context.Context, dbPath string) error {
|
||||||
log.Debug("Running VACUUM on database", "path", dbPath)
|
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 {
|
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
|
return nil
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
package snapshot
|
package snapshot
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"database/sql"
|
"database/sql"
|
||||||
"io"
|
"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) {
|
func TestCleanSnapshotDBEmptySnapshot(t *testing.T) {
|
||||||
// Initialize logger
|
// Initialize logger
|
||||||
log.Initialize(log.Config{})
|
log.Initialize(log.Config{})
|
||||||
|
|||||||
@@ -114,9 +114,6 @@ main() {
|
|||||||
# from CI. Nothing on the host is ever used as a linter, at any
|
# from CI. Nothing on the host is ever used as a linter, at any
|
||||||
# version, so installing one here would buy nothing.
|
# version, so installing one here would buy nothing.
|
||||||
|
|
||||||
# sqlite3 CLI: the test suite shells out to it (VACUUM).
|
|
||||||
if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi
|
|
||||||
|
|
||||||
# goreleaser, at the version pinned by script/install-goreleaser and
|
# goreleaser, at the version pinned by script/install-goreleaser and
|
||||||
# verified against a hardcoded sha256. Package managers are not used
|
# verified against a hardcoded sha256. Package managers are not used
|
||||||
# for it: they ship whatever version they happen to carry, and the
|
# for it: they ship whatever version they happen to carry, and the
|
||||||
|
|||||||
Reference in New Issue
Block a user