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.
Will, Elbart, what do you think of it ? Do you use that option, and/or find it useful ?
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.
(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. :)
(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.
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.
(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.
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.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Created attachment 8649741 [details] [review] 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.
Attachment #8649741 - Flags: review?(sabergeass)
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! :)
Attachment #8649741 - Flags: review?(sabergeass) → review+
Thanks! Landed in https://github.com/mozilla/mozregression/commit/d1ad66d150c6f8fdcbadd73abe871cdef8fb2433 and https://github.com/mozilla/mozregression/commit/6ce5be582b509bca4d2ff078af862350d4ebb30c.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.