New issue
Advanced search Search tips

Issue 736222 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug


Participants' hotlists:
Downloads-Framework-Service


Sign in to add a comment

[Download Service] Handle startup component failures

Project Member Reported by dtrainor@chromium.org, Jun 23 2017

Issue description

We need to handle failures from the underlying components (Model, Driver, and FileMonitor).  If any of these fail we probably want to drop all jobs and notify the Client.

Current behavior:
(1) If any component fails we make the service effectively unavailable and notify the Client.

Future behavior:
(1) If any component fails, we nuke all state and start from scratch.
(2) We notify the Client that we had a bad startup.

It would be worth considering whether or not we still have "unrecoverable" failures that cause the service to effectively turn off though (we could just reject all StartDownload() requests with an UNKNOWN error to the callback).
 
Owner: dtrainor@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6358cc30ae742c0da6289da16c8ee4925f38827

commit c6358cc30ae742c0da6289da16c8ee4925f38827
Author: David Trainor <dtrainor@chromium.org>
Date: Thu Jul 13 00:17:19 2017

Add a recovery method to FileMonitor

Add a method that resets FileMonitor in a hard recovery attempt.  This
CL will, with subsequent CLs that add similar recovery methods to other
components, allow recovering (or at least cleaning up) a failed
DownloadService Controller.

The cleanup steps are:
1. Try to erase all files in the directory.
2. Attempt to re-initialize the component (make sure the directory is
created and accessed).
3. Return the result of the re-initialization to the caller.

BUG= 736222 

Change-Id: Ic5a8ad63c836eedd37aebaa817fb201a9a36663f
Reviewed-on: https://chromium-review.googlesource.com/567865
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486176}
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/controller_impl.cc
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/controller_impl_unittest.cc
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/file_monitor.h
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/file_monitor_impl.cc
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/file_monitor_impl.h
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/file_monitor_unittest.cc
[modify] https://crrev.com/c6358cc30ae742c0da6289da16c8ee4925f38827/components/download/internal/stats.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43ea8e9ea440430b3cdc028fea962b742ff7b8f8

commit 43ea8e9ea440430b3cdc028fea962b742ff7b8f8
Author: David Trainor <dtrainor@chromium.org>
Date: Fri Jul 14 03:00:35 2017

Add recovery pipeline for DownloadDriver

Add the infrastructure for a DownloadDriver recovery flow.  This CL
doesn't actually perform any cleanup or recovery of a DownloadDriver,
but it just returns success.  Implementing the actual recovery code will
be included in a follow up, but this allows us to implement the
DownloadService comprehensive recovery code without blocking on the
implementation of this component in particular.

BUG= 736222 

Change-Id: Ibbbb8d64b5114593404339d514bafb8b7d984a9b
Reviewed-on: https://chromium-review.googlesource.com/569073
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486654}
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/content/internal/BUILD.gn
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/content/internal/download_driver_impl.cc
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/content/internal/download_driver_impl.h
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/content/internal/download_driver_impl_unittest.cc
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/internal/controller_impl.cc
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/internal/controller_impl.h
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/internal/download_driver.h
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/internal/test/test_download_driver.cc
[modify] https://crrev.com/43ea8e9ea440430b3cdc028fea962b742ff7b8f8/components/download/internal/test/test_download_driver.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bcf45bcb0681524eba2dc117cccb309f14d3d08a

commit bcf45bcb0681524eba2dc117cccb309f14d3d08a
Author: David Trainor <dtrainor@chromium.org>
Date: Mon Jul 17 18:14:39 2017

Add recovery method to the Model

Add the way for the DownloadService Model to attempt a hard recovery.
This will be used along with other component CLs to allow the
DownloadService to attempt to reset itself.

The recovery method for the Model happens via the following steps:
1. Clears the Model of all Entries.
2. Has the Store destroy itself.
3. The Store attempts to re-initialize.
4. Return the result back to the Model::Client.

BUG= 736222 

Change-Id: I4e0a256d0b241ef627d457d9c2276691d61da8ce
Reviewed-on: https://chromium-review.googlesource.com/568712
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487160}
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/controller_impl.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/controller_impl.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/download_store.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/download_store.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/download_store_unittest.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/model.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/model_impl.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/model_impl.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/model_impl_unittest.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/store.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/test/mock_model_client.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/test/noop_store.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/test/noop_store.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/test/test_store.cc
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/download/internal/test/test_store.h
[modify] https://crrev.com/bcf45bcb0681524eba2dc117cccb309f14d3d08a/components/leveldb_proto/testing/fake_db.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79c98dd4756772f458ccc7989d10e9d002aff168

commit 79c98dd4756772f458ccc7989d10e9d002aff168
Author: David Trainor <dtrainor@chromium.org>
Date: Tue Jul 18 19:52:44 2017

Move Controller setup to a state machine

As part of the effort to build in recovery states to the controller, we
need to add a basic state machine so it's possible to control the new
set of states: CREATED, INITIALIZED, READY, RECOVERING, and UNAVAILABLE.

BUG= 736222 

Change-Id: I22811afece7f7d491c235247c33ba593f142bbc1
Reviewed-on: https://chromium-review.googlesource.com/567652
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487564}
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/controller.h
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/controller_impl.cc
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/controller_impl.h
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/controller_impl_unittest.cc
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/download_service_impl.cc
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/download_service_impl_unittest.cc
[modify] https://crrev.com/79c98dd4756772f458ccc7989d10e9d002aff168/components/download/internal/test/mock_controller.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2430c59b71c4a842226ec3f5024fe880ed95d6c

commit f2430c59b71c4a842226ec3f5024fe880ed95d6c
Author: David Trainor <dtrainor@chromium.org>
Date: Wed Jul 19 05:13:29 2017

Implement download service recovery

Tie together the three download service subcomponent recovery mechanisms
into one recovery flow in the controller state machine.  This will
attempt to recovery the DownloadDriver, Model, and FileMonitor if any
fail to initialize.  If recovery fails, the service will be unavailable.
However if recovery is successful, the service will initialize and
notify the client that state was lost.

BUG= 736222 

Change-Id: Ieacc4c4bf1d4916a8eaaf3815a743a595e75ae35
Reviewed-on: https://chromium-review.googlesource.com/575357
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Jian Li <jianli@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487759}
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/controller_impl.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/controller_impl.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/controller_impl_unittest.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/model.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/model_impl.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/model_impl_unittest.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/startup_status.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/startup_status.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/stats.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/stats.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/test/empty_client.cc
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/test/empty_client.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/test/mock_client.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/internal/test/mock_model_client.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/components/download/public/client.h
[modify] https://crrev.com/f2430c59b71c4a842226ec3f5024fe880ed95d6c/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment