Skip to content

Commit

Permalink
Histogram versions and extended error codes for SQLite databases.
Browse files Browse the repository at this point in the history
Track database versions across the fleet for purposes of making
decisions about whether old migration code can be removed.

As part of this, refactor histogram code to key off a
per-database tag rather than histogram name, and to log in a
form which can be fanned out via the field trial logic in
histograms.xml

[FYI michaeln from webkit/OWNERS, erikwright from {chrome,content}/net/OWNERS, sky for chrome/browser/history/OWNERS]

BUG=none
TBR=michaeln@chromium.org, erikwright@chromium.org, sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/14976003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200216 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
shess@chromium.org committed May 15, 2013
1 parent 0b0032a commit 210ce0a
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 50 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/history/history_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ HistoryDatabase::~HistoryDatabase() {

sql::InitStatus HistoryDatabase::Init(const base::FilePath& history_name,
sql::ErrorDelegate* error_delegate) {
db_.set_error_histogram_name("Sqlite.History.Error");
db_.set_histogram_tag("History");

// Set the exceptional sqlite error handler.
db_.set_error_delegate(error_delegate);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/history/text_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ bool TextDatabase::Init() {
return false;
}

db_.set_error_histogram_name("Sqlite.Text.Error");
db_.set_histogram_tag("Text");

// Set the database page size to something a little larger to give us
// better performance (we're typically seek rather than bandwidth limited).
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/history/thumbnail_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ sql::InitStatus ThumbnailDatabase::Init(

sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db,
const base::FilePath& db_name) {
db->set_error_histogram_name("Sqlite.Thumbnail.Error");
db->set_histogram_tag("Thumbnail");

// Thumbnails db now only stores favicons, so we don't need that big a page
// size or cache.
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/history/top_sites_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ bool TopSitesDatabase::RemoveURL(const MostVisitedURL& url) {
sql::Connection* TopSitesDatabase::CreateDB(const base::FilePath& db_name) {
scoped_ptr<sql::Connection> db(new sql::Connection());
// Settings copied from ThumbnailDatabase.
db->set_error_histogram_name("Sqlite.Thumbnail.Error");
db->set_histogram_tag("TopSites");
db->set_page_size(4096);
db->set_cache_size(32);

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/net/sqlite_server_bound_cert_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ void SQLiteServerBoundCertStore::Backend::LoadOnDBThread(
UMA_HISTOGRAM_COUNTS("DomainBoundCerts.DBSizeInKB", db_size / 1024 );

db_.reset(new sql::Connection);
db_->set_error_histogram_name("Sqlite.DomainBoundCerts.Error");
db_->set_histogram_tag("DomainBoundCerts");
db_->set_error_delegate(new KillDatabaseErrorDelegate(this));

if (!db_->Open(path_)) {
Expand Down
2 changes: 1 addition & 1 deletion components/webdata/common/web_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ sql::InitStatus WebDatabase::Init(const base::FilePath& db_name) {
if (!content::NotificationService::current())
notification_service_.reset(content::NotificationService::Create());

db_.set_error_histogram_name("Sqlite.Web.Error");
db_.set_histogram_tag("Web");

// We don't store that much data in the tables so use a small page size.
// This provides a large benefit for empty tables (which is very likely with
Expand Down
2 changes: 1 addition & 1 deletion content/browser/net/sqlite_persistent_cookie_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
UMA_HISTOGRAM_COUNTS("Cookie.DBSizeInKB", db_size / 1024 );

db_.reset(new sql::Connection);
db_->set_error_histogram_name("Sqlite.Cookie.Error");
db_->set_histogram_tag("Cookie");
db_->set_error_delegate(new KillDatabaseErrorDelegate(this));

if (!db_->Open(path_)) {
Expand Down
49 changes: 21 additions & 28 deletions sql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/metrics/sparse_histogram.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
#include "base/utf_string_conversions.h"
Expand Down Expand Up @@ -708,35 +709,27 @@ void Connection::StatementRefDeleted(StatementRef* ref) {
open_statements_.erase(i);
}

void Connection::AddTaggedHistogram(const std::string& name,
size_t sample) const {
if (histogram_tag_.empty())
return;

// TODO(shess): The histogram macros create a bit of static storage
// for caching the histogram object. This code shouldn't execute
// often enough for such caching to be crucial. If it becomes an
// issue, the object could be cached alongside histogram_prefix_.
std::string full_histogram_name = name + "." + histogram_tag_;
base::HistogramBase* histogram =
base::SparseHistogram::FactoryGet(
full_histogram_name,
base::HistogramBase::kUmaTargetedHistogramFlag);
if (histogram)
histogram->Add(sample);
}

int Connection::OnSqliteError(int err, sql::Statement *stmt) {
// Strip extended error codes.
int base_err = err&0xff;

static size_t kSqliteErrorMax = 50;
UMA_HISTOGRAM_ENUMERATION("Sqlite.Error", base_err, kSqliteErrorMax);
if (base_err == SQLITE_IOERR) {
// TODO(shess): Consider folding the IOERR range into the main
// histogram directly. Perhaps 30..49? The downside risk would
// be that SQLite core adds a bunch of codes and this becomes a
// complicated mapping.
static size_t kSqliteIOErrorMax = 20;
UMA_HISTOGRAM_ENUMERATION("Sqlite.Error.IOERR", err>>8, kSqliteIOErrorMax);
}

if (!error_histogram_name_.empty()) {
// TODO(shess): The histogram macros create a bit of static
// storage for caching the histogram object. Since SQLite is
// being used for I/O, generally without error, this code
// shouldn't execute often enough for such caching to be crucial.
// If it becomes an issue, the object could be cached alongside
// error_histogram_name_.
base::HistogramBase* histogram =
base::LinearHistogram::FactoryGet(
error_histogram_name_, 1, kSqliteErrorMax, kSqliteErrorMax + 1,
base::HistogramBase::kUmaTargetedHistogramFlag);
if (histogram)
histogram->Add(base_err);
}
UMA_HISTOGRAM_SPARSE_SLOWLY("Sqlite.Error", err);
AddTaggedHistogram("Sqlite.Error", err);

// Always log the error.
LOG(ERROR) << "sqlite error " << err
Expand Down
18 changes: 11 additions & 7 deletions sql/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,17 @@ class SQL_EXPORT Connection {
error_delegate_.reset(delegate);
}

// SQLite error codes for errors on all connections are logged to
// enum histogram "Sqlite.Error". Setting this additionally logs
// errors to the histogram |name|.
void set_error_histogram_name(const std::string& name) {
error_histogram_name_ = name;
// Set this tag to enable additional connection-type histogramming
// for SQLite error codes and database version numbers.
void set_histogram_tag(const std::string& tag) {
histogram_tag_ = tag;
}

// Record a sparse UMA histogram sample under
// |name|+"."+|histogram_tag_|. If |histogram_tag_| is empty, no
// histogram is recorded.
void AddTaggedHistogram(const std::string& name, size_t sample) const;

// Initialization ------------------------------------------------------------

// Initializes the SQL connection for the given file, returning true if the
Expand Down Expand Up @@ -497,8 +501,8 @@ class SQL_EXPORT Connection {
// commands or statements. It can be null which means default handling.
scoped_ptr<ErrorDelegate> error_delegate_;

// Auxiliary error-code histogram.
std::string error_histogram_name_;
// Tag for auxiliary histograms.
std::string histogram_tag_;

DISALLOW_COPY_AND_ASSIGN(Connection);
};
Expand Down
2 changes: 2 additions & 0 deletions sql/meta_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ bool MetaTable::Init(Connection* db, int version, int compatible_version) {
// there, we should create an index.
SetVersionNumber(version);
SetCompatibleVersionNumber(compatible_version);
} else {
db_->AddTaggedHistogram("Sqlite.Version", GetVersionNumber());
}
return transaction.Commit();
}
Expand Down
73 changes: 68 additions & 5 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7150,11 +7150,14 @@ other types of suffix sets.
</histogram>

<histogram name="Sqlite.Error" enum="SqliteErrorCode">
<summary>Error codes returned by sqlite for all databases.</summary>
<summary>SQLite extended error codes.</summary>
</histogram>

<histogram name="Sqlite.Error.IOERR" enum="SqliteIOERRCode">
<summary>Error codes returned by sqlite for all databases.</summary>
<obsolete>
Replaced 5/14/2013 by expanded Sqlite.Error histogram.
</obsolete>
<summary>SQLite extended SQLITE_IOERR codes for all databases.</summary>
</histogram>

<histogram name="Sqlite.History.Error" enum="SqliteErrorCode">
Expand All @@ -7173,6 +7176,10 @@ other types of suffix sets.
<summary>Error codes returned by sqlite for the thumbnail db.</summary>
</histogram>

<histogram name="Sqlite.Version">
<summary>Version of pre-existing database at startup.</summary>
</histogram>

<histogram name="Sqlite.Web.Error" enum="SqliteErrorCode">
<summary>Error codes returned by sqlite the web db.</summary>
</histogram>
Expand Down Expand Up @@ -11261,7 +11268,7 @@ other types of suffix sets.
</enum>

<enum name="SqliteErrorCode" type="int">
<summary>Error codes returned by SQLite - see sqlite.h</summary>
<summary>Error codes returned by SQLite - see sqlite3.h</summary>
<int value="0" label="SQLITE_OK">Successful result</int>
<int value="1" label="SQLITE_ERROR">SQL error or missing database</int>
<int value="2" label="SQLITE_INTERNAL">
Expand Down Expand Up @@ -11308,7 +11315,7 @@ other types of suffix sets.
<int value="23" label="SQLITE_AUTH">Authorization denied</int>
<int value="24" label="SQLITE_FORMAT">Auxiliary database format error</int>
<int value="25" label="SQLITE_RANGE">
2nd parameter to sqlite3_bind out of range
2nd parameter to sqlite3_bind() out of range
</int>
<int value="26" label="SQLITE_NOTADB">
File opened that is not a database file
Expand All @@ -11317,10 +11324,50 @@ other types of suffix sets.
<int value="101" label="SQLITE_DONE">
sqlite3_step() has finished executing
</int>
<int value="261" label="SQLITE_BUSY_RECOVERY">TBD</int>
<int value="262" label="SQLITE_LOCKED_SHAREDCACHE">TBD</int>
<int value="266" label="SQLITE_IOERR_READ">Error reading from file</int>
<int value="270" label="SQLITE_CANTOPEN_NOTEMPDIR">TBD</int>
<int value="522" label="SQLITE_IOERR_SHORT_READ">Short read from file</int>
<int value="778" label="SQLITE_IOERR_WRITE">
Error writing to file (other than SQLITE_FULL)
</int>
<int value="1034" label="SQLITE_IOERR_FSYNC">Error syncing to disk</int>
<int value="1290" label="SQLITE_IOERR_DIR_FSYNC">
Error syncing directory changes to disk
</int>
<int value="1546" label="SQLITE_IOERR_TRUNCATE">Error truncating file</int>
<int value="1802" label="SQLITE_IOERR_FSTAT">Error reading file metadata</int>
<int value="2058" label="SQLITE_IOERR_UNLOCK">Error unlocking file</int>
<int value="2314" label="SQLITE_IOERR_RDLOCK">
Error getting read lock - should not be possible
</int>
<int value="2570" label="SQLITE_IOERR_DELETE">Error deleting file</int>
<int value="2826" label="SQLITE_IOERR_BLOCKED">
Deadlock due to other process access to SQLite files
</int>
<int value="3082" label="SQLITE_IOERR_NOMEM">Error mapping shared memory</int>
<int value="3338" label="SQLITE_IOERR_ACCESS">
Error getting file attributes (other than not found)
</int>
<int value="3594" label="SQLITE_IOERR_CHECKRESERVEDLOCK">
Error while querying lock status
</int>
<int value="3850" label="SQLITE_IOERR_LOCK">Error acquiring lock</int>
<int value="4106" label="SQLITE_IOERR_CLOSE">Error closing file</int>
<int value="4362" label="SQLITE_IOERR_DIR_CLOSE">Unused</int>
<int value="4618" label="SQLITE_IOERR_SHMOPEN">Error mmapping file</int>
<int value="4874" label="SQLITE_IOERR_SHMSIZE">
Error in stat while mmapping file
</int>
<int value="5130" label="SQLITE_IOERR_SHMLOCK">Unused</int>
</enum>

<enum name="SqliteIOERRCode" type="int">
<summary>Extended error codes returned by SQLite - see sqlite.h</summary>
<obsolete>
Replaced 5/14/2013 by expanded Sqlite.Error histogram.
</obsolete>
<summary>Extended error codes returned by SQLite - see sqlite3.h</summary>
<int value="0" label="SQLITE_IOERR">No extended code given</int>
<int value="1" label="SQLITE_IOERR_READ">Error reading from file</int>
<int value="2" label="SQLITE_IOERR_SHORT_READ">Short read from file</int>
Expand Down Expand Up @@ -13009,6 +13056,22 @@ other types of suffix sets.
<affected-histogram name="Net.SpdySettingsCwnd"/>
</fieldtrial>

<fieldtrial name="SqliteDatabases" separator=".">
<group name="AppCache" label="AppCache"/>
<group name="Cookie" label="Cookie"/>
<group name="DatabaseTracker" label="DatabaseTracker"/>
<group name="DomainBoundCerts" label="DomainBoundCerts"/>
<group name="DomStorageDatabase" label="DomStorageDatabase"/>
<group name="History" label="History"/>
<group name="Quota" label="Quota"/>
<group name="Text" label="Text"/>
<group name="Thumbnail" label="Thumbnail"/>
<group name="TopSites" label="TopSites"/>
<group name="Web" label="Web"/>
<affected-histogram name="Sqlite.Error"/>
<affected-histogram name="Sqlite.Version"/>
</fieldtrial>

<fieldtrial name="SSLFalseStart">
<group name="FalseStart_enabled"/>
<group name="FalseStart_disabled"/>
Expand Down
2 changes: 1 addition & 1 deletion webkit/appcache/appcache_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ bool AppCacheDatabase::LazyOpen(bool create_if_needed) {
db_.reset(new sql::Connection);
meta_table_.reset(new sql::MetaTable);

db_->set_error_histogram_name("Sqlite.AppCache.Error");
db_->set_histogram_tag("AppCache");

bool opened = false;
if (use_in_memory_db) {
Expand Down
2 changes: 1 addition & 1 deletion webkit/database/database_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ bool DatabaseTracker::LazyInit() {
return false;
}

db_->set_error_histogram_name("Sqlite.DatabaseTracker.Error");
db_->set_histogram_tag("DatabaseTracker");

databases_table_.reset(new DatabasesTable(db_.get()));
meta_table_.reset(new sql::MetaTable());
Expand Down
2 changes: 1 addition & 1 deletion webkit/quota/quota_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ bool QuotaDatabase::LazyOpen(bool create_if_needed) {
db_.reset(new sql::Connection);
meta_table_.reset(new sql::MetaTable);

db_->set_error_histogram_name("Sqlite.Quota.Error");
db_->set_histogram_tag("Quota");

bool opened = false;
if (in_memory_only) {
Expand Down

0 comments on commit 210ce0a

Please sign in to comment.