999019 - Parallel download in the background

Status: RESOLVED FIXED

(Testing :: mozregression, defect)

Description

[William Lachance (:wlach)]

[import from github]

darchons:

Depending on the connection, it can take a while to download a nightly. mozregression should download the next two bisecting nightlies in the background while the user is testing the current one. Depending on the whether the current one is good or bad, it can abort downloading one of the two nightlies.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This might download unnecessary builds for the user. This can become annoying if you have a limited bandwidth. Instead it would be nice if the download could be split up in multiple chunks, which could then be downloaded in parallel. That's something similar we also want to add to mozdownload.

Comment 3

[Julien Pagès (:parkouss)]

I think this is a good idea, but that could be annoying indeed. Maybe we could do this only if a flag is given on the command line ? Like --background-download for example.

Comment 4

[Harrison]

I would be interested in doing this, however I would need a bit of help with getting around the codebase.

Comment 5

[YF (Yang)]

(In reply to Harrison from comment #4)
> I would be interested in doing this, however I would need a bit of help with
> getting around the codebase.

https://github.com/mozilla/mozregression

Comment 6

[Julien Pagès (:parkouss)]

Hey Harrison,

For now, builds are downloaded from here:
https://github.com/mozilla/mozregression/blob/master/mozregression/launchers.py#L53

And this is only triggered by:
https://github.com/mozilla/mozregression/blob/master/mozregression/test_runner.py#L52
called by
https://github.com/mozilla/mozregression/blob/master/mozregression/bisector.py#L273

Wich basically means that for each build we want to test, we download it unless persist
option has been defined (in such case, we reuse a previously downloaded build if available):
https://github.com/mozilla/mozregression/blob/master/mozregression/launchers.py#L45

From my point of view, first thing to do is to to write a new class to handle the downloading
/ reusing downloads. Let's call this DownloadManager. I would say that this class must be able to:

 - download one file (and block)
 - start the downloading of two files in the background, with the possibility to stop one or both later
 - continue the downloading of one file that was done in background and wait (block until done)
 - This class must be able to reuse downloaded builds (do not download again the same thing)
 - If persist option is used, we download and keep those files in a user defined dir, else we can use
   a temp dir that we will delete at the end of the session

There is also a tricky thing about progress indicator - It should be activated when we download one build alone,
deactivated when we download two at the same time, then reactivated when we know which build is important to download
and that we block for downloading it.

This is the biggest part of the job. Once we have this, we could instantiate this class in the main.py file (or in the bisector.py), pass it around to the bisector.Bisector instance and use it there. We will need to remove some code in the Runner classes that will be handled by DownloadManager. Also we may have to remove some stuff in utils.py that will probably move (I'm thinking about the download_url function).

Yup, some interesting work! First step is the DownloadManager, and I propose a new file for this, 'download_manager.py' (surprise :)).

I suppose this class must be initialized with the persist option. Also, I think that download method(s) may take build dict info as parameter, as returned here:

https://github.com/mozilla/mozregression/blob/master/mozregression/bisector.py#L271

Well, I think this is good enough for a start. Please let me know if you have any question.

Comment 7

[Harrison]

That sounds great, I'm sure I'll have questions soon.

how do I get assigned again?

Comment 8

[Rahul Ranjan]

Is this issue is still open? I would like to contribute to it.

Comment 9

[Julien Pagès (:parkouss)]

Yes the issue is open.

Harrison, are you still working on this bug ?

Comment 10

[Julien Pagès (:parkouss)]

Rahul, in the same time you can look at bug 1106804 if you are interested. This is a good first bug, much simpler than this one to get started into mozregression hacking.

If you're interested, please add a comment to say so in bug 1106804, and I will assign you the bug.

Comment 11

[Harrison]

I'm fine with being dropped from this bug. Unfortunately school, started meaning I can't contribute as much as I would have liked to!

Comment 12

[Julien Pagès (:parkouss)]

Ok Harrison, thanks for letting us know!

You're welcome any time if you want to contribute later!

Comment 13

[Julien Pagès (:parkouss)]

Assigning this to myself, I would like to work on it :)

Comment 14

[Julien Pagès (:parkouss)]

oups

Comment 15

[Julien Pagès (:parkouss)]

Hey, I got something working for this. There are some big changes, but before asking for review, I would like you Will to git it a try.

So basically, it downloads next builds in background while we are evaluating one. Once user picked the next build (good or bad), this downloads continue and the other one is canceled.

To find the nex builds to downloads, we have to find the next mid points (left and right) of the current center. So the time before testing the first build is increased. In practice however, I can't really notice this side effect (you'll tell me).

Once first build is downloaded, wait a few seconds, then pick something between g/b. You will see that the progress bar does not starts from 0. :)

Tell me what you think about it. I feel it is good, and very usable. If you agree on that, I'll continue the work (mainly to add unitestests and clear things a bit).

You can try it from my download-background branch: https://github.com/parkouss/mozregression/tree/download-background.

Comment 16

[William Lachance (:wlach)]

This looks good! I did some quick testing and this really speeds up the bisection process.

Two bits of feedback:

1. Should we clean up the downloaded files as we go along? It looks like our store of downloaded files could get quite big, depending on the length of bisection.
2. We should probably provide a command-line option to disable this behavior.

Comment 17

[Julien Pagès (:parkouss)]

(In reply to William Lachance (:wlach) from comment #16)
> This looks good! I did some quick testing and this really speeds up the
> bisection process.

Excellent.

> 1. Should we clean up the downloaded files as we go along? It looks like our
> store of downloaded files could get quite big, depending on the length of
> bisection.

Yeah, I don't know about that. All I can say is that files are removed when the download is not finished, or at the end of the session (given that --persist flag is not here). I would say that this is good how it is, but maybe that can be annoying for some users that does not have plenty of disk space. In that case, it is possible to add the deletion of background downloads that we did not used I suppose. Your choice. :)

> 2. We should probably provide a command-line option to disable this behavior.

Sure. Having an option to disable this seems good, and this implies that we active it by default (I think that is a good idea), unlike what I said in comment 3. I'll add an option to disable this.

Comment 18

[Julien Pagès (:parkouss)]

Attachment: Download next builds in background

Yep, got the job done.

yay, I'm sorry in advance for the huge review here. I have tested it, unit tested it, seems good to me.

This does not include the removing of useless download files as we go along. that could be added pretty easily I'm sure, so you tell me.

Comment 19

[William Lachance (:wlach)]

Comment on attachment 8573216 [details] [review]
Download next builds in background

Hey, sorry for the delay here. I didn't give this an extremely thorough review, but from a 10 minute scan it looks good. Let's release a new version with this enabled and see how it goes. :)

Comment 20

[Julien Pagès (:parkouss)]

Great, I merged this in. :)