New issue
Advanced search Search tips

Issue 737730 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Component Updater downloader is not cleaning up stale jobs.

Project Member Reported by sorin@chromium.org, Jun 28 2017

Issue description

On Windows, stale jobs are left behind and may clog up BITS queue.

The component updater has a mechanism to clean up jobs that are older than 7 days.

There are several reasons why jobs could be left behind, for instance browser restarts, crashes, system restarts, etc. BITS has a default policy to cancel jobs after 90 days of inactivity but we need something more aggressive.

The component updater is using the job description to find the jobs it has started. Due to a bug in the code that read the job description, stale jobs were never found, and consequently never clean up.

Further changes may include defining a UMA metric to report the number of Chrome BITS jobs in the BITS queue, and possibly, a more aggressive clean up schedule.

 

Comment 1 by sorin@chromium.org, Jun 29 2017

Components: Internals>Installer>Components
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 29 2017

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

commit dfad2747b8013ad4535a03cc3679586b469ac1c5
Author: Sorin Jianu <sorin@chromium.org>
Date: Thu Jun 29 20:39:50 2017

Component Updater downloader is not cleaning up stale jobs.

The component updater is using the job description to find the jobs it
has started. Due to a bug in the code that read the job description,
stale jobs were never found, and consequently never clean up.

TBR=waffles

Bug:  737730 
Change-Id: I4fd3f615261f76d5b568a9a9e8f32658dbd57197
Reviewed-on: https://chromium-review.googlesource.com/556499
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483478}
[modify] https://crrev.com/dfad2747b8013ad4535a03cc3679586b469ac1c5/components/update_client/background_downloader_win.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 30 2017

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

commit 4782af57f14cd7acf259b381d4b47f1d6974d56c
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jun 30 16:06:37 2017

Define a UMA stat to count the number of component updater BITS jobs.

We'd like to have an idea how effective the component updater is at
cleaning up orphaned jobs in the BITS queue. The component updater has
a mechanism to clean up jobs that are older than 7 days. There could be
several reasons why jobs are left behind, such as browser restarts,
crashes, system restarts, or simply code bugs.

TBR=waffles

Bug:  737730 
Change-Id: I7eee79a1f9126846e0620b670c91c72bda774786
Reviewed-on: https://chromium-review.googlesource.com/556479
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483718}
[modify] https://crrev.com/4782af57f14cd7acf259b381d4b47f1d6974d56c/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/4782af57f14cd7acf259b381d4b47f1d6974d56c/tools/metrics/histograms/histograms.xml

Project Member

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

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

commit 8a439977cb7e98358e41cdbb532db92d42f8f14d
Author: Sorin Jianu <sorin@chromium.org>
Date: Thu Jul 06 01:06:26 2017

Better handle errors in BackgroundDownloader.

The previous code had several execution paths where the job creation
would return early and leave a partially initialized job in the
BITS queue.

This change streamlines the job creation so that the job is cleaned up
if errors occur during creation.

TBR=waffles
Bug:737730

Change-Id: I7f97196a2874919fabd5da56bf47a08c491dd721
Reviewed-on: https://chromium-review.googlesource.com/559870
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484433}
[modify] https://crrev.com/8a439977cb7e98358e41cdbb532db92d42f8f14d/components/update_client/background_downloader_win.cc

Project Member

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

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

commit be8dfafebb1fc65da3397385759d4487e49e8bf9
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jul 07 02:14:12 2017

Use WRL::ComPtr instead of deprecated ScopedComPtr.

R=ganesh
TBR=waffles

Bug:  737730 
Change-Id: I2d2627325119522f09e276c213b87a2a41ff79e4
Reviewed-on: https://chromium-review.googlesource.com/560798
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484795}
[modify] https://crrev.com/be8dfafebb1fc65da3397385759d4487e49e8bf9/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/be8dfafebb1fc65da3397385759d4487e49e8bf9/components/update_client/background_downloader_win.h

Project Member

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

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

commit 4d41e9494f5b314cd29f9549976eca95eadf6f5f
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jul 07 15:54:23 2017

Simplify the predicates in the background downloader code.

The new code uses functional binders and partial application to
get rid of some boilerplate code.

Due to name resolution of overloaded functions in the std::functional, 
the predicates must take the ComPtr argument by value instead of
reference to const.

TBR=waffles

Bug:  737730 
Change-Id: I87d518b5e4d1b9a2fa4802ce00627cf01b887a81
Reviewed-on: https://chromium-review.googlesource.com/562909
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484941}
[modify] https://crrev.com/4d41e9494f5b314cd29f9549976eca95eadf6f5f/components/update_client/background_downloader_win.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 7 2017

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

commit cc3f9c3f3c57b1a4e0c506828932c0c46170e723
Author: Sorin Jianu <sorin@chromium.org>
Date: Fri Jul 07 22:19:06 2017

Limit the number of BITS jobs created by the component updater.

Also, make the clean up of stale jobs more aggressive.

TBR=waffles

Bug:  737730 
Change-Id: I61bf9831aa5f753003c10ef50169a81a2545792a
Reviewed-on: https://chromium-review.googlesource.com/563765
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485071}
[modify] https://crrev.com/cc3f9c3f3c57b1a4e0c506828932c0c46170e723/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/cc3f9c3f3c57b1a4e0c506828932c0c46170e723/components/update_client/background_downloader_win.h
[modify] https://crrev.com/cc3f9c3f3c57b1a4e0c506828932c0c46170e723/components/update_client/update_client_errors.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 10 2017

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

commit 90a387f2b11ef858d6cff5f5b8779f380954b170
Author: Sorin Jianu <sorin@chromium.org>
Date: Mon Jul 10 19:43:02 2017

Added a UMA stat to for new/old BITS jobs in the component updater.

The component updater can look into the BITS queue to see if
a job already exist to download a given component. This stat
provides an insight of whether this optimization is useful.

TBR=waffles

Bug:  737730 
Change-Id: I56b31ad7b382197970b2dc49e8b42a7620a8e943
Reviewed-on: https://chromium-review.googlesource.com/564178
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485352}
[modify] https://crrev.com/90a387f2b11ef858d6cff5f5b8779f380954b170/components/update_client/background_downloader_win.cc
[modify] https://crrev.com/90a387f2b11ef858d6cff5f5b8779f380954b170/components/update_client/background_downloader_win.h
[modify] https://crrev.com/90a387f2b11ef858d6cff5f5b8779f380954b170/tools/metrics/histograms/histograms.xml

Comment 9 by sorin@chromium.org, Jul 11 2017

Status: Fixed (was: Started)

Comment 10 by sorin@chromium.org, Oct 12 2017

Status: Started (was: Fixed)
User feedback shows that the issue is still present in 61.0.3163.100.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12 2017

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

commit f8c12a97e59d719564a334379f11203fbe500d3b
Author: Sorin Jianu <sorin@chromium.org>
Date: Thu Oct 12 20:04:56 2017

Handle errors when calling GetBackgroundDownloaderJobCount.

The implementation of BackgroundDownloader::QueueBitsJob defaults to
creating a new BITS job when the count of BITS jobs can't be read.

This could result in flooding the BITS queue with too many jobs.

Bug:  737730 
Change-Id: Ifd26d151384fa34510bbe507734ad48ff9707e71
Reviewed-on: https://chromium-review.googlesource.com/716704
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508416}
[modify] https://crrev.com/f8c12a97e59d719564a334379f11203fbe500d3b/components/update_client/background_downloader_win.cc

Comment 12 by sorin@chromium.org, Dec 11 2017

Status: Fixed (was: Started)

Sign in to add a comment