1195085 - remove --http-cache-dir option ?

Status: RESOLVED FIXED

(Testing :: mozregression, defect)

Description

[Julien Pagès (:parkouss)]

I personally never use the --http-cache-dir option, and I think it is not useful.

I don't see a great performance win with that option enabled, I doubt somebody uses that ? This is now completely useless for inbounds, as we use task cluster, and nighlies will be using taskcluster at some point too (maybe not for old builds though).

So I propose to remove that option, the related code and the dependency needed for it.

Comment 1

[Julien Pagès (:parkouss)]

Will, Elbart, what do you think of it ? Do you use that option, and/or find it useful ?

Comment 2

[Elbart]

Wasn't it added in case somebody retried a build without a persist-folder, in which case mozreg wouldn't unnecessarily redownload the build again?

I've never used it as an option to define a folder, as I figured that's what persist already covers.

Comment 3

[Julien Pagès (:parkouss)]

(In reply to Elbart from comment #2)
> Wasn't it added in case somebody retried a build without a persist-folder,
> in which case mozreg wouldn't unnecessarily redownload the build again?

Well it helped - because we had no proper solution at the time. Now this is handled in another way.

> I've never used it as an option to define a folder, as I figured that's what
> persist already covers.

Cool, nice to know that. I also think that persist folder is what we want. :)

Comment 4

[William Lachance (:wlach)]

(In reply to Julien Pagès from comment #3)
> (In reply to Elbart from comment #2)
> > Wasn't it added in case somebody retried a build without a persist-folder,
> > in which case mozreg wouldn't unnecessarily redownload the build again?
> 
> Well it helped - because we had no proper solution at the time. Now this is
> handled in another way.

How do we prevent additional inbound downloads now, if the user re-downloads a build (e.g. from running mozregression a second time)? I don't see how taskcluster helps with that.

Comment 5

[Julien Pagès (:parkouss)]

Will, I think what you are talking about is --persist, that allows to keep builds in a folder.

But builds that the user try again inside a run are no more re downloaded:

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

The http cache utility is only useful for caching http requests to url we are visiting (ie list urls inside a web page), but I think it is not really useful.

taskcluster helps because we are no more crawling web pages. Task cluster give us the url, and we download the file, that's all.

Comment 6

[William Lachance (:wlach)]

(In reply to Julien Pagès from comment #5)
> Will, I think what you are talking about is --persist, that allows to keep
> builds in a folder.

Nope. The whole intent of the --http-cache was to automatically and transparently keep a cache of downloaded builds *between* runs of mozregression: see bug 1019528. I think this functionality is useful and we should keep it-- perhaps even make it the default. Redownloading builds is pretty time consuming.

Comment 7

[Julien Pagès (:parkouss)]

So we discussed more this subject with Will. We came up with:

 - remove the http-cache-dir options
 - make a persist folder by default (no need to use it on cmdline or to configure in the configuration file)
 - add an option to limit the size of the persist folder (with a good default, like probably 1G)

So let's can keep this bug to remove the http-cache-dir option.

Comment 8

[Julien Pagès (:parkouss)]

Attachment: Remove http cache

MikeLing, I ask you for this review. This is mainly code removal, just added one test because the code was covered indirectly by some test I removed related to linitedfilecache.

Comment 9

[MikeLing]

Comment on attachment 8649741 [details] [review]
Remove http cache

I just test the PR locally and I think it works fine, thank you for your trust! :)

Comment 10

[Julien Pagès (:parkouss)]

Thanks!

Landed in https://github.com/mozilla/mozregression/commit/d1ad66d150c6f8fdcbadd73abe871cdef8fb2433 and https://github.com/mozilla/mozregression/commit/6ce5be582b509bca4d2ff078af862350d4ebb30c.