Open the database read-only for report and trees (closes #8)
check / check (push) Successful in 1m17s
check / check (push) Successful in 1m17s
report and trees now connect read-only (mode=ro, query_only, the same busy timeout) and no longer set the journal mode, which is a write. A read-only connection to a WAL database still needs its -wal and -shm files, or write access to the directory to create them, so scan now switches the database back to rollback-journal mode whenever it closes it: between scans the file alone holds the database. If a report has the database open at that moment the switch is refused; scan warns and the database stays in WAL mode, with its -wal and -shm files, until the next scan. README §Database states what readers need. Model: opus-5-5
This commit was merged in pull request #72.
This commit is contained in:
@@ -152,16 +152,28 @@ All three subcommands operate on a single SQLite database file:
|
|||||||
use. `report` and `trees` require an existing database; a missing
|
use. `report` and `trees` require an existing database; a missing
|
||||||
database file is a fatal error (exit 1) telling the user to run
|
database file is a fatal error (exit 1) telling the user to run
|
||||||
`scan` first.
|
`scan` first.
|
||||||
- The database uses WAL journal mode and a busy timeout, so running a
|
- While `scan` runs, the database is in WAL journal mode with a busy
|
||||||
report while a cron `scan` is in progress is safe. The filesystem
|
timeout, so running a report while a cron `scan` is in progress is
|
||||||
is authoritative; the database is an eventually-consistent
|
safe. The filesystem is authoritative; the database is an
|
||||||
reflection of it. Hashed records are committed in batched
|
eventually-consistent reflection of it. Hashed records are
|
||||||
transactions while the scan is still running (keeping the WAL
|
committed in batched transactions while the scan is still running
|
||||||
small and letting concurrent reports observe progress), so a
|
(keeping the WAL small and letting concurrent reports observe
|
||||||
report may see a scan's changes partially applied, and a scan
|
progress), so a report may see a scan's changes partially applied,
|
||||||
that dies partway leaves a valid database holding everything
|
and a scan that dies partway leaves a valid database holding
|
||||||
hashed so far; the next scan skips those records and converges
|
everything hashed so far; the next scan skips those records and
|
||||||
toward the filesystem.
|
converges toward the filesystem.
|
||||||
|
- `scan` switches the database back to rollback-journal mode when it
|
||||||
|
closes it, so between scans the database file alone holds the whole
|
||||||
|
database. Each switch needs the database to itself: a `scan` that
|
||||||
|
starts while a report is still reading waits for it up to the
|
||||||
|
10-second busy timeout, then fails; a `scan` that ends while a
|
||||||
|
report has the database open warns and leaves the database in WAL
|
||||||
|
mode until the next scan.
|
||||||
|
- `report` and `trees` open the database read-only and need only read
|
||||||
|
access to the database file, and no write access to its directory.
|
||||||
|
While the database is in WAL mode they also read the `-wal` and
|
||||||
|
`-shm` files beside it, which SQLite creates with the database
|
||||||
|
file's permissions.
|
||||||
- Schema (`PRAGMA user_version` is the schema version, currently 1; a
|
- Schema (`PRAGMA user_version` is the schema version, currently 1; a
|
||||||
database with any other version is a fatal error):
|
database with any other version is a fatal error):
|
||||||
|
|
||||||
|
|||||||
@@ -29,6 +29,10 @@
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- `report` and `trees` open the database read-only, and `scan` leaves it
|
||||||
|
out of WAL mode, so reading needs only read access (2026-10-03, closes
|
||||||
|
https://git.eeqj.de/sneak/sfdupes/issues/8)
|
||||||
|
|
||||||
- escape tabs, newlines, carriage returns and backslashes in report,
|
- escape tabs, newlines, carriage returns and backslashes in report,
|
||||||
trees and warning paths; the root directory's path is `/`
|
trees and warning paths; the root directory's path is `/`
|
||||||
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7)
|
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7)
|
||||||
|
|||||||
@@ -74,16 +74,24 @@ func databasePath() string {
|
|||||||
return defaultDatabasePath
|
return defaultDatabasePath
|
||||||
}
|
}
|
||||||
|
|
||||||
// openDB opens the SQLite database at path with WAL journaling and a
|
// scanParams are the connection parameters for scan: read-write, with
|
||||||
// busy timeout, so a report can run while a cron scan is in progress.
|
// WAL journaling and a busy timeout, so a report can run while a cron
|
||||||
// It does not create or verify the schema.
|
// scan is in progress. closeScanDatabase leaves WAL mode again.
|
||||||
func openDB(path string) (*sql.DB, error) {
|
const scanParams = "_pragma=busy_timeout(10000)" +
|
||||||
dsn := "file:" + path +
|
"&_pragma=journal_mode(WAL)" +
|
||||||
"?_pragma=busy_timeout(10000)" +
|
"&_pragma=synchronous(NORMAL)"
|
||||||
"&_pragma=journal_mode(WAL)" +
|
|
||||||
"&_pragma=synchronous(NORMAL)"
|
|
||||||
|
|
||||||
db, err := sql.Open("sqlite", dsn)
|
// reportParams are the connection parameters for report and trees:
|
||||||
|
// read-only, with the same busy timeout. They set no journal mode,
|
||||||
|
// because setting one is a write.
|
||||||
|
const reportParams = "mode=ro" +
|
||||||
|
"&_pragma=busy_timeout(10000)" +
|
||||||
|
"&_pragma=query_only(1)"
|
||||||
|
|
||||||
|
// openDB opens the SQLite database at path with the connection
|
||||||
|
// parameters params. It does not create or verify the schema.
|
||||||
|
func openDB(path, params string) (*sql.DB, error) {
|
||||||
|
db, err := sql.Open("sqlite", "file:"+path+"?"+params)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("open database %s: %w", path, err)
|
return nil, fmt.Errorf("open database %s: %w", path, err)
|
||||||
}
|
}
|
||||||
@@ -104,7 +112,7 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
|
|||||||
return nil, fmt.Errorf("create database directory: %w", err)
|
return nil, fmt.Errorf("create database directory: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
db, err := openDB(path)
|
db, err := openDB(path, scanParams)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -119,6 +127,24 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
|
|||||||
return db, nil
|
return db, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// closeScanDatabase switches the database at path from WAL back to
|
||||||
|
// rollback-journal mode and closes it. Out of WAL mode the database
|
||||||
|
// file alone holds the whole database, so a reader needs no -wal or
|
||||||
|
// -shm file beside it, nor write access to create them. The switch
|
||||||
|
// fails while a report has the database open; the database then stays
|
||||||
|
// in WAL mode, still readable, until a later scan closes it.
|
||||||
|
func closeScanDatabase(ctx context.Context, db *sql.DB, path string) {
|
||||||
|
// Runs on the way out of a cancelled scan too.
|
||||||
|
_, err := db.ExecContext(context.WithoutCancel(ctx),
|
||||||
|
"PRAGMA journal_mode = DELETE")
|
||||||
|
if err != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "scan: database %s left in WAL mode: %v\n",
|
||||||
|
path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_ = db.Close()
|
||||||
|
}
|
||||||
|
|
||||||
// openReportDatabase opens an existing database for the report and
|
// openReportDatabase opens an existing database for the report and
|
||||||
// trees subcommands. A missing database file is an error directing the
|
// trees subcommands. A missing database file is an error directing the
|
||||||
// user to run scan first; the schema version must match exactly.
|
// user to run scan first; the schema version must match exactly.
|
||||||
@@ -134,7 +160,7 @@ func openReportDatabase(ctx context.Context,
|
|||||||
return nil, fmt.Errorf("database: %w", err)
|
return nil, fmt.Errorf("database: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
db, err := openDB(path)
|
db, err := openDB(path, reportParams)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
+44
@@ -5,6 +5,7 @@ import (
|
|||||||
"database/sql"
|
"database/sql"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"slices"
|
"slices"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -130,6 +131,49 @@ func TestOpenReportDatabaseOK(t *testing.T) {
|
|||||||
_ = db.Close()
|
_ = db.Close()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCloseScanDatabaseWhileReportOpen(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A report holding the database open stops scan from taking it out
|
||||||
|
// of WAL mode. The -wal and -shm files must then stay beside it, so
|
||||||
|
// that a later report still needs only read access.
|
||||||
|
path := testDBPath(t)
|
||||||
|
|
||||||
|
scanDB, err := openScanDatabase(t.Context(), path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
reportDB, err := openReportDatabase(t.Context(), path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
closeScanDatabase(t.Context(), scanDB, path)
|
||||||
|
|
||||||
|
_ = reportDB.Close()
|
||||||
|
|
||||||
|
_, err = os.Stat(path + "-wal")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("no -wal left: the switch out of WAL mode was not "+
|
||||||
|
"stopped: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
makeReadOnly(t, path)
|
||||||
|
|
||||||
|
reportDB, err = openReportDatabase(t.Context(), path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("openReportDatabase: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
defer func() { _ = reportDB.Close() }()
|
||||||
|
|
||||||
|
_, err = loadFileRows(t.Context(), reportDB)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("loadFileRows: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestApplyChangesRoundTrip(t *testing.T) {
|
func TestApplyChangesRoundTrip(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
+83
-7
@@ -44,6 +44,44 @@ func assertNoSidecars(t *testing.T, path string) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// makeReadOnly takes write permission away from the database at path,
|
||||||
|
// from any WAL sidecar beside it, and from their directory, as for a
|
||||||
|
// user reading a database that a root cron scan keeps. Root ignores
|
||||||
|
// file permissions, so it skips the test when run as root.
|
||||||
|
func makeReadOnly(t *testing.T, path string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
if os.Geteuid() == 0 {
|
||||||
|
t.Skip("root ignores file permissions")
|
||||||
|
}
|
||||||
|
|
||||||
|
err := os.Chmod(path, 0o400)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, suffix := range walSuffixes {
|
||||||
|
err = os.Chmod(path+suffix, 0o400)
|
||||||
|
if err != nil && !errors.Is(err, fs.ErrNotExist) {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
dir := filepath.Dir(path)
|
||||||
|
|
||||||
|
//nolint:gosec // reaching the database needs the search bit
|
||||||
|
err = os.Chmod(dir, 0o500)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Runs before t.TempDir's own cleanup, which must delete the files.
|
||||||
|
t.Cleanup(func() {
|
||||||
|
//nolint:gosec // removing the directory needs its search bit back
|
||||||
|
_ = os.Chmod(dir, 0o700)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// captureStdout redirects os.Stdout to a file for the rest of the test
|
// captureStdout redirects os.Stdout to a file for the rest of the test
|
||||||
// and returns a function reading back everything written to it. Only
|
// and returns a function reading back everything written to it. Only
|
||||||
// machine-readable data belongs on stdout (README design goal 4), so
|
// machine-readable data belongs on stdout (README design goal 4), so
|
||||||
@@ -91,13 +129,13 @@ func captureStdout(t *testing.T) func() string {
|
|||||||
// brokenDatabase writes a database that opens cleanly and passes the
|
// brokenDatabase writes a database that opens cleanly and passes the
|
||||||
// schema-version check but has no files table, so the first query
|
// schema-version check but has no files table, so the first query
|
||||||
// fails with the database already open: a fatal error on a path that
|
// fails with the database already open: a fatal error on a path that
|
||||||
// owns an open database.
|
// owns an open database. It closes the database the way scan does.
|
||||||
func brokenDatabase(t *testing.T) string {
|
func brokenDatabase(t *testing.T) string {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
path := testDBPath(t)
|
path := testDBPath(t)
|
||||||
|
|
||||||
db, err := openDB(path)
|
db, err := openDB(path, scanParams)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
@@ -108,10 +146,7 @@ func brokenDatabase(t *testing.T) string {
|
|||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = db.Close()
|
closeScanDatabase(t.Context(), db, path)
|
||||||
if err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return path
|
return path
|
||||||
}
|
}
|
||||||
@@ -144,7 +179,10 @@ func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
|
|||||||
|
|
||||||
func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
|
func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
|
||||||
// Every subcommand that owns an open database must close it when
|
// Every subcommand that owns an open database must close it when
|
||||||
// it fails: no os.Exit between the open and the return.
|
// it fails: no os.Exit between the open and the return. The
|
||||||
|
// sidecar check is evidence of the close only for scan: report and
|
||||||
|
// trees only read a database that is out of WAL mode, which leaves
|
||||||
|
// nothing on disk whether they close it or not.
|
||||||
cases := map[string][]string{
|
cases := map[string][]string{
|
||||||
cmdScan: {cmdScan},
|
cmdScan: {cmdScan},
|
||||||
cmdReport: {cmdReport},
|
cmdReport: {cmdReport},
|
||||||
@@ -395,3 +433,41 @@ func TestRunTreesSucceeds(t *testing.T) {
|
|||||||
|
|
||||||
assertNoSidecars(t, path)
|
assertNoSidecars(t, path)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
|
||||||
|
// README §Database: report and trees need only read access to the
|
||||||
|
// database file. With its directory read-only as well, SQLite
|
||||||
|
// cannot create any file beside it.
|
||||||
|
path := testDBPath(t)
|
||||||
|
t.Setenv(databaseEnv, path)
|
||||||
|
|
||||||
|
dupes := scanFixture(t)
|
||||||
|
assertNoSidecars(t, path)
|
||||||
|
makeReadOnly(t, path)
|
||||||
|
|
||||||
|
cases := map[string]string{
|
||||||
|
cmdReport: "first\tdupe\tsize\n" +
|
||||||
|
dupes[0] + "\t" + dupes[1] + "\t300\n",
|
||||||
|
cmdTrees: "first\tdupe\tfiles\tsize\n" +
|
||||||
|
filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) +
|
||||||
|
"\t1\t300\n",
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, want := range cases {
|
||||||
|
var stderr bytes.Buffer
|
||||||
|
|
||||||
|
stdout := captureStdout(t)
|
||||||
|
|
||||||
|
code := run([]string{name}, &stderr)
|
||||||
|
if code != exitOK {
|
||||||
|
t.Errorf("run(%s) = %d, want %d; stderr: %s",
|
||||||
|
name, code, exitOK, stderr.String())
|
||||||
|
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := stdout(); got != want {
|
||||||
|
t.Errorf("%s stdout = %q, want %q", name, got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -31,9 +31,9 @@ type scanRec struct {
|
|||||||
// loadRecords opens the database and reads every file record for the
|
// loadRecords opens the database and reads every file record for the
|
||||||
// report and trees subcommands. Any database problem — including a
|
// report and trees subcommands. Any database problem — including a
|
||||||
// missing database — is fatal. The error is returned rather than
|
// missing database — is fatal. The error is returned rather than
|
||||||
// exiting, so that the deferred close — which checkpoints the SQLite
|
// exiting, so that the deferred close always runs; the database is
|
||||||
// WAL — always runs; the database is closed before the caller formats
|
// closed before the caller formats its output, so it stays closed even
|
||||||
// its output, so it stays closed even if that output fails.
|
// if that output fails.
|
||||||
func loadRecords(ctx context.Context) ([]scanRec, error) {
|
func loadRecords(ctx context.Context) ([]scanRec, error) {
|
||||||
dbPath := databasePath()
|
dbPath := databasePath()
|
||||||
|
|
||||||
|
|||||||
@@ -87,8 +87,9 @@ type fileMeta struct {
|
|||||||
// hash only when its size, head, and tail match another file's. Flag
|
// hash only when its size, head, and tail match another file's. Flag
|
||||||
// parsing and the at-least-one-operand check are done by cobra. Errors
|
// parsing and the at-least-one-operand check are done by cobra. Errors
|
||||||
// are returned rather than exiting, so that the deferred close — which
|
// are returned rather than exiting, so that the deferred close — which
|
||||||
// checkpoints the SQLite WAL — always runs. Cancelling ctx unwinds the
|
// takes the database out of WAL mode — always runs. Cancelling ctx
|
||||||
// worker pools and aborts the scan with the context's error.
|
// unwinds the worker pools and aborts the scan with the context's
|
||||||
|
// error.
|
||||||
func runScan(ctx context.Context, roots []string, workers int,
|
func runScan(ctx context.Context, roots []string, workers int,
|
||||||
oneFS bool,
|
oneFS bool,
|
||||||
) error {
|
) error {
|
||||||
@@ -108,7 +109,7 @@ func runScan(ctx context.Context, roots []string, workers int,
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
defer func() { _ = db.Close() }()
|
defer closeScanDatabase(ctx, db, dbPath)
|
||||||
|
|
||||||
st, err := syncScan(ctx, db, roots, workers, oneFS)
|
st, err := syncScan(ctx, db, roots, workers, oneFS)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user