Index: extensions/browser/value_store/leveldb_value_store.cc |
diff --git a/extensions/browser/value_store/leveldb_value_store.cc b/extensions/browser/value_store/leveldb_value_store.cc |
index c11e6551b280a99cbc983f1c3adec6d4670dbc01..4b38b7e0960b9baf2bb6da40a2ad55c1309c71ba 100644 |
--- a/extensions/browser/value_store/leveldb_value_store.cc |
+++ b/extensions/browser/value_store/leveldb_value_store.cc |
@@ -29,6 +29,16 @@ namespace { |
const char kInvalidJson[] = "Invalid JSON"; |
const char kCannotSerialize[] = "Cannot serialize value to JSON"; |
+// UMA values used when recovering from a corrupted leveldb. |
+// Do not change/delete these values as you will break reporting for older |
+// copies of Chrome. Only add new values to the end. |
+enum LevelDBCorruptionRecoveryValue { |
+ LEVELDB_RESTORE_DELETE_SUCCESS = 0, |
+ LEVELDB_RESTORE_DELETE_FAILURE, |
+ LEVELDB_RESTORE_REPAIR_SUCCESS, |
+ LEVELDB_RESTORE_MAX |
+}; |
+ |
// Scoped leveldb snapshot which releases the snapshot on destruction. |
class ScopedSnapshot { |
public: |
@@ -54,7 +64,7 @@ class ScopedSnapshot { |
LeveldbValueStore::LeveldbValueStore(const std::string& uma_client_name, |
const base::FilePath& db_path) |
- : db_path_(db_path), open_histogram_(nullptr) { |
+ : db_path_(db_path), open_histogram_(nullptr), restore_histogram_(nullptr) { |
DCHECK_CURRENTLY_ON(BrowserThread::FILE); |
// Used in lieu of UMA_HISTOGRAM_ENUMERATION because the histogram name is |
@@ -63,6 +73,9 @@ LeveldbValueStore::LeveldbValueStore(const std::string& uma_client_name, |
"Extensions.Database.Open." + uma_client_name, 1, |
leveldb_env::LEVELDB_STATUS_MAX, leveldb_env::LEVELDB_STATUS_MAX + 1, |
base::Histogram::kUmaTargetedHistogramFlag); |
+ restore_histogram_ = base::LinearHistogram::FactoryGet( |
Devlin
2015/11/06 19:54:46
This is histogram black magic to me, so I'll trust
cmumford
2015/11/09 23:04:21
I manually hack-ed in a call to "restore_histogram
|
+ "Extensions.Database.Restore." + uma_client_name, 1, LEVELDB_RESTORE_MAX, |
+ LEVELDB_RESTORE_MAX + 1, base::Histogram::kUmaTargetedHistogramFlag); |
base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( |
this, "LeveldbValueStore", base::ThreadTaskRunnerHandle::Get()); |
} |
@@ -287,28 +300,26 @@ ValueStore::WriteResult LeveldbValueStore::Clear() { |
bool LeveldbValueStore::Restore() { |
DCHECK_CURRENTLY_ON(BrowserThread::FILE); |
- ReadResult result = Get(); |
- std::string previous_key; |
- while (result->IsCorrupted()) { |
- // If we don't have a specific corrupted key, or we've tried and failed to |
- // clear this specific key, or we fail to restore the key, then wipe the |
- // whole database. |
- if (!result->error().key.get() || *result->error().key == previous_key || |
- !RestoreKey(*result->error().key)) { |
- DeleteDbFile(); |
- result = Get(); |
- break; |
- } |
+ // Possible to have a corrupted open database, so first close it. |
+ db_.reset(); |
+ |
+ leveldb::Options options; |
+ options.create_if_missing = true; |
+ |
+ // Repair can drop an unbounded number of leveldb tables (key/value sets) |
+ leveldb::Status status = leveldb::RepairDB(db_path_.AsUTF8Unsafe(), options); |
+ if (status.ok()) { |
+ restore_histogram_->Add(LEVELDB_RESTORE_REPAIR_SUCCESS); |
+ return true; |
+ } |
- // Otherwise, re-Get() the database to check if there is still any |
- // corruption. |
- previous_key = *result->error().key; |
- result = Get(); |
+ if (DeleteDbFile()) { |
+ restore_histogram_->Add(LEVELDB_RESTORE_DELETE_SUCCESS); |
+ return true; |
} |
- // If we still have an error, it means we've tried deleting the database file, |
- // and failed. There's nothing more we can do. |
- return !result->IsCorrupted(); |
+ restore_histogram_->Add(LEVELDB_RESTORE_DELETE_FAILURE); |
+ return false; |
} |
bool LeveldbValueStore::RestoreKey(const std::string& key) { |
@@ -381,6 +392,12 @@ scoped_ptr<ValueStore::Error> LeveldbValueStore::EnsureDbIsOpen() { |
leveldb::DB::Open(options, db_path_.AsUTF8Unsafe(), &db); |
if (open_histogram_) |
open_histogram_->Add(leveldb_env::GetLevelDBStatusUMAValue(status)); |
+ if (status.IsCorruption()) { |
+ // Returning a corruption error should result in Restore() being called. |
+ // However, since once a leveldb becomes corrupt it's unusable without |
+ // some kind of repair or delete, so do that right now. |
+ Restore(); |
+ } |
if (!status.ok()) |
return ToValueStoreError(status, util::NoKey()); |
@@ -468,12 +485,14 @@ bool LeveldbValueStore::IsEmpty() { |
return is_empty; |
} |
-void LeveldbValueStore::DeleteDbFile() { |
+bool LeveldbValueStore::DeleteDbFile() { |
db_.reset(); // release any lock on the directory |
if (!base::DeleteFile(db_path_, true /* recursive */)) { |
LOG(WARNING) << "Failed to delete LeveldbValueStore database at " << |
db_path_.value(); |
+ return false; |
} |
+ return true; |
} |
scoped_ptr<ValueStore::Error> LeveldbValueStore::ToValueStoreError( |