From 3d257afd49b4d40db1714abe97b0468ba2046f5a Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Mon, 18 May 2026 19:05:29 -0400 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20Update=20transaction?= =?UTF-8?q?=20style=20and=20add=20busy=20timeout=20PRAGMA=20(#3271)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 Update transaction style and add busy PRAGMA * refactor: 💡 Add return to insert statement for consistency * refactor: 💡 Also add check for db * test: 💍 Fix flaky test --- addons/api/addon/workers/sqlite-worker.js | 52 +++++++++++++++---- .../acceptance/storage-buckets/update-test.js | 1 + 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/addons/api/addon/workers/sqlite-worker.js b/addons/api/addon/workers/sqlite-worker.js index f16b89631..e038a4d69 100644 --- a/addons/api/addon/workers/sqlite-worker.js +++ b/addons/api/addon/workers/sqlite-worker.js @@ -46,6 +46,15 @@ import { // See https://sqlite.org/rescode.html#corrupt const SQLITE_CORRUPT = 11; + /** + * Throws if the database has not been initialized or has been closed. + */ + const requireDb = () => { + if (!db) { + throw new Error('Database is not available'); + } + }; + const methods = { initializeSQLite: async () => { sqlite3 = await sqlite3InitModule({ @@ -57,6 +66,9 @@ import { }); function setupDb() { + // Allow SQLite to wait up to 5 s when it encounters a busy lock + db.exec('PRAGMA busy_timeout = 5000;'); + const [row] = db.exec('PRAGMA user_version;', { rowMode: 'object' }); // Check if we're on a newer schema version and clear the database if so if (SCHEMA_VERSION > row.user_version) { @@ -121,16 +133,16 @@ import { } }, analyzeDatabase: () => { + requireDb(); db.exec('PRAGMA optimize'); }, clearDatabase: () => { + requireDb(); db.exec(CLEAR_DB); db.exec(CREATE_TABLES(SCHEMA_VERSION)); }, deleteDatabase: () => { - if (!db) { - throw new Error('No database was initialized'); - } + requireDb(); const name = db.dbFilename(); db.close(); @@ -138,9 +150,11 @@ import { poolUtil.unlink(`/${name}`); }, downloadDatabase: () => { + requireDb(); return sqlite3.capi.sqlite3_js_db_export(db); }, fetchResource: ({ sql, parameters = [] }) => { + requireDb(); try { return db.exec({ sql, @@ -157,6 +171,7 @@ import { if (items.length === 0) { return []; } + requireDb(); const numberOfParameters = items.reduce((acc, item) => { return acc + item.length; @@ -165,11 +180,15 @@ import { // We want to use as many VALUES as part of the insert statement as it's quicker // than doing multiple inserts. If we have too many rows to insert, we'll manually // chunk them up to avoid hitting the maximum number of host parameters - // and execute these separate insert statements in a transaction + // and execute these separate insert statements in a transaction. if (numberOfParameters > MAX_HOST_PARAMETERS) { const chunkSize = Math.floor(MAX_HOST_PARAMETERS / items[0].length); - return db.transaction(() => { + // Note: We use explicit BEGIN/COMMIT instead of db.transaction() because + // db.transaction() uses SAVEPOINT internally, which fails with SQLITE_BUSY + // when any statement is active on the connection. + db.exec('BEGIN'); + try { const results = []; for (let i = 0; i < items.length; i += chunkSize) { const chunk = items.slice(i, i + chunkSize); @@ -180,20 +199,29 @@ import { }); results.push(result); } + db.exec('COMMIT'); return results; - }); + } catch (err) { + db.exec('ROLLBACK'); + throw err; + } } - db.exec({ + return db.exec({ sql: INSERT_STATEMENTS(resource, items, modelMapping), bind: items.flat(), rowMode: 'object', }); }, deleteResource: ({ resource, ids }) => { - // Check if we have too many parameters for the deletion and chunk if so + requireDb(); + // Check if we have too many parameters for the deletion and chunk if so. if (ids?.length > MAX_HOST_PARAMETERS) { - return db.transaction(() => { + // Note: We use explicit BEGIN/COMMIT instead of db.transaction() because + // db.transaction() uses SAVEPOINT internally, which fails with SQLITE_BUSY + // when any statement is active on the connection. + db.exec('BEGIN'); + try { const results = []; for (let i = 0; i < ids.length; i += MAX_HOST_PARAMETERS) { const chunk = ids.slice(i, i + MAX_HOST_PARAMETERS); @@ -204,8 +232,12 @@ import { }); results.push(result); } + db.exec('COMMIT'); return results; - }); + } catch (err) { + db.exec('ROLLBACK'); + throw err; + } } return db.exec({ diff --git a/ui/admin/tests/acceptance/storage-buckets/update-test.js b/ui/admin/tests/acceptance/storage-buckets/update-test.js index f3f0bff27..6480ec6b5 100644 --- a/ui/admin/tests/acceptance/storage-buckets/update-test.js +++ b/ui/admin/tests/acceptance/storage-buckets/update-test.js @@ -346,6 +346,7 @@ module('Acceptance | storage-buckets | update', function (hooks) { await click(commonSelectors.EDIT_BTN, 'Click edit mode'); + await waitFor(commonSelectors.CODE_EDITOR_CM); assert.dom(commonSelectors.CODE_EDITOR_CODE).isVisible(); assert.dom(selectors.READONLY_WORKER_FILTER).doesNotExist(); });