[sql] Cleanup open and close error histograms.

sql/connection.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 "sql/connection.h"
 
 #include <string.h>
 
 #include "base/files/file_path.h"
 #include "base/file_util.h"
@@ skipping 218 matching lines @@
   open_statements_.clear();
 
   if (db_) {
     // Call to AssertIOAllowed() cannot go at the beginning of the function
     // because Close() must be called from destructor to clean
     // statement_cache_, it won't cause any disk access and it most probably
     // will happen on thread not allowing disk access.
     // TODO(paivanof@gmail.com): This should move to the beginning
     // of the function. http://crbug.com/136655.
     AssertIOAllowed();
     // TODO(shess): Histogram for failure.

[Scott Hess - ex-Googler, 2013/07/22 17:57:40]

Erp, removing this TODO.

-    sqlite3_close(db_);
+    int rc = sqlite3_close(db_);
+    if (rc != SQLITE_OK) {
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.CloseFailure", rc);
+      DLOG(FATAL) << "sqlite3_close failed: " << GetErrorMessage();
+    }
     db_ = NULL;
   }
 }
 
 void Connection::Close() {
   // If the database was already closed by RazeAndClose(), then no
   // need to close again.  Clear the |poisoned_| bit so that incorrect
   // API calls are caught.
   if (poisoned_) {
     poisoned_ = false;
@@ skipping 460 matching lines @@
   // RazeAndClose().  Until regular Close() is called, the caller
   // should be treating the database as open, but is_open() currently
   // only considers the sqlite3 handle's state.
   // TODO(shess): Revise is_open() to consider poisoned_, and review
   // to see if any non-testing code even depends on it.
   DLOG_IF(FATAL, poisoned_) << "sql::Connection is already open.";
   poisoned_ = false;
 
   int err = sqlite3_open(file_name.c_str(), &db_);
   if (err != SQLITE_OK) {
+    // Extended error codes cannot be enabled until a handle is
+    // available, fetch manually.
+    err = sqlite3_extended_errcode(db_);
+
     // Histogram failures specific to initial open for debugging
     // purposes.
-    UMA_HISTOGRAM_ENUMERATION("Sqlite.OpenFailure", err & 0xff, 50);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.OpenFailure", err);
 
     OnSqliteError(err, NULL);
     Close();
     db_ = NULL;
     return false;
   }
 
   // SQLite uses a lookaside buffer to improve performance of small mallocs.
   // Chromium already depends on small mallocs being efficient, so we disable
   // this to avoid the extra memory overhead.
   // This must be called immediatly after opening the database before any SQL
   // statements are run.
   sqlite3_db_config(db_, SQLITE_DBCONFIG_LOOKASIDE, NULL, 0, 0);
 
+  // Enable extended result codes to provide more color on I/O errors.
+  // Not having extended result codes is not a fatal problem, as
+  // Chromium code does not attempt to handle I/O errors anyhow.  The
+  // current implementation always returns SQLITE_OK, the DCHECK is to
+  // quickly notify someone if SQLite changes.
+  err = sqlite3_extended_result_codes(db_, 1);
+  DCHECK_EQ(err, SQLITE_OK) << "Could not enable extended result codes";
+
   // sqlite3_open() does not actually read the database file (unless a
   // hot journal is found).  Successfully executing this pragma on an
   // existing database requires a valid header on page 1.
   // TODO(shess): For now, just probing to see what the lay of the
   // land is.  If it's mostly SQLITE_NOTADB, then the database should
   // be razed.
   err = ExecuteAndReturnErrorCode("PRAGMA auto_vacuum");
   if (err != SQLITE_OK)
-    UMA_HISTOGRAM_ENUMERATION("Sqlite.OpenProbeFailure", err & 0xff, 50);
-
-  // Enable extended result codes to provide more color on I/O errors.
-  // Not having extended result codes is not a fatal problem, as
-  // Chromium code does not attempt to handle I/O errors anyhow.  The
-  // current implementation always returns SQLITE_OK, the DCHECK is to
-  // quickly notify someone if SQLite changes.
-  err = sqlite3_extended_result_codes(db_, 1);
-  DCHECK_EQ(err, SQLITE_OK) << "Could not enable extended result codes";
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.OpenProbeFailure", err);
 
 #if defined(OS_IOS) && defined(USE_SYSTEM_SQLITE)
   // The version of SQLite shipped with iOS doesn't enable ICU, which includes
   // REGEXP support. Add it in dynamically.
   err = sqlite3IcuInit(db_);
   DCHECK_EQ(err, SQLITE_OK) << "Could not enable ICU support";
 #endif  // OS_IOS && USE_SYSTEM_SQLITE
 
   // If indicated, lock up the database before doing anything else, so
   // that the following code doesn't have to deal with locking.
@@ skipping 134 matching lines @@
   }
 
   // Best effort to put things back as they were before.
   const char kNoWritableSchema[] = "PRAGMA writable_schema = OFF";
   ignore_result(Execute(kNoWritableSchema));
 
   return ret;
 }
 
 }  // namespace sql