RESOLVED FIXED 219628
Add max age for a root to be reused.
https://bugs.webkit.org/show_bug.cgi?id=219628
Summary Add max age for a root to be reused.
dewei_zhu
Reported 2020-12-07 21:20:48 PST
Add max age for a root to be reused.
Attachments
Patch (15.47 KB, patch)
2020-12-07 21:33 PST, dewei_zhu
rniwa: review+
dewei_zhu
Comment 1 2020-12-07 21:33:14 PST
Ryosuke Niwa
Comment 2 2020-12-07 21:45:44 PST
Comment on attachment 415611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415611&action=review > Websites/perf.webkit.org/public/include/manifest-generator.php:53 > + 'maxReuseRootAgeInDays' => config('maxReuseRootAgeInDays'), max*RootReuse*AgeInDays > Websites/perf.webkit.org/public/v3/models/build-request.js:100 > + const earliestAllowRootCreationDate = rawManifest.maxReuseRootAgeInDays ? > + Date.now() - rawManifest.maxReuseRootAgeInDays * 24 * 3600 * 1000 : 0; I'd call this earliestRootCreatingTimeForReuse > Websites/perf.webkit.org/public/v3/models/commit-set.js:77 > - areAllRootsAvailable() > + areAllRootsAvailableAndNewerThan(earliestCreationDate) I don't think it's necessary to call it "andNewerThan". Also, I'd call the new argument earliestCreationTime instead. > Websites/perf.webkit.org/public/v3/models/manifest.js:25 > static fetch() Why don't we make this async as well? > Websites/perf.webkit.org/public/v3/models/manifest.js:34 > + return await RemoteAPI.getJSON('/data/manifest.json'); Neat. > Websites/perf.webkit.org/public/v3/models/manifest.js:35 > + } catch(error) { We shouldn't swallow all errors though. Just 404. We should probably add a test for that. > Websites/perf.webkit.org/unit-tests/build-request-tests.js:470 > + it('should not reuse a root that older than "maxReuseRootAge"', async () => { that *is* older than.
dewei_zhu
Comment 3 2020-12-09 15:13:07 PST
Comment on attachment 415611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415611&action=review >> Websites/perf.webkit.org/public/v3/models/manifest.js:35 >> + } catch(error) { > > We shouldn't swallow all errors though. Just 404. > We should probably add a test for that. Added in the change.
dewei_zhu
Comment 4 2020-12-09 15:14:15 PST
Landed in r270607.
Note You need to log in before you can comment on or make changes to this bug.