Need to add recovery to the offline pages SQL implementation
Reported by
bburns@chromium.org,
Apr 20 2016
|
||||
Issue descriptionCurrently we don't detect (or recover from) corrupted SQL Lite DBs. This is ok, since it's not in use yet. However, before we turn it on, we need to enable this. See: https://code.google.com/p/chromium/codesearch#chromium/src/components/history/core/browser/thumbnail_database.cc&sq=package:chromium&type=cs&l=428&rcl=1461151052 for an example.
,
Sep 27 2016
,
Nov 8 2016
In the process of metrics analysis of this problem we discovered that clients, which hit the store load problems fall into 2 categories with respect to LoadStatus enum (where possible values are: LOAD_SUCCEEDED, STORE_INIT_FAILED, STORE_LOAD_FAILED, DATA_PARSING_FAILED). The categories are: * Clients that consistently fail, showing 2, 2, 2, 2 (STORE_LOAD_FAILED) with every load, * Clients that fail the load sometimes: 1 (STORE_INIT_FAILED), 0 (LOAD_SUCCEEDED), 0, 1, 1, 1, 0, which sometimes (presumably on slower devices) takes the shape of 1, 1, 1, 1, 1, 1, ... The former case can be solved by doing the appropriate store reset and starting over. (After all these clients never get to load the store properly, so they don't have access to appropriate pages). The latter case is one that exhibits a race condition between initial connection to the store and first load from the store. If the latter attempt is started before the former is finished, we are guaranteed to fail with LoadStatus::STORE_INIT_FAILED and the user cannot access offline page metadata, even thou there is a perfectly good database waiting for them. We will address both problems as part of this bug.
,
Nov 8 2016
Marking as start and taking this thing on. Bumping priority because of race condition and impact on users with slower devices.
,
Nov 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c4ec432de012e9f6c99dfc55cd0535694a4e828 commit 9c4ec432de012e9f6c99dfc55cd0535694a4e828 Author: fgorski <fgorski@chromium.org> Date: Thu Nov 17 21:59:19 2016 [Offline pages] Resetting offline page metadata store to handle LOAD/INIT failures * Adds a store reset to offline page model, in case the initial load fails with STORE_LOAD_FAILED status * Adds Initialize method to the store to handle STORE_INIT_FAILED. * Postpones OfflinePageModel actions until after initialization of the store succeeds/fails. * Adds tests to cover successful reset and unsuccessful one. * Updates tests that needed adjusting. BUG= 605249 Review-Url: https://codereview.chromium.org/2497703002 Cr-Commit-Position: refs/heads/master@{#432984} [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store.h [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_impl_unittest.cc [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_sql.cc [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_sql.h [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl.cc [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl.h [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl_unittest.cc [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_test_store.cc [modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_test_store.h
,
Dec 1 2016
The following patch sorted the problem on RequestQueueStore side: [Offline pages] Adding InitializeStoreTask to handle RQ store initialization and resets * Adds RequestQueueStore::Initialize * Adds InitializeStoreTask with tests. * Updates RequestQueue to call store Initialize (using task) * Updating tests for all RequestQueue related tasks (adds initialization) * Moving in-memory store to test_support target * Updating in-memory store with init/reset test scenarios * Fixes minor bugs * Removes unnecessary methods BUG= 605249 , 651238 R=dougarnett@chromium.org,petewil@chromium.org Committed: https://crrev.com/f871f672d0c9a16ded3374d0ed8667295b13898e Cr-Commit-Position: refs/heads/master@{#435333}
,
Dec 1 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by fgor...@chromium.org
, Sep 9 2016