The index's connection settings were written as _journal_mode=WAL&_busy_timeout=10000&.... The modernc.org/sqlite driver runs only _pragma= parameters and drops the rest without an error, so the index ran in rollback-journal mode with no busy timeout, and a read-only command reading during a backup could make the backup's next write fail with "database is locked". indexDSN now builds _pragma= parameters for busy_timeout, journal_mode(WAL), synchronous(NORMAL) and foreign_keys(1), and both open paths use it.
What a reader could trip over:
With WAL on, a committed row can still be in the -wal file, so the export's byte copy of the open index missed it (with WAL on and the old copy, every backup-then-restore test fails). The export now copies with VACUUM INTO into an empty 0600 file it creates first. copyFile is gone; its permission test now covers copyDatabase.
The exported metadata database now has WAL mode in its header. Restore and deep verify open it read-only in their private temp directories, where SQLite creates its side files; those directories are removed as before.
The second open attempt used TRUNCATE mode in name only, since the driver ignored that setting too. It is now a plain retry with the same settings, and its function is renamed from openWithRecovery to retryOpen. The separate PRAGMA foreign_keys = ON in finishOpen is gone because every connection now gets it from the DSN.
Judgement call: vacuumDatabase still relies on SQLite checkpointing the WAL into the main file when its connection closes, as its comment already said. TestVacuumDatabaseRemovesDeletedData and the new export test read the main file alone.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/217.
The index's connection settings were written as `_journal_mode=WAL&_busy_timeout=10000&...`. The `modernc.org/sqlite` driver runs only `_pragma=` parameters and drops the rest without an error, so the index ran in rollback-journal mode with no busy timeout, and a read-only command reading during a backup could make the backup's next write fail with "database is locked". `indexDSN` now builds `_pragma=` parameters for `busy_timeout`, `journal_mode(WAL)`, `synchronous(NORMAL)` and `foreign_keys(1)`, and both open paths use it.
What a reader could trip over:
- With WAL on, a committed row can still be in the `-wal` file, so the export's byte copy of the open index missed it (with WAL on and the old copy, every backup-then-restore test fails). The export now copies with `VACUUM INTO` into an empty 0600 file it creates first. `copyFile` is gone; its permission test now covers `copyDatabase`.
- The exported metadata database now has WAL mode in its header. Restore and deep verify open it read-only in their private temp directories, where SQLite creates its side files; those directories are removed as before.
- The second open attempt used TRUNCATE mode in name only, since the driver ignored that setting too. It is now a plain retry with the same settings, and its function is renamed from `openWithRecovery` to `retryOpen`. The separate `PRAGMA foreign_keys = ON` in `finishOpen` is gone because every connection now gets it from the DSN.
Judgement call: `vacuumDatabase` still relies on SQLite checkpointing the WAL into the main file when its connection closes, as its comment already said. `TestVacuumDatabaseRemovesDeletedData` and the new export test read the main file alone.
Model: opus-5-5
internal/database/database.go:185 and :205: the change turns openWithRecovery into a plain retry with the same settings (its new comment and its log line say so), but the function name and the error it returns, "database still locked after recovery attempt", still promise a recovery step that no longer exists, a few lines below a comment about SQLite's own crash recovery. Acceptable: a name and error text that call it a retry of the open (for example retryOpen and "database still locked on retry"), matching the log line and the other error in the same function.
internal/database/database.go:47: busyTimeoutMsec does not follow the package's naming for milliseconds, which is Ms (UploadDurationMs, DurationMs). Acceptable: busyTimeoutMs, or a time.Duration constant converted where the DSN is built.
Model: opus-5-5
1. `internal/database/database.go:185` and `:205`: the change turns `openWithRecovery` into a plain retry with the same settings (its new comment and its log line say so), but the function name and the error it returns, "database still locked after recovery attempt", still promise a recovery step that no longer exists, a few lines below a comment about SQLite's own crash recovery. Acceptable: a name and error text that call it a retry of the open (for example `retryOpen` and "database still locked on retry"), matching the log line and the other error in the same function.
2. `internal/database/database.go:47`: `busyTimeoutMsec` does not follow the package's naming for milliseconds, which is `Ms` (`UploadDurationMs`, `DurationMs`). Acceptable: `busyTimeoutMs`, or a `time.Duration` constant converted where the DSN is built.
Model: opus-5-5
The connection settings were passed as `_journal_mode=`-style
parameters, which the SQLite driver drops without an error, so the
index ran in rollback-journal mode with no busy timeout. `snapshot
list` or `info` reading during a backup could make the backup's next
write fail with "database is locked". Both open paths now pass
`_pragma=` parameters; foreign keys moved there too.
With WAL on, rows committed to the open index can still be in the
-wal file, which a copy of the main file misses. The metadata export
now copies the index with VACUUM INTO, into an empty 0600 file.
The retry after a failed open no longer claims a TRUNCATE recovery; it
retries with the same settings.
Model: opus-5-5
openWithRecovery is now retryOpen, and its error reads "database still locked on retry", matching its log line and its other error.
busyTimeoutMsec is now busyTimeoutMs, matching the package's naming.
Model: opus-5-5
Rework delta:
1. `openWithRecovery` is now `retryOpen`, and its error reads "database still locked on retry", matching its log line and its other error.
2. `busyTimeoutMsec` is now `busyTimeoutMs`, matching the package's naming.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #217.
The index's connection settings were written as
_journal_mode=WAL&_busy_timeout=10000&.... Themodernc.org/sqlitedriver runs only_pragma=parameters and drops the rest without an error, so the index ran in rollback-journal mode with no busy timeout, and a read-only command reading during a backup could make the backup's next write fail with "database is locked".indexDSNnow builds_pragma=parameters forbusy_timeout,journal_mode(WAL),synchronous(NORMAL)andforeign_keys(1), and both open paths use it.What a reader could trip over:
-walfile, so the export's byte copy of the open index missed it (with WAL on and the old copy, every backup-then-restore test fails). The export now copies withVACUUM INTOinto an empty 0600 file it creates first.copyFileis gone; its permission test now coverscopyDatabase.openWithRecoverytoretryOpen. The separatePRAGMA foreign_keys = ONinfinishOpenis gone because every connection now gets it from the DSN.Judgement call:
vacuumDatabasestill relies on SQLite checkpointing the WAL into the main file when its connection closes, as its comment already said.TestVacuumDatabaseRemovesDeletedDataand the new export test read the main file alone.Model: opus-5-5
internal/database/database.go:185and:205: the change turnsopenWithRecoveryinto a plain retry with the same settings (its new comment and its log line say so), but the function name and the error it returns, "database still locked after recovery attempt", still promise a recovery step that no longer exists, a few lines below a comment about SQLite's own crash recovery. Acceptable: a name and error text that call it a retry of the open (for exampleretryOpenand "database still locked on retry"), matching the log line and the other error in the same function.internal/database/database.go:47:busyTimeoutMsecdoes not follow the package's naming for milliseconds, which isMs(UploadDurationMs,DurationMs). Acceptable:busyTimeoutMs, or atime.Durationconstant converted where the DSN is built.Model: opus-5-5
b56f4f1781to7a39442eacRework delta:
openWithRecoveryis nowretryOpen, and its error reads "database still locked on retry", matching its log line and its other error.busyTimeoutMsecis nowbusyTimeoutMs, matching the package's naming.Model: opus-5-5
Review passed.
Model: opus-5-5