Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2954593002:
History init: add metrics and skip migration. (Closed)
DescriptionHistory 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}
Committed: https://chromium.googlesource.com/chromium/src/+/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973
Patch Set 1 #Patch Set 2 : Fixes #Patch Set 3 : Fixes #
Total comments: 1
Patch Set 4 : Comment typo #
Total comments: 2
Patch Set 5 : Copy column names #Patch Set 6 : Explicit enum values #
Messages
Total messages: 35 (18 generated)
Description was changed from ========== 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 severam 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. 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. BUG=734194, 736136 ========== to ========== 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 severam 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. 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 ==========
Description was changed from ========== 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 severam 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. 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 ========== to ========== 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 severam 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. 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 ==========
Fixes
brettw@chromium.org changed reviewers: + dtrainor@chromium.org, gangwu@chromium.org
Fixes
The CQ bit was checked by brettw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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 severam 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. 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 ========== to ========== 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. 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 ==========
yzshen@chromium.org changed reviewers: + yzshen@chromium.org
LGTM https://codereview.chromium.org/2954593002/diff/40001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2954593002/diff/40001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:24387: + History database version from which which history migration failed. If nit: duplicate "which"
Comment typo
Description was changed from ========== 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. 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 ========== to ========== 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. 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. TBR=mpearson for histograms owners BUG=734194, 736136 ==========
brettw@chromium.org changed reviewers: + mpearson@chromium.org - dtrainor@chromium.org
+mpearson TBR for histograms owners (this is blocking release, will address any feedback in a followup)
The CQ bit was checked by brettw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org Link to the patchset: https://codereview.chromium.org/2954593002/#ps60001 (title: "Comment typo")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dtrainor@chromium.org changed reviewers: + dtrainor@chromium.org
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
metrics lgtm for the record, modulo one comment about labeling/comments in a .cc --mark https://codereview.chromium.org/2954593002/diff/60001/components/history/core... File components/history/core/browser/history_database.cc (right): https://codereview.chromium.org/2954593002/diff/60001/components/history/core... components/history/core/browser/history_database.cc:61: enum class InitStep { Please follow the best practices for enum histograms here: https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histo...
lgtm
Copy column names
Description was changed from ========== 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. 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. TBR=mpearson for histograms owners BUG=734194, 736136 ========== to ========== 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. TBR=mpearson for histograms owners BUG=734194, 736136 ==========
Explicit enum values
The CQ bit was checked by brettw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gangwu@chromium.org, dtrainor@chromium.org, yzshen@chromium.org, mpearson@chromium.org Link to the patchset: https://codereview.chromium.org/2954593002/#ps100001 (title: "Explicit enum values")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2954593002/diff/60001/components/history/core... File components/history/core/browser/history_database.cc (right): https://codereview.chromium.org/2954593002/diff/60001/components/history/core... components/history/core/browser/history_database.cc:61: enum class InitStep { Done (I had to do a new patch anyway). Thanks!
Description was changed from ========== 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. TBR=mpearson for histograms owners BUG=734194, 736136 ========== to ========== 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 ==========
Description was changed from ========== 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 ========== to ========== 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} Committed: https://chromium.googlesource.com/chromium/src/+/ac7b23b2ea06f2826e7b2bca7747... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) manually as ac7b23b2ea06f2826e7b2bca77475b2c11ce0973 (presubmit successful). |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
