From 8b8218c6107e65d58332a85fe93bcda619ff530e Mon Sep 17 00:00:00 2001 From: Mike Conley Date: Wed, 17 Apr 2024 20:50:47 +0000 Subject: [PATCH] Bug 1891141 - Add configuration parameters for pages per step and step delay to mozIStorageAsyncConnection.backupToFileAsync. r=mak This also fixes a bug in the test_connection_online_backup.js test which wasn't properly evaluating that the page_size PRAGMA was being copied properly. Differential Revision: https://phabricator.services.mozilla.com/D207471 --- dom/cache/Connection.cpp | 3 +- storage/mozIStorageAsyncConnection.idl | 10 +++- storage/mozStorageConnection.cpp | 42 +++++++++---- .../unit/test_connection_online_backup.js | 60 +++++++++++++++++-- toolkit/modules/Sqlite.sys.mjs | 41 +++++++++---- 5 files changed, 127 insertions(+), 29 deletions(-) diff --git a/dom/cache/Connection.cpp b/dom/cache/Connection.cpp index e9c9becebc7e..f7afca0d7a5a 100644 --- a/dom/cache/Connection.cpp +++ b/dom/cache/Connection.cpp @@ -288,7 +288,8 @@ uint32_t Connection::DecreaseTransactionNestingLevel( NS_IMETHODIMP Connection::BackupToFileAsync(nsIFile* aDestinationFile, - mozIStorageCompletionCallback* aCallback) { + mozIStorageCompletionCallback* aCallback, + uint32_t aPagesPerStep, uint32_t aStepDelayMs) { // async methods are not supported return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/storage/mozIStorageAsyncConnection.idl b/storage/mozIStorageAsyncConnection.idl index 767f5232d0d0..7fca212dca35 100644 --- a/storage/mozIStorageAsyncConnection.idl +++ b/storage/mozIStorageAsyncConnection.idl @@ -395,6 +395,12 @@ interface mozIStorageAsyncConnection : nsISupports { * - status: the status of the operation, use this to check if making * the copy was successful. * - value: unused. + * @param [aPagesPerStep=5] + * The number of pages in the database to copy per step. Defaults to 5 + * if omitted or set to 0. + * @param [aStepDelayMs=250] + * The number of milliseconds to wait between each step. Defaults to + * 250 if omitted or set to 0. * @throws NS_ERROR_ABORT * If the application has begun the process of shutting down already. * @throws NS_ERROR_NOT_INITIALIZED @@ -405,5 +411,7 @@ interface mozIStorageAsyncConnection : nsISupports { * If the execution thread cannot be acquired. */ void backupToFileAsync(in nsIFile aDestinationFile, - in mozIStorageCompletionCallback aCallback); + in mozIStorageCompletionCallback aCallback, + [optional] in unsigned long aPagesPerStep, + [optional] in unsigned long aStepDelayMs); }; diff --git a/storage/mozStorageConnection.cpp b/storage/mozStorageConnection.cpp index 3e298d0d825d..b0d55b71a55f 100644 --- a/storage/mozStorageConnection.cpp +++ b/storage/mozStorageConnection.cpp @@ -606,12 +606,15 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback { */ AsyncBackupDatabaseFile(Connection* aConnection, sqlite3* aNativeConnection, nsIFile* aDestinationFile, - mozIStorageCompletionCallback* aCallback) + mozIStorageCompletionCallback* aCallback, + int32_t aPagesPerStep, uint32_t aStepDelayMs) : Runnable("storage::AsyncBackupDatabaseFile"), mConnection(aConnection), mNativeConnection(aNativeConnection), mDestinationFile(aDestinationFile), mCallback(aCallback), + mPagesPerStep(aPagesPerStep), + mStepDelayMs(aStepDelayMs), mBackupFile(nullptr), mBackupHandle(nullptr) { MOZ_ASSERT(NS_IsMainThread()); @@ -681,19 +684,14 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback { rv = file->InitWithPath(tempPath); DISPATCH_AND_RETURN_IF_FAILED(rv); - // The number of milliseconds to wait between each batch of copies. - static constexpr uint32_t STEP_DELAY_MS = 250; - // The number of pages to copy per step - static constexpr int COPY_PAGES = 5; - - int srv = ::sqlite3_backup_step(mBackupHandle, COPY_PAGES); + int srv = ::sqlite3_backup_step(mBackupHandle, mPagesPerStep); if (srv == SQLITE_OK || srv == SQLITE_BUSY || srv == SQLITE_LOCKED) { // We're continuing the backup later. Release the guard to avoid closing // the database. guard.release(); // Queue up the next step - return NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, - STEP_DELAY_MS, nsITimer::TYPE_ONE_SHOT, + return NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, mStepDelayMs, + nsITimer::TYPE_ONE_SHOT, GetCurrentSerialEventTarget()); } #ifdef DEBUG @@ -771,6 +769,8 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback { nsCOMPtr mTimer; nsCOMPtr mDestinationFile; nsCOMPtr mCallback; + int32_t mPagesPerStep; + uint32_t mStepDelayMs; sqlite3* mBackupFile; sqlite3_backup* mBackupHandle; }; @@ -2962,7 +2962,8 @@ uint32_t Connection::DecreaseTransactionNestingLevel( NS_IMETHODIMP Connection::BackupToFileAsync(nsIFile* aDestinationFile, - mozIStorageCompletionCallback* aCallback) { + mozIStorageCompletionCallback* aCallback, + uint32_t aPagesPerStep, uint32_t aStepDelayMs) { NS_ENSURE_ARG(aDestinationFile); NS_ENSURE_ARG(aCallback); NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD); @@ -2984,9 +2985,28 @@ Connection::BackupToFileAsync(nsIFile* aDestinationFile, return NS_ERROR_NOT_INITIALIZED; } + // The number of pages of the database to copy per step + static constexpr int32_t DEFAULT_PAGES_PER_STEP = 5; + // The number of milliseconds to wait between each step. + static constexpr uint32_t DEFAULT_STEP_DELAY_MS = 250; + + CheckedInt pagesPerStep(aPagesPerStep); + if (!pagesPerStep.isValid()) { + return NS_ERROR_INVALID_ARG; + } + + if (!pagesPerStep.value()) { + pagesPerStep = DEFAULT_PAGES_PER_STEP; + } + + if (!aStepDelayMs) { + aStepDelayMs = DEFAULT_STEP_DELAY_MS; + } + // Create and dispatch our backup event to the execution thread. nsCOMPtr backupEvent = - new AsyncBackupDatabaseFile(this, mDBConn, aDestinationFile, aCallback); + new AsyncBackupDatabaseFile(this, mDBConn, aDestinationFile, aCallback, + pagesPerStep.value(), aStepDelayMs); rv = asyncThread->Dispatch(backupEvent, NS_DISPATCH_NORMAL); return rv; } diff --git a/storage/test/unit/test_connection_online_backup.js b/storage/test/unit/test_connection_online_backup.js index 3599d8c82427..ca8e27e9a21e 100644 --- a/storage/test/unit/test_connection_online_backup.js +++ b/storage/test/unit/test_connection_online_backup.js @@ -85,9 +85,15 @@ async function getPreparedAsyncDatabase() { * * @param {mozIStorageAsyncConnection} connection * A connection to a database that should be copied. + * @param {number} [pagesPerStep] + * The number of pages to copy per step. If not supplied or is 0, falls back + * to the platform default which is currently 5. + * @param {number} [stepDelayMs] + * The number of milliseconds to wait between copying step. If not supplied + * or is 0, falls back to the platform default which is currently 250. * @returns {Promise} */ -async function createCopy(connection) { +async function createCopy(connection, pagesPerStep, stepDelayMs) { let destFilePath = PathUtils.join(PathUtils.profileDir, BACKUP_FILE_NAME); let destFile = await IOUtils.getFile(destFilePath); Assert.ok( @@ -96,10 +102,15 @@ async function createCopy(connection) { ); await new Promise(resolve => { - connection.backupToFileAsync(destFile, result => { - Assert.ok(Components.isSuccessCode(result)); - resolve(result); - }); + connection.backupToFileAsync( + destFile, + result => { + Assert.ok(Components.isSuccessCode(result)); + resolve(result); + }, + pagesPerStep, + stepDelayMs + ); }); return destFile; @@ -121,7 +132,7 @@ async function assertSuccessfulCopy(file, expectedEntries = TEST_ROWS) { await executeSimpleSQLAsync(conn, "PRAGMA page_size", resultSet => { let result = resultSet.getNextRow(); - Assert.equal(TEST_PAGE_SIZE, result.getResultByIndex(0).getAsUint32()); + Assert.equal(TEST_PAGE_SIZE, result.getResultByIndex(0)); }); let stmt = conn.createAsyncStatement("SELECT COUNT(*) FROM test"); @@ -190,6 +201,36 @@ add_task(async function test_backupToFileAsync_during_insert() { await asyncClose(newConnection); }); +/** + * Tests that alternative pages-per-step and step delay values can be set when + * calling backupToFileAsync. + */ +add_task(async function test_backupToFileAsync_during_insert() { + let newConnection = await getPreparedAsyncDatabase(); + + // Let's try some higher values... + let copyFile = await createCopy(newConnection, 15, 500); + Assert.ok( + await IOUtils.exists(copyFile.path), + "A new file was created by backupToFileAsync" + ); + + await assertSuccessfulCopy(copyFile); + await IOUtils.remove(copyFile.path); + + // And now we'll try some lower values... + copyFile = await createCopy(newConnection, 1, 25); + Assert.ok( + await IOUtils.exists(copyFile.path), + "A new file was created by backupToFileAsync" + ); + + await assertSuccessfulCopy(copyFile); + await IOUtils.remove(copyFile.path); + + await asyncClose(newConnection); +}); + /** * Tests the behaviour of backupToFileAsync as exposed through Sqlite.sys.mjs. */ @@ -206,6 +247,13 @@ add_task(async function test_backupToFileAsync_via_Sqlite_module() { await assertSuccessfulCopy(copyFile); await IOUtils.remove(copyFile.path); + + // Also check that we can plumb through pagesPerStep and stepDelayMs. + await moduleConnection.backup(copyFilePath, 15, 500); + Assert.ok(await IOUtils.exists(copyFilePath), "A new file was created"); + await assertSuccessfulCopy(copyFile); + await IOUtils.remove(copyFile.path); + await moduleConnection.close(); await asyncClose(xpcomConnection); }); diff --git a/toolkit/modules/Sqlite.sys.mjs b/toolkit/modules/Sqlite.sys.mjs index b1f48c28be66..ca58904d6bd0 100644 --- a/toolkit/modules/Sqlite.sys.mjs +++ b/toolkit/modules/Sqlite.sys.mjs @@ -1184,9 +1184,15 @@ ConnectionData.prototype = Object.freeze({ * @param {string} destFilePath * The path on the local filesystem to write the database copy. Any existing * file at this path will be overwritten. + * @param {number} [pagesPerStep=0] + * The number of pages to copy per step. If not supplied or is 0, falls back + * to the platform default which is currently 5. + * @param {number} [stepDelayMs=0] + * The number of milliseconds to wait between copying step. If not supplied + * or is 0, falls back to the platform default which is currently 250. * @return Promise */ - async backupToFile(destFilePath) { + async backupToFile(destFilePath, pagesPerStep = 0, stepDelayMs = 0) { if (!this._dbConn) { return Promise.reject( new Error("No opened database connection to create a backup from.") @@ -1194,13 +1200,18 @@ ConnectionData.prototype = Object.freeze({ } let destFile = await IOUtils.getFile(destFilePath); return new Promise((resolve, reject) => { - this._dbConn.backupToFileAsync(destFile, result => { - if (Components.isSuccessCode(result)) { - resolve(); - } else { - reject(result); - } - }); + this._dbConn.backupToFileAsync( + destFile, + result => { + if (Components.isSuccessCode(result)) { + resolve(); + } else { + reject(result); + } + }, + pagesPerStep, + stepDelayMs + ); }); }, }); @@ -2002,10 +2013,20 @@ OpenedConnection.prototype = Object.freeze({ * @param {string} destFilePath * The path on the local filesystem to write the database copy. Any existing * file at this path will be overwritten. + * @param {number} [pagesPerStep=0] + * The number of pages to copy per step. If not supplied or is 0, falls back + * to the platform default which is currently 5. + * @param {number} [stepDelayMs=0] + * The number of milliseconds to wait between copying step. If not supplied + * or is 0, falls back to the platform default which is currently 250. * @return Promise */ - backup(destFilePath) { - return this._connectionData.backupToFile(destFilePath); + backup(destFilePath, pagesPerStep = 0, stepDelayMs = 0) { + return this._connectionData.backupToFile( + destFilePath, + pagesPerStep, + stepDelayMs + ); }, });