diff --git a/TODO.md b/TODO.md index 33cafc9..d3915ca 100644 --- a/TODO.md +++ b/TODO.md @@ -28,6 +28,11 @@ The other open issue is https://git.eeqj.de/sneak/routewatch/issues/30. # Completed Steps +- 2026-10-03: a new database is created with `auto_vacuum` set to + incremental, through the connection string so it is set before the file + is first written, and each periodic incremental vacuum now returns up to + 1000 free pages; the late `PRAGMA auto_vacuum` in `Initialize`, which + SQLite ignored, is gone (closes #43) - 2026-10-03: looking up an IP address no longer reads every IPv6 route: both families find the most specific live route with at most 33 or 129 lookups on the prefix index, and the IPv4 range columns are gone. Prefixes diff --git a/internal/database/database.go b/internal/database/database.go index 14da155..6dfddbf 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -91,8 +91,13 @@ func New(cfg *config.Config, logger *logger.Logger) (*Database, error) { // a transaction that reads before writing starts as a reader and, when it // then writes while another connection holds the write lock, fails at once // with "database is locked" without waiting for _busy_timeout. + // _auto_vacuum=incremental lets Vacuum return free pages to the filesystem. + // SQLite only accepts it before the database file is first written, and the + // switch to WAL writes it, so it must be here: the driver applies it on open, + // before _journal_mode. On an existing file it changes nothing. dsn := fmt.Sprintf( - "file:%s?_cache_size=%d&_synchronous=OFF&_busy_timeout=%d&_journal_mode=WAL&_txlock=immediate", + "file:%s?_cache_size=%d&_synchronous=OFF&_busy_timeout=%d"+ + "&_auto_vacuum=incremental&_journal_mode=WAL&_txlock=immediate", dbPath, sqliteCacheSizeKiB, sqliteBusyTimeoutMs, @@ -137,9 +142,8 @@ func (d *Database) Initialize() error { // heap. The heap limits below are process-wide, so setting them once here is // enough for the whole pool. pragmas := []string{ - "PRAGMA journal_mode=WAL", // Write-Ahead Logging - "PRAGMA analysis_limit=0", // Disable automatic ANALYZE - "PRAGMA auto_vacuum=INCREMENTAL", // Enable incremental vacuum + "PRAGMA journal_mode=WAL", // Write-Ahead Logging + "PRAGMA analysis_limit=0", // Disable automatic ANALYZE fmt.Sprintf("PRAGMA soft_heap_limit=%d", sqliteSoftHeapLimitBytes), fmt.Sprintf("PRAGMA hard_heap_limit=%d", sqliteHardHeapLimitBytes), } @@ -1857,10 +1861,20 @@ func (d *Database) Vacuum(ctx context.Context) error { // Free up to 1000 pages per call (~4MB with default 4KB page size) // This keeps each vacuum operation quick and non-blocking const pagesToFree = 1000 - _, err := d.db.ExecContext(ctx, fmt.Sprintf("PRAGMA incremental_vacuum(%d)", pagesToFree)) + rows, err := d.db.QueryContext(ctx, fmt.Sprintf("PRAGMA incremental_vacuum(%d)", pagesToFree)) if err != nil { return fmt.Errorf("failed to run incremental vacuum: %w", err) } + defer func() { _ = rows.Close() }() + + // SQLite frees one page each time the statement steps, and each step + // returns a row, so every row must be read for the pragma to free more + // than one page. + for rows.Next() { + } + if err := rows.Err(); err != nil { + return fmt.Errorf("failed to run incremental vacuum: %w", err) + } return nil } diff --git a/internal/database/database_test.go b/internal/database/database_test.go index e15ec1a..82753a9 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "fmt" "net/netip" "sync" "testing" @@ -18,6 +19,13 @@ import ( // memory"; the DSN change must leave temp_store below this so they spill to disk. const tempStoreMemory = 2 +// autoVacuumIncremental is the PRAGMA auto_vacuum value meaning "incremental". +const autoVacuumIncremental = 2 + +// vacuumTestRoutes is how many routes the vacuum test writes and then deletes, +// enough to leave many free pages in the file. +const vacuumTestRoutes = 2000 + // heldConnections is how many pooled connections the pragma test holds open at // once so each is a distinct SQLite connection that parsed the DSN. const heldConnections = 5 @@ -127,7 +135,8 @@ func TestGetIPInfoFindsMostSpecificLiveRoute(t *testing.T) { // TestConnectionPoolPragmas holds several pooled connections open at once and // checks each one carries the per-connection settings from the DSN, plus the -// process-wide hard heap limit. +// process-wide hard heap limit, and sees the new file with auto_vacuum +// incremental. func TestConnectionPoolPragmas(t *testing.T) { cfg := &config.Config{StateDir: t.TempDir()} @@ -188,6 +197,74 @@ func TestConnectionPoolPragmas(t *testing.T) { if hardHeapLimit != sqliteHardHeapLimitBytes { t.Errorf("conn %d: hard_heap_limit = %d, want %d", i, hardHeapLimit, sqliteHardHeapLimitBytes) } + + var autoVacuum int + if err := c.QueryRowContext(ctx, "PRAGMA auto_vacuum").Scan(&autoVacuum); err != nil { + t.Fatalf("conn %d: failed to read auto_vacuum: %v", i, err) + } + if autoVacuum != autoVacuumIncremental { + t.Errorf("conn %d: auto_vacuum = %d, want %d (incremental)", i, autoVacuum, autoVacuumIncremental) + } + } +} + +// TestVacuumReturnsFreePages checks that after routes are deleted from a new +// database, one Vacuum call returns every page they used. The deletes leave +// fewer free pages than the 1000 Vacuum frees per call, so none may remain. +// With auto_vacuum off (issue https://git.eeqj.de/sneak/routewatch/issues/43) +// the free pages stayed in the file, and with the PRAGMA run by ExecContext +// Vacuum freed only one page per call. +func TestVacuumReturnsFreePages(t *testing.T) { + cfg := &config.Config{StateDir: t.TempDir()} + + db, err := New(cfg, logger.New()) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + defer func() { _ = db.Close() }() + + ctx := context.Background() + ts := time.Now().UTC() + const asn = 64500 + + routes := make([]*LiveRoute, 0, vacuumTestRoutes) + deletions := make([]LiveRouteDeletion, 0, vacuumTestRoutes) + for i := range vacuumTestRoutes { + route := mkV4Route(t, fmt.Sprintf("10.%d.%d.0/24", i/256, i%256), asn, ts) + routes = append(routes, route) + deletions = append(deletions, LiveRouteDeletion{ + Prefix: route.Prefix, + OriginASN: asn, + PeerIP: route.PeerIP, + IPVersion: ipVersionV4, + }) + } + + if err := db.UpsertLiveRouteBatch(routes); err != nil { + t.Fatalf("failed to write routes: %v", err) + } + if err := db.DeleteLiveRouteBatch(deletions); err != nil { + t.Fatalf("failed to delete routes: %v", err) + } + + var before int + if err := db.db.QueryRowContext(ctx, "PRAGMA freelist_count").Scan(&before); err != nil { + t.Fatalf("failed to read freelist_count: %v", err) + } + if before == 0 { + t.Fatalf("no free pages after deleting %d routes", vacuumTestRoutes) + } + + if err := db.Vacuum(ctx); err != nil { + t.Fatalf("Vacuum failed: %v", err) + } + + var after int + if err := db.db.QueryRowContext(ctx, "PRAGMA freelist_count").Scan(&after); err != nil { + t.Fatalf("failed to read freelist_count: %v", err) + } + if after != 0 { + t.Errorf("free pages after Vacuum = %d of %d, want 0", after, before) } }