[sql] Recover Favicons v5 databases, with more recovery automation.

chrome/browser/history/thumbnail_database.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/history/thumbnail_database.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 257 matching lines @@
     // want once-only handling?  Sqlite.Error.Thumbnail shows
     // CORRUPT and READONLY as almost 95% of all reports on these
     // channels, so probably easier to just harvest from the field.
     if (rand < kReportsPerMillion) {
       reported = true;
       ReportError(db, extended_error);
     }
   }
 }
 
+// Create v5 schema for recovery code.
+bool InitSchemaV5(sql::Connection* db) {
+  // This schema was derived from the strings used when v5 was in
+  // force.  The [favicons] index and the [icon_mapping] items were
+  // copied from the current strings, after verifying that the
+  // resulting schema exactly matches the schema created by the
+  // original versions of those strings.  This allows the linker to
+  // share the strings if they match, while preferring correctness of
+  // the current versions change.
+
+  const char kFaviconsV5[] =
+      "CREATE TABLE favicons("

[pkotwicz, 2013/11/04 02:38:31]

Nit: It would be nice if we were consistent in using either "CREATE TABLE" or
"CREATE TABLE IF NOT EXISTS"

[Scott Hess - ex-Googler, 2013/11/04 21:41:44]

As a general rule, I would prefer if we never ever did "CREATE TABLE IF NOT
EXISTS", because it implies that we don't know the current state of the
database, and thus cannot reason about the final state, either.  If the table
already exists, that's a fatal error, we should blow everything away! 
Unfortunately, given the way our code has evolved over time, I'm not totally
clear where that change could be made safely.  For this database, probably could
happen now, though it would require tweaks to the setup ordering (either upgrade
or create, never create-then-upgrade).

In any case, the comment above is why this function is inconsistent.  I'll
change this one to use IF NOT EXISTS to be consistent.

+      "id INTEGER PRIMARY KEY,"
+      "url LONGVARCHAR NOT NULL,"
+      "last_updated INTEGER DEFAULT 0,"
+      "image_data BLOB,"
+      "icon_type INTEGER DEFAULT 1,"
+      "sizes LONGVARCHAR"
+      ")";
+  const char kFaviconsIndexV5[] =
+      "CREATE INDEX IF NOT EXISTS favicons_url ON favicons(url)";
+  if (!db->Execute(kFaviconsV5) || !db->Execute(kFaviconsIndexV5))
+    return false;
+
+  const char kIconMappingV5[] =
+      "CREATE TABLE IF NOT EXISTS icon_mapping"
+      "("
+      "id INTEGER PRIMARY KEY,"
+      "page_url LONGVARCHAR NOT NULL,"
+      "icon_id INTEGER"
+      ")";
+  const char kIconMappingUrlIndexV5[] =
+      "CREATE INDEX IF NOT EXISTS icon_mapping_page_url_idx"
+      " ON icon_mapping(page_url)";
+  const char kIconMappingIdIndexV5[] =
+      "CREATE INDEX IF NOT EXISTS icon_mapping_icon_id_idx"
+      " ON icon_mapping(icon_id)";
+  if (!db->Execute(kIconMappingV5) ||
+      !db->Execute(kIconMappingUrlIndexV5) ||
+      !db->Execute(kIconMappingIdIndexV5)) {
+    return false;
+  }
+
+  return true;
+}
+
+// TODO(shess): Consider InitSchemaV7().  InitSchemaV5() is worthwhile
+// because there appear to be 10s of thousands of marooned v5
+// databases in the wild.  Once recovery reaches stable, the number of
+// corrupt-but-recoverable databases should drop, possibly to the
+// point where it is not worthwhile to maintain previous-version
+// recovery code.
+// TODO(shess): Alternately, think on a way to more cleanly represent
+// versioned schema going forward.
 bool InitTables(sql::Connection* db) {
   const char kIconMappingSql[] =
       "CREATE TABLE IF NOT EXISTS icon_mapping"
       "("
       "id INTEGER PRIMARY KEY,"
       "page_url LONGVARCHAR NOT NULL,"
       "icon_id INTEGER"
       ")";
   if (!db->Execute(kIconMappingSql))
     return false;
@@ skipping 47 matching lines @@
       "favicon_bitmaps(icon_id)";
   if (!db->Execute(kFaviconBitmapsIndexSql))
     return false;
 
   return true;
 }
 
 enum RecoveryEventType {
   RECOVERY_EVENT_RECOVERED = 0,
   RECOVERY_EVENT_FAILED_SCOPER,
-  RECOVERY_EVENT_FAILED_META_VERSION_ERROR,
-  RECOVERY_EVENT_FAILED_META_VERSION_NONE,
+  RECOVERY_EVENT_FAILED_META_VERSION_ERROR,  // obsolete
+  RECOVERY_EVENT_FAILED_META_VERSION_NONE,  // obsolete
   RECOVERY_EVENT_FAILED_META_WRONG_VERSION6,  // obsolete
-  RECOVERY_EVENT_FAILED_META_WRONG_VERSION5,
+  RECOVERY_EVENT_FAILED_META_WRONG_VERSION5,  // obsolete
   RECOVERY_EVENT_FAILED_META_WRONG_VERSION,
-  RECOVERY_EVENT_FAILED_RECOVER_META,
+  RECOVERY_EVENT_FAILED_RECOVER_META,  // obsolete
   RECOVERY_EVENT_FAILED_META_INSERT,  // obsolete
   RECOVERY_EVENT_FAILED_INIT,
-  RECOVERY_EVENT_FAILED_RECOVER_FAVICONS,
-  RECOVERY_EVENT_FAILED_FAVICONS_INSERT,
-  RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS,
-  RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT,
-  RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING,
-  RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT,
+  RECOVERY_EVENT_FAILED_RECOVER_FAVICONS,  // obsolete
+  RECOVERY_EVENT_FAILED_FAVICONS_INSERT,  // obsolete
+  RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS,  // obsolete
+  RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT,  // obsolete
+  RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING,  // obsolete
+  RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT,  // obsolete
   RECOVERY_EVENT_RECOVERED_VERSION6,
   RECOVERY_EVENT_FAILED_META_INIT,
+  RECOVERY_EVENT_FAILED_META_VERSION,
+  RECOVERY_EVENT_DEPRECATED,
+  RECOVERY_EVENT_FAILED_V5_INITSCHEMA,
+  RECOVERY_EVENT_FAILED_V5_AUTORECOVER_FAVICONS,
+  RECOVERY_EVENT_FAILED_V5_AUTORECOVER_ICON_MAPPING,
+  RECOVERY_EVENT_RECOVERED_VERSION5,
+  RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICONS,
+  RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICON_BITMAPS,
+  RECOVERY_EVENT_FAILED_AUTORECOVER_ICON_MAPPING,
 
   // Always keep this at the end.
   RECOVERY_EVENT_MAX,
 };
 
 void RecordRecoveryEvent(RecoveryEventType recovery_event) {
   UMA_HISTOGRAM_ENUMERATION("History.FaviconsRecovery",
                             recovery_event, RECOVERY_EVENT_MAX);
 }
 
