Enable SQLite WAL mode and add periodic VACUUM INTO

- 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

chrome/content/zotero/preferences/preferences_advanced.js

@@ -237,13 +237,6 @@ Zotero_Preferences.Advanced = {
 					return;
 				}
 				
-				try {
-					await Zotero.DB.vacuum();
-				}
-				catch (e) {
-					Zotero.logError(e);
-					ok = false;
-				}
 			}
 			var str = ok ? 'passed' : 'failed';
 			

chrome/content/zotero/xpcom/db.js

@@ -94,6 +94,7 @@ Zotero.DBConnection = function (dbNameOrPath) {
 	this._lastTransactionDate = null;
 	this._transactionRollback = false;
 	this._transactionNestingLevel = 0;
+	this._commitCount = 0;
 	this._callbacks = {
 		begin: [],
 		commit: [],
@@ -481,6 +482,7 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
 			}
 			
 			result = await conn.executeTransaction(func);
+			this._commitCount++;
 			Zotero.debug(`Committed DB transaction ${id}`, 4);
 		}
 		finally {
@@ -500,12 +502,6 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
 		
 		this._transactionID = null;
 		
-		if (options.vacuumOnCommit) {
-			Zotero.debug('Vacuuming database');
-			await this.queryAsync('VACUUM');
-			Zotero.debug('Done vacuuming');
-		}
-		
 		// Function to run once transaction has been committed but before any
 		// permanent callbacks
 		if (options.onCommit) {
@@ -906,10 +902,16 @@ Zotero.DBConnection.prototype.executeSQLFile = async function (sql) {
 /*
  * Implements nsIObserver
  */
-Zotero.DBConnection.prototype.observe = function (subject, topic, data) {
+Zotero.DBConnection.prototype.observe = async function (subject, topic, data) {
 	switch (topic) {
 		case 'idle':
-			this.backUpDatabase({ online: true });
+			try {
+				await this.backUpDatabase({ online: true });
+				await this.vacuum();
+			}
+			catch (e) {
+				Zotero.logError(e);
+			}
 			break;
 	}
 }
@@ -925,16 +927,129 @@ Zotero.DBConnection.prototype.getCachedStatements = function () {
 };
 
 
-// TEMP
-Zotero.DBConnection.prototype.vacuum = function () {
-	return this.executeTransaction(async function () {}, { vacuumOnCommit: true });
+/**
+ * Vacuum the database using VACUUM INTO and perform an atomic file swap
+ *
+ * Creates a compacted copy of the database without blocking writes during
+ * the copy phase, then closes the connection and atomically replaces the
+ * original file if no writes occurred during compaction.
+ *
+ * @param {Object} [options]
+ * @param {Boolean} [options.force] - Skip time/freelist/disk-space checks
+ * @return {Promise<Boolean>} - Whether vacuum was performed
+ */
+Zotero.DBConnection.prototype.vacuum = async function ({ force } = {}) {
+	if (this._externalDB) {
+		return false;
+	}
+
+	if (this.inTransaction()) {
+		await this.waitForTransaction();
+	}
+
+	if (!force) {
+		// Check time threshold
+		let lastVacuum = Zotero.Prefs.get('vacuum.lastTime') || 0;
+		let intervalDays = Zotero.Prefs.get('vacuum.interval') || 14;
+		let intervalMs = intervalDays * 24 * 60 * 60 * 1000;
+		if ((Date.now() - lastVacuum) < intervalMs) {
+			Zotero.debug("Database was vacuumed recently -- skipping");
+			return false;
+		}
+
+		// Check freelist threshold
+		let freelistCount = await this.valueQueryAsync("PRAGMA freelist_count");
+		let pageCount = await this.valueQueryAsync("PRAGMA page_count");
+		let threshold = Zotero.Prefs.get('vacuum.freelistThreshold') || 10;
+		if (pageCount > 0 && (freelistCount / pageCount * 100) < threshold) {
+			Zotero.debug(`Database freelist is ${freelistCount}/${pageCount} pages `
+				+ `(${(freelistCount / pageCount * 100).toFixed(1)}%) `
+				+ `-- below ${threshold}% threshold, skipping`);
+			return false;
+		}
+
+		// Check disk space
+		let dbFile = Zotero.File.pathToFile(this._dbPath);
+		let dbSize = (await IOUtils.stat(this._dbPath)).size;
+		let freeSpace = dbFile.diskSpaceAvailable;
+		if (freeSpace < dbSize) {
+			Zotero.debug(`Not enough disk space to vacuum database `
+				+ `(${freeSpace} available, ${dbSize} needed) -- skipping`);
+			return false;
+		}
+	}
+
+	let tmpFile = this._dbPath + '.vacuum.tmp';
+
+	try {
+		// Clean up any leftover temp file from a previous failed attempt
+		if (await IOUtils.exists(tmpFile)) {
+			await IOUtils.remove(tmpFile);
+		}
+
+		Zotero.debug("Vacuuming database");
+		let t = new Date();
+
+		let commitCountBefore = this._commitCount;
+
+		// Disable auto_vacuum for the output file if previously enabled -- periodic VACUUM handles
+		// compaction, and auto_vacuum causes fragmentation
+		await this.queryAsync("PRAGMA auto_vacuum=0");
+
+		// VACUUM INTO creates a compacted copy as a snapshot of committed data. Concurrent writes
+		// in WAL are NOT included in the output.
+		await this.queryAsync(`VACUUM INTO '${tmpFile.replace(/'/g, "''")}'`);
+
+		// Block other code from reopening the connection during the swap
+		let resolveVacuumPromise;
+		this._offlineBackupPromise = new Promise(function () {
+			resolveVacuumPromise = arguments[0];
+		});
+
+		try {
+			// Close the database -- this waits for any in-flight transactions and checkpoints WAL
+			await this.closeDatabase();
+
+			// If any writes happened between VACUUM INTO start and close, the compacted copy is
+			// stale -- abort
+			if (this._commitCount !== commitCountBefore) {
+				Zotero.debug("Database was modified during vacuum -- aborting swap", 1);
+				await IOUtils.remove(tmpFile);
+				return false;
+			}
+
+			// Atomic swap
+			await IOUtils.move(tmpFile, this._dbPath);
+
+			Zotero.Prefs.set('vacuum.lastTime', Date.now());
+			Zotero.debug("Vacuumed database in " + (new Date() - t) + " ms");
+
+			return true;
+		}
+		finally {
+			this._offlineBackupPromise = null;
+			resolveVacuumPromise();
+		}
+	}
+	catch (e) {
+		Zotero.logError(e);
+		try {
+			if (await IOUtils.exists(tmpFile)) {
+				await IOUtils.remove(tmpFile);
+			}
+		}
+		catch (e2) {
+			Zotero.logError(e2);
+		}
+		return false;
+	}
 };
 
 
 // TEMP
 Zotero.DBConnection.prototype.info = async function () {
 	var info = {};
-	var pragmas = ['auto_vacuum', 'cache_size', 'main.locking_mode', 'page_size'];
+	var pragmas = ['auto_vacuum', 'cache_size', 'journal_mode', 'main.locking_mode', 'page_size', 'synchronous'];
 	for (let p of pragmas) {
 		info[p] = await Zotero.DB.valueQueryAsync(`PRAGMA ${p}`);
 	}
@@ -978,6 +1093,18 @@ Zotero.DBConnection.prototype.closeDatabase = async function (permanent) {
 		}
 		
 		Zotero.debug("Closing database");
+
+		// Checkpoint WAL before closing so all data is in the main file
+		// and -wal file is truncated. Use _connection.execute() directly
+		// to avoid deadlocking with _offlineBackupPromise in queryAsync.
+		try {
+			Zotero.debug("PRAGMA wal_checkpoint(TRUNCATE)");
+			await this._connection.execute("PRAGMA wal_checkpoint(TRUNCATE)");
+		}
+		catch (e) {
+			Zotero.logError(e);
+		}
+
 		this.closed = true;
 		await this._connection.close();
 		this._connection = undefined;
@@ -1285,7 +1412,15 @@ Zotero.DBConnection.prototype._getConnectionAsync = async function () {
 		else {
 			await this.queryAsync("PRAGMA main.locking_mode=NORMAL");
 		}
-		
+
+		// Enable WAL mode for better write performance. With locking_mode=EXCLUSIVE
+		// set first, SQLite uses heap memory for the WAL index instead of shared
+		// memory, so no -shm file is created on disk.
+		await this.queryAsync("PRAGMA journal_mode=WAL");
+		// NORMAL synchronous is safe with WAL -- only risks losing the last
+		// transaction on power loss, not corruption
+		await this.queryAsync("PRAGMA synchronous=NORMAL");
+
 		// Set page cache size to 8MB
 		let pageSize = await this.valueQueryAsync("PRAGMA page_size");
 		let cacheSize = 8192000 / pageSize;

chrome/content/zotero/xpcom/schema.js

@@ -2206,11 +2206,9 @@ Zotero.Schema = new function () {
 			try {
 				var userLibraryID = 1;
 				
-				// Enable auto-vacuuming
 				await Zotero.DB.queryAsync("PRAGMA page_size = 4096");
 				await Zotero.DB.queryAsync("PRAGMA encoding = 'UTF-8'");
-				await Zotero.DB.queryAsync("PRAGMA auto_vacuum = 1");
-				
+
 				var sql = await _getSchemaSQL('system');
 				await Zotero.DB.executeSQLFile(sql);
 				

defaults/preferences/zotero.js

@@ -68,6 +68,10 @@ pref("extensions.zotero.feeds.defaultCleanupUnreadAfter", 30);
 pref("extensions.zotero.backup.numBackups", 2);
 pref("extensions.zotero.backup.interval", 1440);
 
+pref("extensions.zotero.vacuum.lastTime", 0);
+pref("extensions.zotero.vacuum.interval", 14);  // days
+pref("extensions.zotero.vacuum.freelistThreshold", 10);  // percentage of free pages to trigger
+
 pref("extensions.zotero.lastCreatorFieldMode",0);
 pref("extensions.zotero.lastAbstractExpand", true);
 pref("extensions.zotero.lastRenameAssociatedFile", false);

test/tests/dbTest.js

@@ -453,4 +453,37 @@ describe("Zotero.DB", function () {
 			assert.isTrue(await IOUtils.exists(bakFile));
 		});
 	});
+
+
+	describe("#vacuum()", function () {
+		it("should vacuum the database with force option", async function () {
+			let result = await Zotero.DB.vacuum({ force: true });
+			assert.isTrue(result);
+
+			// DB should still be functional
+			let count = await Zotero.DB.valueQueryAsync("SELECT COUNT(*) FROM items");
+			assert.isNumber(count);
+
+			// Vacuum timestamp should be updated
+			assert.isAbove(Zotero.Prefs.get('vacuum.lastTime'), 0);
+
+			// Temp file should be cleaned up
+			assert.isFalse(await IOUtils.exists(Zotero.DB.path + '.vacuum.tmp'));
+		});
+
+		it("should skip vacuum when recently vacuumed", async function () {
+			Zotero.Prefs.set('vacuum.lastTime', Date.now());
+			let result = await Zotero.DB.vacuum();
+			assert.isFalse(result);
+			Zotero.Prefs.clear('vacuum.lastTime');
+		});
+
+		it("should skip vacuum when freelist is below threshold", async function () {
+			Zotero.Prefs.clear('vacuum.lastTime');
+			Zotero.Prefs.set('vacuum.freelistThreshold', 99);
+			let result = await Zotero.DB.vacuum();
+			assert.isFalse(result);
+			Zotero.Prefs.clear('vacuum.freelistThreshold');
+		});
+	});
 });