[sql] Track uploads of diagnostic data to prevent duplication.

components/history/core/browser/thumbnail_database.cc

Index: components/history/core/browser/thumbnail_database.cc
diff --git a/components/history/core/browser/thumbnail_database.cc b/components/history/core/browser/thumbnail_database.cc
index 35afa12319fc9a021b6c7c3d7dfd30971e35cf5f..78dc8582df6e7bc31292072a881ebc5fbf60b551 100644
--- a/components/history/core/browser/thumbnail_database.cc
+++ b/components/history/core/browser/thumbnail_database.cc
@@ -9,9 +9,7 @@
 
 #include "base/bind.h"
 #include "base/debug/alias.h"
-#include "base/debug/dump_without_crashing.h"
 #include "base/files/file_util.h"
-#include "base/format_macros.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
@@ -121,164 +119,26 @@ void RecordInvalidStructure(InvalidStructureType invalid_type) {
                             invalid_type, STRUCTURE_EVENT_MAX);
 }
 
-// Attempt to pass 2000 bytes of |debug_info| into a crash dump.
-void DumpWithoutCrashing2000(const std::string& debug_info) {
-  char debug_buf[2000];
-  base::strlcpy(debug_buf, debug_info.c_str(), arraysize(debug_buf));
-  base::debug::Alias(&debug_buf);
-
-  base::debug::DumpWithoutCrashing();
-}
-
-void ReportCorrupt(sql::Connection* db, size_t startup_kb) {
-  // Buffer for accumulating debugging info about the error.  Place
-  // more-relevant information earlier, in case things overflow the
-  // fixed-size buffer.
-  std::string debug_info;
-
-  base::StringAppendF(&debug_info, "SQLITE_CORRUPT, integrity_check:\n");
-
-  // Check files up to 8M to keep things from blocking too long.
-  const size_t kMaxIntegrityCheckSize = 8192;
-  if (startup_kb > kMaxIntegrityCheckSize) {
-    base::StringAppendF(&debug_info, "too big %" PRIuS "\n", startup_kb);
-  } else {
-    std::vector<std::string> messages;
-
-    const base::TimeTicks before = base::TimeTicks::Now();
-    db->FullIntegrityCheck(&messages);
-    base::StringAppendF(&debug_info, "# %" PRIx64 " ms, %" PRIuS " records\n",
-                        (base::TimeTicks::Now() - before).InMilliseconds(),
-                        messages.size());
-
-    // SQLite returns up to 100 messages by default, trim deeper to
-    // keep close to the 2000-character size limit for dumping.
-    //
-    // TODO(shess): If the first 20 tend to be actionable, test if
-    // passing the count to integrity_check makes it exit earlier.  In
-    // that case it may be possible to greatly ease the size
-    // restriction.
-    const size_t kMaxMessages = 20;
-    for (size_t i = 0; i < kMaxMessages && i < messages.size(); ++i) {
-      base::StringAppendF(&debug_info, "%s\n", messages[i].c_str());
-    }
-  }
-
-  DumpWithoutCrashing2000(debug_info);
-}
-
-void ReportError(sql::Connection* db, int error) {
-  // Buffer for accumulating debugging info about the error.  Place
-  // more-relevant information earlier, in case things overflow the
-  // fixed-size buffer.
-  std::string debug_info;
-
-  // The error message from the failed operation.
-  base::StringAppendF(&debug_info, "db error: %d/%s\n",
-                      db->GetErrorCode(), db->GetErrorMessage());
-
-  // System errno information.
-  base::StringAppendF(&debug_info, "errno: %d\n", db->GetLastErrno());
-
-  // SQLITE_ERROR reports seem to be attempts to upgrade invalid
-  // schema, try to log that info.
-  if (error == SQLITE_ERROR) {
-    const char* kVersionSql = "SELECT value FROM meta WHERE key = 'version'";
-    if (db->IsSQLValid(kVersionSql)) {
-      sql::Statement statement(db->GetUniqueStatement(kVersionSql));
-      if (statement.Step()) {
-        debug_info += "version: ";
-        debug_info += statement.ColumnString(0);
-        debug_info += '\n';
-      } else if (statement.Succeeded()) {
-        debug_info += "version: none\n";
-      } else {
-        debug_info += "version: error\n";
-      }
-    } else {
-      debug_info += "version: invalid\n";
-    }
-
-    debug_info += "schema:\n";
-
-    // sqlite_master has columns:
-    //   type - "index" or "table".
-    //   name - name of created element.
-    //   tbl_name - name of element, or target table in case of index.
-    //   rootpage - root page of the element in database file.
-    //   sql - SQL to create the element.
-    // In general, the |sql| column is sufficient to derive the other
-    // columns.  |rootpage| is not interesting for debugging, without
-    // the contents of the database.  The COALESCE is because certain
-    // automatic elements will have a |name| but no |sql|,
-    const char* kSchemaSql = "SELECT COALESCE(sql, name) FROM sqlite_master";
-    sql::Statement statement(db->GetUniqueStatement(kSchemaSql));
-    while (statement.Step()) {
-      debug_info += statement.ColumnString(0);
-      debug_info += '\n';
-    }
-    if (!statement.Succeeded())
-      debug_info += "error\n";
-  }
-
-  // TODO(shess): Think of other things to log.  Not logging the
-  // statement text because the backtrace should suffice in most
-  // cases.  The database schema is a possibility, but the
-  // likelihood of recursive error callbacks makes that risky (same
-  // reasoning applies to other data fetched from the database).
-
-  DumpWithoutCrashing2000(debug_info);
-}
-
-// TODO(shess): If this proves out, perhaps lift the code out to
-// chrome/browser/diagnostics/sqlite_diagnostics.{h,cc}.
+// TODO(shess): If this proves out, move it all into sql::Connection to be
+// shared.
 void GenerateDiagnostics(sql::Connection* db,
-                         size_t startup_kb,
-                         int extended_error) {
-  int error = (extended_error & 0xFF);
-
-  // Infrequently report information about the error up to the crash
-  // server.
-  static const uint64 kReportsPerMillion = 50000;
-
+                         int extended_error,
+                         sql::Statement* stmt) {
   // Since some/most errors will not resolve themselves, only report
   // once per Chrome run.
   static bool reported = false;
   if (reported)
     return;
+  reported = true;
 
+  // Only pass 5% of new reports to prevent a thundering herd of dumps.
+  // TODO(shess): If this could be related to the time in the channel, then the
+  // rate could ramp up over time.  Perhaps could remember the timestamp the
+  // first time upload is considered, and ramp up 1% per day?
+  static const uint64 kReportsPerMillion = 50000;
   uint64 rand = base::RandGenerator(1000000);

[pkotwicz, 2015/10/21 15:23:33]

Nit: Would it be clearer if the RandGenerator had a max value of 100?

[Scott Hess - ex-Googler, 2015/10/22 00:22:11]

Done.  Originally I wasn't sure if it would want to go below 1% for certain
types of errors, but that wasn't necessary.  I don't think it is likely to
become necessary, but it can always be changed again if so.

-  if (error == SQLITE_CORRUPT) {
-    // Once the database is known to be corrupt, it will generate a
-    // stream of errors until someone fixes it, so give one chance.
-    // Set first in case of errors in generating the report.
-    reported = true;
-
-    // Corrupt cases currently dominate, report them very infrequently.
-    static const uint64 kCorruptReportsPerMillion = 10000;
-    if (rand < kCorruptReportsPerMillion)
-      ReportCorrupt(db, startup_kb);
-  } else if (error == SQLITE_READONLY) {
-    // SQLITE_READONLY appears similar to SQLITE_CORRUPT - once it
-    // is seen, it is almost guaranteed to be seen again.
-    reported = true;
-
-    if (rand < kReportsPerMillion)
-      ReportError(db, extended_error);
-  } else {
-    // Only set the flag when making a report.  This should allow
-    // later (potentially different) errors in a stream of errors to
-    // be reported.
-    //
-    // TODO(shess): Would it be worthwile to audit for which cases
-    // 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);
-    }
-  }
+  if (rand <= kReportsPerMillion)
+    db->ReportDiagnosticInfo(extended_error, stmt);
 }
 
 // NOTE(shess): Schema modifications must consider initial creation in
