Bug 219628 – Add max age for a root to be reused.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

Add max age for a root to be reused.

Created attachment 415611 [details]
Patch

[Ryosuke Niwa]

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.

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.

Landed in r270607.