@@ skipping 20 matching lines @@
     //
     // Possible responses are unclear.  If the failure relates to a
     // problem somehow specific to the temporary file used to back the
     // database, then an in-memory database could possibly be used.
     // This could potentially allow recovering the main database, and
     // might be simple to implement w/in Begin().
     RecordRecoveryEvent(RECOVERY_EVENT_FAILED_SCOPER);
     return;
   }
 
-  // Setup the meta recovery table, and check that the version number
-  // is covered by the recovery code.
-  // TODO(shess): sql::Recovery should provide a helper to handle meta.
-  int version = 0;  // For reporting which version was recovered.
-  {
-    const char kRecoverySql[] =
-        "CREATE VIRTUAL TABLE temp.recover_meta USING recover"
-        "("
-        "corrupt.meta,"
-        "key TEXT NOT NULL,"
-        "value TEXT"  // Really?  Never int?
-        ")";
-    if (!recovery->db()->Execute(kRecoverySql)) {
-      // TODO(shess): Failure to create the recover_meta table could
-      // mean that the main database is too corrupt to access, or that
-      // the meta table doesn't exist.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_META);
-      return;
-    }
+  // Setup the meta recovery table and fetch the version number from
+  // the corrupt database.
+  int version = 0;
+  if (!recovery->SetupMeta() || !recovery->GetMetaVersionNumber(&version)) {
+    // TODO(shess): Prior histograms indicate all failures are in
+    // creating the recover virtual table for corrupt.meta.  The table
+    // may not exist, or the database may be too far gone.  Either
+    // way, unclear how to resolve.
+    sql::Recovery::Rollback(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION);
+    return;
+  }
 
-    {
-      const char kRecoveryVersionSql[] =
-          "SELECT value FROM recover_meta WHERE key = 'version'";
-      sql::Statement recovery_version(
-          recovery->db()->GetUniqueStatement(kRecoveryVersionSql));
-      if (!recovery_version.Step()) {
-        if (!recovery_version.Succeeded()) {
-          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION_ERROR);
-          // TODO(shess): An error while processing the statement is
-          // probably not recoverable.
-        } else {
-          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION_NONE);
-          // TODO(shess): If a positive version lock cannot be achieved,
-          // the database could still be recovered by optimistically
-          // attempting to copy things.  In the limit, the schema found
-          // could be inspected.  Less clear is whether optimistic
-          // recovery really makes sense.
-        }
-        recovery_version.Clear();
-        sql::Recovery::Rollback(recovery.Pass());
-        return;
-      }
-      version = recovery_version.ColumnInt(0);
-
-      // Recovery code is generally schema-dependent.  Version 7 and
-      // version 6 are very similar, so can be handled together.
-      // Track version 5, to see whether it's worth writing recovery
-      // code for.
-      if (version != 7 && version != 6) {
-        if (version == 5) {
-          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION5);
-        } else {
-          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION);
-        }
-        recovery_version.Clear();
-        sql::Recovery::Rollback(recovery.Pass());
-        return;
-      }
-    }
-
-    // Either version 6 or version 7 recovers to current.
+  // Recover v5 database to v5 schema.  Next pass through Init() will
+  // migrate to v7.
+  if (version == 5) {
     sql::MetaTable recover_meta_table;
-    if (!recover_meta_table.Init(recovery->db(), kCurrentVersionNumber,
-                                 kCompatibleVersionNumber)) {
+    if (!recover_meta_table.Init(recovery->db(), version, version)) {
       sql::Recovery::Rollback(recovery.Pass());
       RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_INIT);
       return;
     }
+
+    // TODO(shess): These tests are separate for histogram purposes,
+    // but once things look stable it can be tightened up.
+    if (!InitSchemaV5(recovery->db())) {
+      sql::Recovery::Rollback(recovery.Pass());
+      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_INITSCHEMA);
+      return;
+    }
+
+    if (!recovery->AutoRecoverTable("favicons", 0)) {
+      sql::Recovery::Rollback(recovery.Pass());
+      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_AUTORECOVER_FAVICONS);
+      return;
+    }
+
+    if (!recovery->AutoRecoverTable("icon_mapping", 0)) {
+      sql::Recovery::Rollback(recovery.Pass());
+      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_AUTORECOVER_ICON_MAPPING);
+      return;
+    }
+
+    ignore_result(sql::Recovery::Recovered(recovery.Pass()));
+    RecordRecoveryEvent(RECOVERY_EVENT_RECOVERED_VERSION5);
+    return;
+  }
+
+  // This code may be able to fetch versions that the regular
+  // deprecation path cannot.
+  if (version <= kDeprecatedVersionNumber) {
+    sql::Recovery::Unrecoverable(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_DEPRECATED);
+    return;
+  }
+
+  // TODO(shess): Earlier versions have been handled or deprecated,
+  // later versions should be impossible.  Unrecoverable() seems
+  // reasonable.
+  if (version != 6 && version != 7) {
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION);
+    sql::Recovery::Rollback(recovery.Pass());
+    return;
+  }
+
+  // Both v6 and v7 recover to current schema version.
+  sql::MetaTable recover_meta_table;
+  if (!recover_meta_table.Init(recovery->db(), kCurrentVersionNumber,
+                               kCompatibleVersionNumber)) {
+    sql::Recovery::Rollback(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_INIT);
+    return;
   }
 
   // Create a fresh version of the database.  The recovery code uses
   // conflict-resolution to handle duplicates, so the indices are
   // necessary.
   if (!InitTables(recovery->db()) || !InitIndices(recovery->db())) {
     // TODO(shess): Unable to create the new schema in the new
     // database.  The new database should be a temporary file, so
     // being unable to work with it is pretty unclear.
     //
     // What are the potential responses, even?  The recovery database
     // could be opened as in-memory.  If the temp database had a
     // filesystem problem and the temp filesystem differs from the
     // main database, then that could fix it.
     sql::Recovery::Rollback(recovery.Pass());
     RecordRecoveryEvent(RECOVERY_EVENT_FAILED_INIT);
     return;
   }
 
