Add history AUTOINCREMENT migration code |
|||||||
Issue descriptionThe migration code for migrating users to use AUTOINCREMENT for the History URL table was removed in an attempt to address bug 734194 . As a result, some users will have been migrated, and some won't have been, even when they're at the most recent version. With it, the favicon_id colum was removed. This means that some databases will have this column and some won't. If removing the migration doesn't fix the issue, we just need to add a new version number and add back the migration. -- If removing the version 34 migration does resolve the issue, then we need to figure out how to get users updated. Some ideas: 1. Just add back the migration code and tell users with the problem to free up space on their phone. Browser features will be in a weird state until they do this. 2. Same as #1 but add some UI messaging about not enough space. Then at least users know what's going on. 3. Do the migration but ignore errors and continue if it fails. Such users won't get AUTOINCREMENT and may experience weird sync effects, but there will be no obvious feature failures. 4. The same as #3 but write detection code to check of the AUTOINCREMENT isn't there and fix it if there's enough space on the device. Presumably the user will free up space at some point and we can fix the problem. In the mean time we can -- My preferred solution is #2: add an error notification. We can always hit SQLITE_FULL during initialization even if we use trivial amounts of space. This will cause strange effects and it's best just to tell users to fix the problem. Secondarily, I think we should check all history operations for SQLITE_FULL and show this error, since runtime errors, even if they aren't as fatal as initialization ones, can still cause very strange effects like failed downloads. -- We will also need to make the existence of the favicon_id column consistent (it shouldn't be around).
,
Jun 23 2017
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973 commit ac7b23b2ea06f2826e7b2bca77475b2c11ce0973 Author: Brett Wilson <brettw@chromium.org> Date: Fri Jun 23 18:13:21 2017 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} [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/history_backend.cc [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/history_database.cc [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/in_memory_database.cc [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/components/history/core/browser/url_database.cc [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/tools/metrics/histograms/enums.xml [modify] https://crrev.com/ac7b23b2ea06f2826e7b2bca77475b2c11ce0973/tools/metrics/histograms/histograms.xml
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/56680659ec10ca2d931cf1787b836ce481e46ded commit 56680659ec10ca2d931cf1787b836ce481e46ded Author: Brett Wilson <brettw@chromium.org> Date: Fri Jun 23 18:30:58 2017 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-Original-Commit-Position: refs/heads/master@{#481958} Review-Url: https://codereview.chromium.org/2952333003 . Cr-Commit-Position: refs/branch-heads/3071@{#822} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/history_backend.cc [modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/history_database.cc [modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/in_memory_database.cc [modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/components/history/core/browser/url_database.cc [modify] https://crrev.com/56680659ec10ca2d931cf1787b836ce481e46ded/tools/metrics/histograms/histograms.xml
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/567c0200d67292ebdc86d894ca5348acc3e2dea2 commit 567c0200d67292ebdc86d894ca5348acc3e2dea2 Author: Alex Mineer <amineer@chromium.org> Date: Fri Jun 23 20:44:08 2017 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 (cherry picked from commit ac7b23b2ea06f2826e7b2bca77475b2c11ce0973) Review-Url: https://codereview.chromium.org/2954593002 . Cr-Original-Commit-Position: refs/heads/master@{#481958} Cr-Commit-Position: refs/branch-heads/3139@{#7} Cr-Branched-From: 2dd0ea84298d45e0962c595c591529bc84dd0ebd-refs/heads/master@{#481757} [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/history_backend.cc [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/history_database.cc [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/in_memory_database.cc [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/components/history/core/browser/url_database.cc [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/tools/metrics/histograms/enums.xml [modify] https://crrev.com/567c0200d67292ebdc86d894ca5348acc3e2dea2/tools/metrics/histograms/histograms.xml
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/81fc74ee0a399613c2f4d054c363744ccfe2e291 commit 81fc74ee0a399613c2f4d054c363744ccfe2e291 Author: Brett Wilson <brettw@chromium.org> Date: Mon Jun 26 17:26:45 2017 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-Original-Commit-Position: refs/heads/master@{#481958} Review-Url: https://codereview.chromium.org/2956913002 . Cr-Commit-Position: refs/branch-heads/3112@{#465} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/history_backend.cc [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/history_database.cc [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/in_memory_database.cc [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/components/history/core/browser/url_database.cc [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/tools/metrics/histograms/enums.xml [modify] https://crrev.com/81fc74ee0a399613c2f4d054c363744ccfe2e291/tools/metrics/histograms/histograms.xml
,
Jul 21 2017
,
Oct 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b commit 20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b Author: Gang Wu <gangwu@chromium.org> Date: Mon Oct 09 23:56:15 2017 [USS] Re-Migration AUTOINCREMENT to "urls" table in history DB This is re-do work for CL https://codereview.chromium.org/2721713002, Since the CL got disabled by crbug.com/734194 . This CL will pump up the history DB version from 36 to 37, and migrate old "urls" table to new "urls" table which contains "AUTOINCREMENT". For efficiency, we will check if existing "urls" table contains "AUTOINCREMENT". Bug: 736136 Change-Id: I14c53257c361d3f6673f9da21791caf7ff471dbd Reviewed-on: https://chromium-review.googlesource.com/699013 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Gang Wu <gangwu@chromium.org> Cr-Commit-Position: refs/heads/master@{#507544} [modify] https://crrev.com/20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b/components/history/core/browser/history_database.cc [modify] https://crrev.com/20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b/components/history/core/browser/url_database.cc [modify] https://crrev.com/20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b/components/history/core/browser/url_database.h [modify] https://crrev.com/20ca0fe32eb1998f54b6346fc3f5ad2600a6a41b/components/history/core/browser/url_database_unittest.cc
,
Oct 12 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by brettw@chromium.org
, Jun 23 2017