Last Comment Bug 764587 - When downloading a complete mar after the partial fails it shouldn't be throttled.
: When downloading a complete mar after the partial fails it shouldn't be throt...
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: 15 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla16
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on:
Blocks: bgupdates 759065
  Show dependency treegraph
 
Reported: 2012-06-13 14:23 PDT by Robert Strong [:rstrong] (use needinfo to contact me)
Modified: 2012-06-26 09:24 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Patch (v1) (3.25 KB, patch)
2012-06-13 19:00 PDT, :Ehsan Akhgari
robert.strong.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 14:23:55 PDT
Spinoff of bug 764377.

Specifically see bug 764377 comment #6 through bug 764377 comment #11
Comment 1 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 14:43:36 PDT
Ehsan, it *might* make sense to download unthrottled when it fails in this case but I haven't had a chance to think it through
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#1027
Comment 2 :Ehsan Akhgari 2012-06-13 14:43:58 PDT
Hmm, isn't this just a dupe of bug 764377?
Comment 3 :Ehsan Akhgari 2012-06-13 14:45:06 PDT
(In reply to Robert Strong [:rstrong] (do not email) from comment #1)
> Ehsan, it *might* make sense to download unthrottled when it fails in this
> case but I haven't had a chance to think it through
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1027

Hmm, if we have concerns about our mirror network capacity, I'd guess that the same concerns apply here too, right?  Cause most people want to download and use the partials, which are way smaller than complete mars.
Comment 4 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 14:46:27 PDT
No, what he is asking for is that background downloads aren't throttled in the way they have always been. See bug 764377 comment #26, specifically, "Even in the case where the UI is not displayed it makes no sense to make a 30 second download take almost an hour and a half..."
Comment 5 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 14:48:08 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #3)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #1)
> > Ehsan, it *might* make sense to download unthrottled when it fails in this
> > case but I haven't had a chance to think it through
> > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> > nsUpdateService.js#1027
> 
> Hmm, if we have concerns about our mirror network capacity, I'd guess that
> the same concerns apply here too, right?  Cause most people want to download
> and use the partials, which are way smaller than complete mars.
Only if it is a common case. For example, when we are unable to apply a mar we download unthrottled when we start back up. So yes, it is a concern if this is common and it is not if it is not common.
Comment 6 :Ehsan Akhgari 2012-06-13 14:48:34 PDT
Heh, ok fair enough.  :-)
Comment 7 :Ehsan Akhgari 2012-06-13 14:49:01 PDT
(In reply to Robert Strong [:rstrong] (do not email) from comment #5)
> (In reply to Ehsan Akhgari [:ehsan] from comment #3)
> > (In reply to Robert Strong [:rstrong] (do not email) from comment #1)
> > > Ehsan, it *might* make sense to download unthrottled when it fails in this
> > > case but I haven't had a chance to think it through
> > > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> > > nsUpdateService.js#1027
> > 
> > Hmm, if we have concerns about our mirror network capacity, I'd guess that
> > the same concerns apply here too, right?  Cause most people want to download
> > and use the partials, which are way smaller than complete mars.
> Only if it is a common case. For example, when we are unable to apply a mar
> we download unthrottled when we start back up. So yes, it is a concern if
> this is common and it is not if it is not common.

To the best of my knowledge, it should not be common.  But I don't have hard evidence to support this claim.  :-)
Comment 8 Bill Gianopoulos [:WG9s] 2012-06-13 15:02:49 PDT
The reason this is common is probably because of bug 764269.
Comment 9 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 15:03:52 PDT
Understood and both regressions need to be fixed.
Comment 10 Bill Gianopoulos [:WG9s] 2012-06-13 15:05:16 PDT
I really wish you people would follow these bugs without me having to point things out to you particularly since you seem to be adverse to accepting my input on things.
Comment 11 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 15:10:29 PDT
Can you please stop with these type of comments. I and others follow the app update component so yes, I am following this and the other bugs. Your input has been and is accepted... I gave you a path forward by filing a mirror network bug to get your input regarding the mirror network into the hands of the people that can do something with it but you seem dead set on trying to get those that can't do anything with it directly to use it. After I have some breathing room I'll even file the bug if you don't but as far as this bug is concerned we already understand what is going on as I explained to you several times in bug 764377 so no more input is needed.
Comment 12 Bill Gianopoulos [:WG9s] 2012-06-13 15:13:06 PDT
(In reply to Robert Strong [:rstrong] (do not email) from comment #11)
> Can you please stop with these type of comments. I and others follow the app
> update component so yes, I am following this and the other bugs. Your input
> has been and is accepted... I gave you a path forward by filing a mirror
> network bug to get your input regarding the mirror network into the hands of
> the people that can do something with it but you seem dead set on trying to
> get those that can't do anything with it directly to use it. After I have
> some breathing room I'll even file the bug if you don't but as far as this
> bug is concerned we already understand what is going on as I explained to
> you several times in bug 764377 so no more input is needed.

But my issue is that I don't think that this is a mirror network issue.  Not sure why you don't get this.  I guess you really don't pay any attention to what I really say in my comments.
Comment 13 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 15:17:24 PDT
It is a mirror network issue in that the throttling was implemented to prevent the mirror network from being overloaded. Any changes to the client side throttling would first need to be evaluated and agreed to by the people that manage the mirror networks.
Comment 14 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 15:18:45 PDT
And PLEASE, take your comments back to bug 764377. This bug is specifically about the regression caused by bgupdates.
Comment 15 Bill Gianopoulos [:WG9s] 2012-06-13 15:25:13 PDT
This is a bug I filed.  It is about what I say it is about and not what you say it is about.
Comment 16 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-13 15:26:32 PDT
No, I filed this bug to specifically be about the regression for bgupdates. You filed bug 764377.
Comment 17 :Ehsan Akhgari 2012-06-13 18:35:34 PDT
(In reply to Robert Strong [:rstrong] (do not email) from comment #1)
> Ehsan, it *might* make sense to download unthrottled when it fails in this
> case but I haven't had a chance to think it through
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/
> nsUpdateService.js#1027

After more thought, I think this makes sense.  Patch forthcoming.
Comment 18 :Ehsan Akhgari 2012-06-13 19:00:29 PDT
Created attachment 632992 [details] [diff] [review]
Patch (v1)
Comment 20 Nick Thomas [:nthomas] 2012-06-14 19:24:46 PDT
Could you summarise the change this makes ? What scenarios will be download the complete at full speed where we would have done background before ?
Comment 21 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-14 19:27:14 PDT
Previously, if the partial download failed or the mar didn't pass any of the checks it would download the complete. This would be done in the UI and it would be without throttling. This makes it so it will do the same for the background updates case.
Comment 22 Robert Strong [:rstrong] (use needinfo to contact me) 2012-06-14 19:27:52 PDT
background staging of the update that is
Comment 23 Nick Thomas [:nthomas] 2012-06-14 20:23:00 PDT
Thanks Robert.
Comment 24 :Ehsan Akhgari 2012-06-19 15:13:26 PDT
Comment on attachment 632992 [details] [diff] [review]
Patch (v1)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 759065
User impact if declined: See comment 0
Testing completed (on m-c, etc.): Both locally and has baked for a few days on m-c
Risk to taking this patch (and alternatives if risky): Minimal
String or UUID changes made by this patch: none
Comment 25 Alex Keybl [:akeybl] 2012-06-19 19:48:21 PDT
Comment on attachment 632992 [details] [diff] [review]
Patch (v1)

[Triage Comment]
Let's land on Aurora 15 in support of re-enabling the feature.
Comment 27 :Ehsan Akhgari 2012-06-26 09:24:36 PDT
*** Bug 768417 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.