-  // Setup favicons table.
-  {
-    // Version 6 had the |sizes| column, version 7 removed it.  The
-    // recover virtual table treats more columns than expected as an
-    // error, but if _fewer_ columns are present, they can be treated
-    // as NULL.  SQLite requires this because ALTER TABLE adds columns
-    // to the schema, but not to the actual table storage.
-    const char kRecoverySql[] =
-        "CREATE VIRTUAL TABLE temp.recover_favicons USING recover"
-        "("
-        "corrupt.favicons,"
-        "id ROWID,"
-        "url TEXT NOT NULL,"
-        "icon_type INTEGER,"
-        "sizes TEXT"
-        ")";
-    if (!recovery->db()->Execute(kRecoverySql)) {
-      // TODO(shess): Failure to create the recovery table probably
-      // means unrecoverable.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_FAVICONS);
-      return;
-    }
-
-    // TODO(shess): Check if the DEFAULT 1 will just cover the
-    // COALESCE().  Either way, the new code has a literal 1 rather
-    // than a NULL, right?
-    const char kCopySql[] =
-        "INSERT OR REPLACE INTO main.favicons "
-        "SELECT id, url, COALESCE(icon_type, 1) FROM recover_favicons";
-    if (!recovery->db()->Execute(kCopySql)) {
-      // TODO(shess): The recover_favicons table should mask problems
-      // with the source file, so this implies failure to write to the
-      // recovery database.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_FAVICONS_INSERT);
-      return;
-    }
+  // [favicons] differs because v6 had an unused [sizes] column which
+  // was removed in v7.
+  if (!recovery->AutoRecoverTable("favicons", 1)) {
+    sql::Recovery::Rollback(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICONS);
+    return;
   }
-
-  // Setup favicons_bitmaps table.
-  {
-    const char kRecoverySql[] =
-        "CREATE VIRTUAL TABLE temp.recover_favicons_bitmaps USING recover"
-        "("
-        "corrupt.favicon_bitmaps,"
-        "id ROWID,"
-        "icon_id INTEGER STRICT NOT NULL,"
-        "last_updated INTEGER,"
-        "image_data BLOB,"
-        "width INTEGER,"
-        "height INTEGER"
-        ")";
-    if (!recovery->db()->Execute(kRecoverySql)) {
-      // TODO(shess): Failure to create the recovery table probably
-      // means unrecoverable.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS);
-      return;
-    }
-
-    const char kCopySql[] =
-        "INSERT OR REPLACE INTO main.favicon_bitmaps "
-        "SELECT id, icon_id, COALESCE(last_updated, 0), image_data, "
-        " COALESCE(width, 0), COALESCE(height, 0) "
-        "FROM recover_favicons_bitmaps";
-    if (!recovery->db()->Execute(kCopySql)) {
-      // TODO(shess): The recover_faviconbitmaps table should mask
-      // problems with the source file, so this implies failure to
-      // write to the recovery database.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT);
-      return;
-    }
+  if (!recovery->AutoRecoverTable("favicon_bitmaps", 0)) {
+    sql::Recovery::Rollback(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICON_BITMAPS);
+    return;
   }
-
-  // Setup icon_mapping table.
-  {
-    const char kRecoverySql[] =
-        "CREATE VIRTUAL TABLE temp.recover_icon_mapping USING recover"
-        "("
-        "corrupt.icon_mapping,"
-        "id ROWID,"
-        "page_url TEXT STRICT NOT NULL,"
-        "icon_id INTEGER STRICT"
-        ")";
-    if (!recovery->db()->Execute(kRecoverySql)) {
-      // TODO(shess): Failure to create the recovery table probably
-      // means unrecoverable.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING);
-      return;
-    }
-
-    const char kCopySql[] =
-        "INSERT OR REPLACE INTO main.icon_mapping "
-        "SELECT id, page_url, icon_id FROM recover_icon_mapping";
-    if (!recovery->db()->Execute(kCopySql)) {
-      // TODO(shess): The recover_icon_mapping table should mask
-      // problems with the source file, so this implies failure to
-      // write to the recovery database.
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT);
-      return;
-    }
+  if (!recovery->AutoRecoverTable("icon_mapping", 0)) {
+    sql::Recovery::Rollback(recovery.Pass());
+    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_ICON_MAPPING);
+    return;
   }
 
   // TODO(shess): Is it possible/likely to have broken foreign-key
   // issues with the tables?
   // - icon_mapping.icon_id maps to no favicons.id
   // - favicon_bitmaps.icon_id maps to no favicons.id
   // - favicons.id is referenced by no icon_mapping.icon_id
   // - favicons.id is referenced by no favicon_bitmaps.icon_id
   // This step is possibly not worth the effort necessary to develop
   // and sequence the statements, as it is basically a form of garbage
@@ skipping 666 matching lines @@
   ignore_result(db_.Execute("DROP TABLE IF EXISTS temp_icon_mapping"));
 
   // Create the tables.
   if (!meta_table_.Init(&db_, kCurrentVersionNumber,
                         kCompatibleVersionNumber) ||
       !InitTables(&db_) ||
       !InitIndices(&db_)) {
     return sql::INIT_FAILURE;
   }
 
   // Version check. We should not encounter a database too old for us to handle

[pkotwicz, 2013/11/04 02:38:31]

Unrelated: Shouldn't this comment be "too new for us to handle".

[Scott Hess - ex-Googler, 2013/11/04 21:41:44]

I think this means that the too-new case fails (with sql::INIT_TOO_NEW), while
the not-too-new case continues, which is safe because we should never see a
database too old to handle.

   // in the wild, so we try to continue in that case.
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
     LOG(WARNING) << "Thumbnail database is too new.";
     return sql::INIT_TOO_NEW;
   }
 
   int cur_version = meta_table_.GetVersionNumber();
 
   if (!db_.DoesColumnExist("favicons", "icon_type")) {
     LOG(ERROR) << "Raze because of missing favicon.icon_type";
@@ skipping 101 matching lines @@
   meta_table_.SetVersionNumber(7);
   meta_table_.SetCompatibleVersionNumber(std::min(7, kCompatibleVersionNumber));
   return true;
 }
 
 bool ThumbnailDatabase::IsFaviconDBStructureIncorrect() {
   return !db_.IsSQLValid("SELECT id, url, icon_type FROM favicons");
 }
 
 }  // namespace history