Skip to content

GitLab

T
tangled
Commit ac7b23b2 authored by Brett Wilson's avatar Brett Wilson
Browse files
Options

History init: add metrics and skip migration. 

Removes the migration code to history DB version 34. This version did a
large migration of the URL table which can use several MB of disk storage. The
suspicion is that this is causing more initialization failures for users with
full devices.

The change in version 34 is to add an AUTOINCREMENT tag on the primary key.
This is required for some sync work that is in progress but is not complete,
so skipping this migration step won't break any users. We will need to fix
this before rolling out the new sync features.

The 34 change also removed the favicon_id column. Because some databases
may have the column and some won't, the code to copy to the in-memory
database (which previously was implicit) now lists the columns to copy
explicitly.

Adds UMA logging for history database initialization failures, as well as
versions for migration failures. Logging for the Sqlite error code for the
various failure states may have been nice but would have required more
plumbing and seemed impractical for now.

There is one behavior change the error logging resulted in: errors migrating
to version 22 were previously ignored but are now fatal. I believe continuing
in this case was a mistake. This will only affect users migrating from an
Android Chrome version prior to 2012, and failures here will likely be fatal
anyway.

BUG=734194, 736136
R=dtrainor@chromium.org, gangwu@chromium.org, mpearson@chromium.org, yzshen@chromium.org

Review-Url: https://codereview.chromium.org/2954593002 .
Cr-Commit-Position: refs/heads/master@{#481958}
parent f06b5d19
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 178 additions and 77 deletions
components/history/core/browser/history_backend.cc
View file @ ac7b23b2
......@@ -651,6 +651,9 @@ void HistoryBackend::InitImpl(
bool kill_db = scheduled_kill_db_;
if (kill_db)
KillHistoryDatabase();
// The frequency of this UMA will indicate how often history
// initialization fails.
UMA_HISTOGRAM_BOOLEAN("History.AttemptedToFixProfileError", kill_db);
} // Falls through.
case sql::INIT_TOO_NEW: {
......
components/history/core/browser/in_memory_database.cc
View file @ ac7b23b2
......@@ -79,8 +79,23 @@ bool InMemoryDatabase::InitFromDisk(const base::FilePath& history_name) {
// Copy URL data to memory.
base::TimeTicks begin_load = base::TimeTicks::Now();
// Need to explicitly specify the column names here since databases on disk
// may or may not have a favicon_id column, but the in-memory one will never
// have it. Therefore, the columns aren't guaranteed to match.
//
// TODO(https://crbug.com/736136) Once we can guarantee that the favicon_id
// column doesn't exist with migration code, this can be replaced with the
// simpler:
// "INSERT INTO urls SELECT * FROM history.urls WHERE typed_count > 0"
// which does not require us to keep the list of columns in sync. However,
// we may still want to keep the explicit columns as a safety measure.
if (!db_.Execute(
"INSERT INTO urls SELECT * FROM history.urls WHERE typed_count > 0")) {
"INSERT INTO urls "
"(id, url, title, visit_count, typed_count, last_visit_time, hidden) "
"SELECT "
"id, url, title, visit_count, typed_count, last_visit_time, hidden "
"FROM history.urls WHERE typed_count > 0")) {
// Unable to get data from the history database. This is OK, the file may
// just not exist yet.
}
......
components/history/core/browser/url_database.cc
View file @ ac7b23b2
......@@ -586,19 +586,32 @@ bool URLDatabase::CreateURLTable(bool is_temporary) {
sql.append(name);
sql.append(
"("
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
// Using AUTOINCREMENT is for sync propose. Sync uses this |id| as an
// unique key to identify the URLs. If here did not use AUTOINCREMENT, and
// Sync was not working somehow, a ROWID could be deleted and re-used
// The id uses AUTOINCREMENT is for sync propose. Sync uses this |id| as
// an unique key to identify the URLs. If here did not use AUTOINCREMENT,
// and Sync was not working somehow, a ROWID could be deleted and re-used
// during this period. Once Sync come back, Sync would use ROWIDs and
// timestamps to see if there are any updates need to be synced. And sync
// will only see the new URL, but missed the deleted URL.
// will only see the new URL, but missed the deleted URL.
//
// IMPORTANT NOTE: Currently new tables are created with AUTOINCREMENT
// but the migration code is disabled. This means that you will not
// be able to count on AUTOINCREMENT behavior without adding
// additional migration steps.
//
// Along with this, an unused favicon_id column will exist for tables
// without AUTOINCREMENT. This should be removed everywhere.
//
// TODO(https://crbug.com/736136) figure out how to update users to use
// AUTOINCREMENT and remove the favicon_id column consistently.
"id INTEGER PRIMARY KEY AUTOINCREMENT,"
"url LONGVARCHAR,"
"title LONGVARCHAR,"
"visit_count INTEGER DEFAULT 0 NOT NULL,"
"typed_count INTEGER DEFAULT 0 NOT NULL,"
"last_visit_time INTEGER NOT NULL,"
"hidden INTEGER DEFAULT 0 NOT NULL)");
// IMPORTANT: If you change the colums, also update in_memory_database.cc
// where the values are copied (InitFromDisk).
return GetDB().Execute(sql.c_str());
}
......
tools/metrics/histograms/enums.xml
View file @ ac7b23b2
......@@ -18768,6 +18768,15 @@ uploading your change for review. These are checked by presubmit scripts.
</int>
</enum>
<enum name="HistoryInitStep">
<int value="0" label="Open"/>
<int value="1" label="Transaction begin"/>
<int value="2" label="Meta table init"/>
<int value="3" label="Create tables"/>
<int value="4" label="Version check and migration (check History.MigrateFailureToVersion)"/>
<int value="5" label="Commit"/>
</enum>
<enum name="HistoryPageView">
<int value="0" label="History"/>
<int value="1" label="Grouped Week (Obsolete Feb. 2017)"/>
tools/metrics/histograms/histograms.xml
View file @ ac7b23b2
......@@ -24028,6 +24028,20 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="History.AttemptedToFixProfileError">
<owner>brettw@chromium.org</owner>
<summary>
Logged whenever history database initialization fails. The frequency of
logging will tell you the total failure rate. True indicate that we think
the database is non-recoverable and it will be deleted. It should be
re-created the next time Chrome is started. False indicates we think the
error is temporary (like out of disk space or file locked). Chrome will run
with no history database and nothing will be done to try to fix the error.
See History.InitializationFailureStep for which particular step of history
initialization failed.
</summary>
</histogram>
<histogram
name="History.ClearBrowsingData.HistoryNoticeShownInFooterWhenUpdated"
enum="BooleanShown">
......@@ -24265,6 +24279,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="History.InitializationFailureStep" enum="HistoryInitStep">
<owner>brettw@chromium.org</owner>
<summary>
The phase of history initialization that failed. This histogram is only
logged on database initialization failure.
History.AttemptedToFixProfileError will tell how often initialization fails
overall.
</summary>
</histogram>
<histogram name="History.InMemoryDBItemCount">
<owner>gab@chromium.org</owner>
<summary>
......@@ -24389,6 +24413,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="History.MigrateFailureFromVersion">
<owner>brettw@chromium.org</owner>
<summary>
History database version from which history migration failed. If there are
higher than normal migration failures, this histogram will indicate which
migration step failed.
</summary>
</histogram>
<histogram name="History.MonthlyHostCount">
<owner>shess@chromium.org</owner>
<summary>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment