Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(15)

Unified Diff: chrome/browser/sync/syncable/directory_backing_store.cc

Issue 8586004: Revert 110356 - Sync: Improve handling of database load failures (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Created 9 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/sync/syncable/directory_backing_store.cc
===================================================================
--- chrome/browser/sync/syncable/directory_backing_store.cc (revision 110383)
+++ chrome/browser/sync/syncable/directory_backing_store.cc (working copy)
@@ -191,38 +191,34 @@
bool DirectoryBackingStore::OpenAndConfigureHandleHelper(
sqlite3** handle) const {
if (SQLITE_OK == sqlite_utils::OpenSqliteDb(backing_filepath_, handle)) {
- sqlite3_busy_timeout(*handle, std::numeric_limits<int>::max());
+ sqlite_utils::scoped_sqlite_db_ptr scoped_handle(*handle);
+ sqlite3_busy_timeout(scoped_handle.get(), std::numeric_limits<int>::max());
{
string integrity_error;
- bool is_ok = CheckIntegrity(*handle, &integrity_error);
+ bool is_ok = CheckIntegrity(scoped_handle.get(), &integrity_error);
if (!is_ok) {
LOG(ERROR) << "Integrity check failed: " << integrity_error;
- sqlite3_close(*handle);
- *handle = NULL;
return false;
}
}
{
sqlite_utils::SQLStatement statement;
- statement.prepare(*handle, "PRAGMA fullfsync = 1");
+ statement.prepare(scoped_handle.get(), "PRAGMA fullfsync = 1");
if (SQLITE_DONE != statement.step()) {
- LOG(ERROR) << sqlite3_errmsg(*handle);
- sqlite3_close(*handle);
- *handle = NULL;
+ LOG(ERROR) << sqlite3_errmsg(scoped_handle.get());
return false;
}
}
{
sqlite_utils::SQLStatement statement;
- statement.prepare(*handle, "PRAGMA synchronous = 2");
+ statement.prepare(scoped_handle.get(), "PRAGMA synchronous = 2");
if (SQLITE_DONE != statement.step()) {
- LOG(ERROR) << sqlite3_errmsg(*handle);
- sqlite3_close(*handle);
- *handle = NULL;
+ LOG(ERROR) << sqlite3_errmsg(scoped_handle.get());
return false;
}
}
- sqlite3_busy_timeout(*handle, kDirectoryBackingStoreBusyTimeoutMs);
+ sqlite3_busy_timeout(scoped_handle.release(),
+ kDirectoryBackingStoreBusyTimeoutMs);
#if defined(OS_WIN)
// Do not index this file. Scanning can occur every time we close the file,
// which causes long delays in SQLite's file locking.
@@ -233,25 +229,14 @@
#endif
return true;
- } else {
- // The sqlite3 API docs indicate a handle is usually returned even
- // in the case of error. We clean it up here.
- if (*handle != NULL) {
- sqlite3_close(*handle);
- *handle = NULL;
- }
- return false;
}
+ return false;
}
bool DirectoryBackingStore::CheckIntegrity(sqlite3* handle, string* error)
const {
sqlite_utils::SQLStatement statement;
- if (SQLITE_OK !=
- statement.prepare(handle, "PRAGMA integrity_check(1)")) {
- *error = sqlite3_errmsg(handle);
- return false;
- }
+ statement.prepare(handle, "PRAGMA integrity_check(1)");
if (SQLITE_ROW != statement.step()) {
*error = sqlite3_errmsg(handle);
return false;
@@ -304,7 +289,37 @@
bool DirectoryBackingStore::BeginLoad() {
DCHECK(load_dbhandle_ == NULL);
- return OpenAndConfigureHandleHelper(&load_dbhandle_);
+ bool ret = OpenAndConfigureHandleHelper(&load_dbhandle_);
+ if (ret)
+ return true;
+ // Something's gone wrong. Nuke the database and try again.
+ using ::operator<<; // For string16.
+ LOG(ERROR) << "Sync database " << backing_filepath_.value()
+ << " corrupt. Deleting and recreating.";
+ file_util::Delete(backing_filepath_, false);
+ bool failed_again = !OpenAndConfigureHandleHelper(&load_dbhandle_);
+
+ // Using failed_again here lets us distinguish from cases where corruption
+ // occurred even when re-opening a fresh directory (they'll go in a separate
+ // double weight histogram bucket). Failing twice in a row means we disable
+ // sync, so it's useful to see this number separately.
+ int bucket = failed_again ? 2 : 1;
+#if defined(OS_WIN)
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedWin", bucket);
+#elif defined(OS_MACOSX)
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedMac", bucket);
+#else
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedNotWinMac", bucket);
+
+#if defined(OS_CHROMEOS)
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedCros", bucket);
+#elif defined(OS_LINUX)
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedLinux", bucket);
+#else
+ UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedOther", bucket);
+#endif // OS_LINUX && !OS_CHROMEOS
+#endif // OS_WIN
+ return !failed_again;
}
void DirectoryBackingStore::EndLoad() {
« no previous file with comments | « chrome/browser/sync/profile_sync_service_unittest.cc ('k') | chrome/browser/sync/syncable/directory_backing_store_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698