@@ -549,7 +409,6 @@ void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) {
 
 void DatabaseErrorCallback(sql::Connection* db,
                            const base::FilePath& db_path,
-                           size_t startup_kb,
                            HistoryBackendClient* backend_client,
                            int extended_error,
                            sql::Statement* stmt) {
@@ -558,7 +417,7 @@ void DatabaseErrorCallback(sql::Connection* db,
   // see how to reach that.
 
   if (backend_client && backend_client->ShouldReportDatabaseError()) {
-    GenerateDiagnostics(db, startup_kb, extended_error);
+    GenerateDiagnostics(db, extended_error, stmt);
   }
 
   // Attempt to recover corrupt databases.
@@ -1205,14 +1064,9 @@ bool ThumbnailDatabase::RetainDataForPageUrls(
 
 sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db,
                                                 const base::FilePath& db_name) {
-  size_t startup_kb = 0;
-  int64 size_64;
-  if (base::GetFileSize(db_name, &size_64))
-    startup_kb = static_cast<size_t>(size_64 / 1024);
-
   db->set_histogram_tag("Thumbnail");
   db->set_error_callback(base::Bind(&DatabaseErrorCallback,
-                                    db, db_name, startup_kb, backend_client_));
+                                    db, db_name, backend_client_));
 
   // Thumbnails db now only stores favicons, so we don't need that big a page
   // size or cache.