We only ever checked for corruption errors from transactions in
queryAsync(), so a corruption error raised by the COMMIT that mozStorage
runs itself bypassed the check. A 10.0 schema upgrade -- which heavily
exercises the database -- that failed due to corruption showed "Database
upgrade error" and a single Sqlite.sys.mjs frame instead of the prompt
offering to restore from a backup.
Two users reported this, but it's not clear what triggered it --
corruption usually occurs during a statement, which we did catch. There
may have been some statement transaction whose corruption error was
caught and ignored rather than being left to abort the transaction,
causing SQLite to then block the commit. That's what the test does, and
it fails without the fix.
https://forums.zotero.org/discussion/133611/
(cherry picked from commit bdaecbb5cc)
* Fix js-ctypes-based symlinking on Linux by using `libc.so.6` instead of `libc.so` in `OS.File.unixSymlink()` and `Zotero.File.createSymlink()`
* Use that instead of `/bin/ln`, which doesn't exist on NixOS
* Replace `/bin/ln` with `Zotero.File.createSymlink()` in symlinked-database test
---------
Co-authored-by: Dan Stillman <dstillman@zotero.org>
(cherry picked from commit d153397151)
CIFS mounts can't create real symlinks without special mount options,
so skip the symlinked-database and broken-symlink tests when a created
symlink doesn't exist or isn't visible as one.
(cherry picked from commit 4e532a17a2)
On macOS, SQLite chooses locking methods based on the filesystem
containing the database, and network filesystems (e.g., SMB, NFS),
read-only volumes, and filesystems without byte-range locking get
methods without shared-memory support, which WAL requires. Opening a
database with an adjacent WAL file on those crashes -- Mozilla's VFS
wrapper hides the missing shared-memory methods from SQLite's WAL
support check -- so the first Zotero 10 run converted the database to
WAL and every launch after that segfaulted during connection
initialization.
On macOS, mirror SQLite's method selection and use a rollback journal
when shared memory isn't available, converting an existing WAL database
before opening it: an empty WAL by reverting the header format versions
in place, and a non-empty WAL by replaying it into a temporary copy on
local disk that replaces the database file only after passing an
integrity check. A WAL file next to an already-converted database
(e.g., from an interrupted conversion) goes through the same
conversion. Also use openNotExclusive during integrity checks and
corruption recovery, which otherwise fail on SMB shares with an I/O
error from the exclusive open lock.
This corrects 22055d92b7, which passed openNotExclusive on all
platforms for an open failure described as affecting macOS and Linux,
and expected locking_mode=EXCLUSIVE to keep the WAL index in heap
memory with no -shm file. Neither claim held up: mozStorage opens the
WAL while initializing the connection, before any pragma can run, so
the index uses shared memory unless the exclusive VFS is in use, and
the exclusive open works on Linux CIFS mounts -- where non-exclusive
access instead made SQLite's lock-upgrade sequence trip over the SMB
byte-range lock mapping, leaving startup hung and the database never
created. So pass openNotExclusive only on macOS. On Linux this restores
unix-excl, which performs all locking under a single held lock and
keeps the WAL index in heap memory; Windows has no distinct exclusive
VFS and is unaffected.
https://forums.zotero.org/discussion/133258/
(cherry picked from commit bba85a3939)
SQLite replays a leftover -wal file (e.g., from a force-quit) into
whatever file next occupies the database path, so copying a backup
over zotero.sqlite produced a corrupted-database error, and the
automatic restore recreated the same mismatch and failed every time.
When corruption is detected, check if the database file is valid without
its journal files, and if so, save a verified copy and restart, swapping
the copy in at the next startup before the database is reopened. (The
copy is made before shutdown because SQLite automatically checkpoints
the WAL into the database file when the last connection closes, which
would write the stale WAL data into the file.) Otherwise, move journal
files along with the .damaged file to clear them from the main path
before restoring from the automatic backup or creating a new database.
Since a mismatched WAL can cause subtle data damage without errors, also
run a full integrity check at startup after an unclean shutdown. Skip
the explicit close-time WAL checkpoint once corruption has been flagged,
and truncate the WAL during idle maintenance to limit stale WAL data.
vacuum() and the APFS-cloning offline backup path both close and
reopen the SQLite connection, which drops all ATTACHed databases --
including the in-memory "indexing" alias used for the fulltextWords
scratch table set up in Fulltext.init(). Once the connection is
reopened, indexing queries fail with "no such table:
indexing.fulltextWords", and indexItems()'s ignoreErrors path
routes the error to logError(), so indexing silently stops working
for the rest of the session.
Add an onConnect() hook on Zotero.DBConnection for per-connection
state that doesn't persist across reopens, and use it from
Fulltext.init() to re-attach the indexing DB on each reconnect.
Regression sources:
- 67288047f3 ("Use APFS cloning for file copies on macOS") flips
online idle backups to offline-with-clone on APFS, so every idle
backup interval (24h default) closes and reopens the connection.
Affects Mac users on APFS.
- b27c4cb023 ("Enable SQLite WAL mode and add periodic VACUUM
INTO") adds the vacuum path, which closes and reopens on the
first idle period in a session that passes the freelist/time
gates. Affects all users with sufficient DB churn, roughly once
per 14 days.
https://forums.zotero.org/discussion/131576/debug-id-d848621212-indexing-of-pdfs-fail-zotero-9-0-3https://forums.zotero.org/discussion/131718/possible-bug-regression-report-search-unusable-on-macos-zotero-9-0-3-with-large-library
These tested racing an offline backup against an in-progress online
backup, which wouldn't happen in practice (schema backups run at startup
before the idle observer, and someone is very unlikely to perform a DB
integrity check immediately after returning from an idle that triggered
a backup). The tests relied on fragile timing and a shared tmp file,
causing failures on Linux after the WAL checkpoint change. The tests
were already skipped on macOS after switching to APFS clones.
- Switch journal mode from DELETE to WAL for better write performance.
With EXCLUSIVE locking mode, SQLite uses heap memory for the WAL
index, avoiding an -shm file. Set synchronous=NORMAL (matching what
Mozilla uses for Places). Checkpoint WAL on database close so the
.sqlite file has all data (for copies or backups).
- Add periodic database compaction on idle (after DB backup) using
VACUUM INTO and do an atomic file swap back to zotero.sqlite if no
writes occurred during the operation. Check if vacuuming is needed
based on time interval (default 14 days) and freelist ratio (default
10% threshold).
- Disable auto_vacuum, which causes fragmentation and is unnecessary
with periodic VACUUM
- Remove the VACUUM call from the integrity check, which was always just
an awkward hack to let people trigger a VACUUM without having an
explicit button
Closes#652
Add Zotero.File.copyFile(), which uses clonefile() on APFS with a
fallback to IOUtils.copy(), and use it for all significant file copies.
APFS is detected and cached for each parent folder via
Zotero.File.isAPFS(), which uses statfs().
On APFS, all database backups now use the offline (close/clone/reopen)
path instead of the SQLite online backup API. Cloning is nearly
instant, and backup files share disk blocks via copy-on-write, saving
potentially gigabytes of space.
Closes#5330
A new function, `Zotero.DB.backUpDatabase()`, by default makes offline
backups, by closing the database, doing a regular file copy, and
reopening the database. It takes an options object with an `online`
flag to make online, incremental backups that can take multiple minutes
to complete, though for now we're still running them on idle.
The old function, `Zotero.DB.backupDatabase(suffix, force)`, is
deprecated and proxies to the new function, making offline backups.
Fixes#4935
- Create userdata tables and indexes that are missing
- Delete tables and triggers that should no longer exist
- Run schema integrity check before user data migration
- Run schema integrity check after restart error
This is meant to address two problems:
1) Database damage, and subsequent use of the DB Repair Tool, that
results in missing tables
2) A small number of cases of schema update steps somehow not being
reflected in users' databases despite their having updated userdata
numbers, which are set within the same transaction. Until we figure
out how that's happening, we should start adding conditional versions
of schema update steps to the integrity check.
This is currently only running the update check after a restart error,
which might not occur for all missed schema update steps, so we might
want other triggers for calling setIntegrityCheckRequired().
- Add an 'exclusive' option to transactions that causes them to block other
transactions and wait for other transactions to finish before starting,
instead of nesting
- Resolve Zotero.DB.waitForTransaction() promise before returning from
executeTransaction()
- A side effect of the above: wait for a newly created item to be selected in
the middle pane and rendered in the right-hand pane before returning from
executeTransaction()
- Don't save items multiple times when adding/removing a non-final creator in
the Info pane
- Use a simpler, non-recursive method for focusing the next field in the Info
pane; this prevents "too much recursion" errors if something causes the
right-hand pane not to be rendered when expected
This fixes an issue where two transactions started around the same time
could run separately instead of nesting, causing the statements from one
to end up running not within a transaction
