Bug 1435446 - Use immediate transactions by default in Places. r?mak
MozReview-Commit-ID: L2rCMwqZZkQ
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -981,37 +981,44 @@ Database::TryToCloneTablesFromCorruptDat
return NS_OK;
}
nsresult
Database::SetupDatabaseConnection(nsCOMPtr<mozIStorageService>& aStorage)
{
MOZ_ASSERT(NS_IsMainThread());
+ // Using immediate transactions allows the main connection to retry writes
+ // that fail with `SQLITE_BUSY` because a cloned connection has locked the
+ // database for writing.
+ nsresult rv = mMainConn->SetDefaultTransactionType(
+ mozIStorageConnection::TRANSACTION_IMMEDIATE);
+ NS_ENSURE_SUCCESS(rv, rv);
+
// WARNING: any statement executed before setting the journal mode must be
// finalized, since SQLite doesn't allow changing the journal mode if there
// is any outstanding statement.
{
// Get the page size. This may be different than the default if the
// database file already existed with a different page size.
nsCOMPtr<mozIStorageStatement> statement;
- nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
+ rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA page_size"
), getter_AddRefs(statement));
NS_ENSURE_SUCCESS(rv, rv);
bool hasResult = false;
rv = statement->ExecuteStep(&hasResult);
NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && hasResult, NS_ERROR_FILE_CORRUPTED);
rv = statement->GetInt32(0, &mDBPageSize);
NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && mDBPageSize > 0, NS_ERROR_FILE_CORRUPTED);
}
// Ensure that temp tables are held in memory, not on disk.
- nsresult rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+ rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA temp_store = MEMORY")
);
NS_ENSURE_SUCCESS(rv, rv);
rv = SetupDurability(mMainConn, mDBPageSize);
NS_ENSURE_SUCCESS(rv, rv);
nsAutoCString busyTimeoutPragma("PRAGMA busy_timeout = ");
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2146,22 +2146,22 @@ PlacesUtils.keywords = {
cache.delete(oldKeyword);
}
}
await db.executeCached(
`INSERT INTO moz_keywords (keyword, place_id, post_data)
VALUES (:keyword, (SELECT id FROM moz_places WHERE url_hash = hash(:url) AND url = :url), :post_data)
`, { url: url.href, keyword, post_data: postData });
+
+ await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
+ db, url, PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source));
});
}
- await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
- db, url, PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source));
-
cache.set(keyword, { keyword, url, postData: postData || null });
// In any case, notify about the new keyword.
await notifyKeywordChange(url.href, keyword, source);
}
);
},
@@ -2190,21 +2190,23 @@ PlacesUtils.keywords = {
keyword = keywordOrEntry.keyword.trim().toLowerCase();
return PlacesUtils.withConnectionWrapper("PlacesUtils.keywords.remove", async db => {
let cache = await promiseKeywordsCache();
if (!cache.has(keyword))
return;
let { url } = cache.get(keyword);
cache.delete(keyword);
- await db.execute(`DELETE FROM moz_keywords WHERE keyword = :keyword`,
- { keyword });
+ await db.executeTransaction(async function() {
+ await db.execute(`DELETE FROM moz_keywords WHERE keyword = :keyword`,
+ { keyword });
- await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
- db, url, PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source));
+ await PlacesSyncUtils.bookmarks.addSyncChangesForBookmarksWithURL(
+ db, url, PlacesSyncUtils.bookmarks.determineSyncChangeDelta(source));
+ });
// Notify bookmarks about the removal.
await notifyKeywordChange(url.href, "", source);
});
},
/**
* Moves all (keyword, POST data) pairs from one URL to another, and fires
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -448,17 +448,17 @@ class SyncedBookmarksMirror {
await this.db.execute(`DELETE FROM itemsChanged`);
await this.db.execute(`DELETE FROM itemsRemoved`);
await this.db.execute(`DELETE FROM itemsMoved`);
await this.db.execute(`DELETE FROM annosChanged`);
await this.db.execute(`DELETE FROM itemsToWeaklyReupload`);
await this.db.execute(`DELETE FROM itemsToUpload`);
return changeRecords;
- }, this.db.TRANSACTION_IMMEDIATE);
+ });
MirrorLog.debug("Replaying recorded observer notifications");
try {
await observersToNotify.notifyAll();
} catch (ex) {
MirrorLog.error("Error notifying Places observers", ex);
}
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -2395,17 +2395,17 @@ nsNavHistory::GetObservers(uint32_t* _co
}
// See RunInBatchMode
nsresult
nsNavHistory::BeginUpdateBatch()
{
if (mBatchLevel++ == 0) {
mBatchDBTransaction = new mozStorageTransaction(mDB->MainConn(), false,
- mozIStorageConnection::TRANSACTION_DEFERRED,
+ mozIStorageConnection::TRANSACTION_DEFAULT,
true);
NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavHistoryObserver, OnBeginUpdateBatch());
}
return NS_OK;
}
// nsNavHistory::EndUpdateBatch
@@ -2460,25 +2460,25 @@ nsNavHistory::GetHistoryDisabled(bool *_
nsresult
nsNavHistory::RemovePagesInternal(const nsCString& aPlaceIdsQueryString)
{
// Return early if there is nothing to delete.
if (aPlaceIdsQueryString.IsEmpty())
return NS_OK;
- mozStorageTransaction transaction(mDB->MainConn(), false,
- mozIStorageConnection::TRANSACTION_DEFERRED,
- true);
-
- // Delete all visits for the specified place ids.
nsCOMPtr<mozIStorageConnection> conn = mDB->MainConn();
if (!conn) {
return NS_ERROR_UNEXPECTED;
}
+ mozStorageTransaction transaction(conn, false,
+ mozIStorageConnection::TRANSACTION_DEFAULT,
+ true);
+
+ // Delete all visits for the specified place ids.
nsresult rv = conn->ExecuteSimpleSQL(
NS_LITERAL_CSTRING(
"DELETE FROM moz_historyvisits WHERE place_id IN (") +
aPlaceIdsQueryString +
NS_LITERAL_CSTRING(")")
);
NS_ENSURE_SUCCESS(rv, rv);