Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (270606 => 270607)
--- trunk/Websites/perf.webkit.org/ChangeLog 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2020-12-09 23:13:28 UTC (rev 270607)
@@ -1,3 +1,26 @@
+2020-12-09 Dewei Zhu <[email protected]>
+
+ Add max age for a root to be reused.
+ https://bugs.webkit.org/show_bug.cgi?id=219628
+
+ Reviewed by Ryosuke Niwa.
+
+ In order to prevent reusing a stale root, we should set a limit on the age of a root to be reused.
+ * public/include/manifest-generator.php: Added 'maxRootReuseAgeInDays' to manifest.
+ * public/v3/models/build-request.js: Added root age check.
+ (BuildRequest.prototype.async findBuildRequestWithSameRoots):
+ * public/v3/models/commit-set.js: Extended 'areAllRootsAvailable' with root age check.
+ (CommitSet.prototype.areAllRootsAvailable):
+ * public/v3/models/manifest.js:
+ (Manifest.fetch): Made it async.
+ (Manifest.async fetchRawResponse): Extract fetching raw manifest out so that 'maxRootReuseAgeInDays'
+ can be read without resetting other data models. Also added code to only fetch from API if requesting
+ /data/manifest.json returns 404.
+ (Manifest._didFetchManifest):
+ * server-tests/api-manifest-tests.js: Updated unit tests.
+ * unit-tests/build-request-tests.js: Updated unit tests and add new tests.
+ * unit-tests/manifest-test.js: Added unit tests.
+
2020-11-20 Dewei Zhu <[email protected]>
'run-analysis' script should schedule retries for A/B tests even after chart analysis failure.
Modified: trunk/Websites/perf.webkit.org/public/include/manifest-generator.php (270606 => 270607)
--- trunk/Websites/perf.webkit.org/public/include/manifest-generator.php 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/public/include/manifest-generator.php 2020-12-09 23:13:28 UTC (rev 270607)
@@ -50,6 +50,7 @@
'summaryPages' => config('summaryPages'),
'fileUploadSizeLimit' => config('uploadFileLimitInMB', 0) * 1024 * 1024,
'testAgeToleranceInHours' => config('testAgeToleranceInHours'),
+ 'maxRootReuseAgeInDays' => config('maxRootReuseAgeInDays'),
);
$this->elapsed_time = (microtime(true) - $start_time) * 1000;
Modified: trunk/Websites/perf.webkit.org/public/v3/models/build-request.js (270606 => 270607)
--- trunk/Websites/perf.webkit.org/public/v3/models/build-request.js 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/public/v3/models/build-request.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -95,6 +95,10 @@
let runningBuildRequest = null;
// Set ignoreCache = true as latest status of test groups is expected.
const allTestGroupsInTask = await TestGroup.fetchForTask(this.analysisTaskId(), true);
+ const rawManifest = await Manifest.fetchRawResponse();
+ const earliestRootCreatingTimeForReuse = rawManifest.maxRootReuseAgeInDays ?
+ Date.now() - rawManifest.maxRootReuseAgeInDays * 24 * 3600 * 1000 : 0;
+
for (const group of allTestGroupsInTask) {
if (group.id() == this.testGroupId())
continue;
@@ -107,7 +111,7 @@
continue;
if (!buildRequest.commitSet().equalsIgnoringRoot(this.commitSet()))
continue;
- if (!buildRequest.commitSet().areAllRootsAvailable())
+ if (!buildRequest.commitSet().areAllRootsAvailable(earliestRootCreatingTimeForReuse))
continue;
if (buildRequest.hasCompleted())
return buildRequest;
Modified: trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js (270606 => 270607)
--- trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -74,9 +74,10 @@
commitsWithTestability() { return this.commits().filter((commit) => !!commit.testability()); }
commits() { return Array.from(this._repositoryToCommitMap.values()); }
- areAllRootsAvailable()
+ areAllRootsAvailable(earliestCreationTime)
{
- return this.allRootFiles().every(rootFile => !rootFile.deletedAt() || this.customRoots().find(rootFile));
+ return this.allRootFiles().every(rootFile => (!rootFile.deletedAt() || this.customRoots().find(rootFile))
+ && rootFile.createdAt() >= earliestCreationTime);
}
revisionForRepository(repository)
Modified: trunk/Websites/perf.webkit.org/public/v3/models/manifest.js (270606 => 270607)
--- trunk/Websites/perf.webkit.org/public/v3/models/manifest.js 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/public/v3/models/manifest.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -22,14 +22,24 @@
Bug.clearStaticMap();
}
- static fetch()
+ static async fetch()
{
this.reset();
- return RemoteAPI.getJSON('/data/manifest.json').catch(function () {
- return RemoteAPI.getJSON('/api/manifest/');
- }).then(this._didFetchManifest.bind(this));
+ const rawManifest = await this.fetchRawResponse();
+ return this._didFetchManifest(rawManifest);
}
+ static async fetchRawResponse()
+ {
+ try {
+ return await RemoteAPI.getJSON('/data/manifest.json');
+ } catch(error) {
+ if (error != 404)
+ throw `Failed to fetch manifest.json with ${error}`
+ return await RemoteAPI.getJSON('/api/manifest/');
+ }
+ }
+
static _didFetchManifest(rawResponse)
{
Instrumentation.startMeasuringTime('Manifest', '_didFetchManifest');
@@ -91,6 +101,8 @@
siteTitle: rawResponse.siteTitle,
dashboards: rawResponse.dashboards, // FIXME: Add an abstraction around dashboards.
summaryPages: rawResponse.summaryPages,
+ testAgeToleranceInHours: rawResponse.testAgeToleranceInHours,
+ maxRootReuseAgeInDays: rawResponse.maxRootReuseAgeInDays,
}
}
}
Modified: trunk/Websites/perf.webkit.org/server-tests/api-manifest-tests.js (270606 => 270607)
--- trunk/Websites/perf.webkit.org/server-tests/api-manifest-tests.js 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/server-tests/api-manifest-tests.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -14,7 +14,8 @@
it("should generate an empty manifest when database is empty", () => {
return TestServer.remoteAPI().getJSON('/api/manifest').then((manifest) => {
assert.deepEqual(Object.keys(manifest).sort(), ['all', 'bugTrackers', 'builders', 'dashboard', 'dashboards',
- 'fileUploadSizeLimit', 'metrics', 'platformGroups', 'repositories', 'siteTitle', 'status', 'summaryPages', 'testAgeToleranceInHours', 'tests', 'triggerables']);
+ 'fileUploadSizeLimit', 'maxRootReuseAgeInDays', 'metrics', 'platformGroups', 'repositories', 'siteTitle',
+ 'status', 'summaryPages', 'testAgeToleranceInHours', 'tests', 'triggerables']);
assert.deepStrictEqual(manifest, {
siteTitle: TestServer.testConfig().siteTitle,
@@ -24,6 +25,7 @@
dashboard: {},
dashboards: {},
fileUploadSizeLimit: 2097152, // 2MB during testing.
+ maxRootReuseAgeInDays: null,
metrics: {},
platformGroups: {},
repositories: {},
Modified: trunk/Websites/perf.webkit.org/unit-tests/build-request-tests.js (270606 => 270607)
--- trunk/Websites/perf.webkit.org/unit-tests/build-request-tests.js 2020-12-09 23:08:25 UTC (rev 270606)
+++ trunk/Websites/perf.webkit.org/unit-tests/build-request-tests.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -1,6 +1,7 @@
'use strict';
const assert = require('assert');
+const crypto = require('crypto');
require('../tools/js/v3-models.js');
const MockModels = require('./resources/mock-v3-models.js').MockModels;
@@ -169,6 +170,7 @@
secondTestGroupOverrides = {};
if (!thirdTestGroupOverrides)
thirdTestGroupOverrides = {};
+ const yesterday = Date.now() - 24 * 3600 * 1000;
return {
"testGroups": [{
"id": "2128",
@@ -369,7 +371,7 @@
}],
"commitSets": [{
"id": "4255",
- "revisionItems": [{"commit": "87832"}, {"commit": "93116"}],
+ "revisionItems": [{"commit": "87832", rootFile: 101}, {"commit": "93116"}],
"customRoots": [],
}, {
"id": "4256",
@@ -397,7 +399,8 @@
"revision": "192736",
"time": 1448225325650
}],
- "uploadedFiles": [],
+ "uploadedFiles": [{id: 101, filename: 'root-101', extension: '.tgz', size: 1,
+ createdAt: yesterday, sha256: crypto.createHash('sha256').update('root-101').digest('hex')}],
"status": "OK"
};
}
@@ -427,6 +430,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(oneTestGroup());
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, null);
});
@@ -448,10 +457,44 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(overrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, BuildRequest.findById(16989))
});
+ it('should not reuse a root that is older than "maxReuseRootAge"', async () => {
+ const overrides = {
+ task: '1376',
+ platform: '32',
+ status: ['completed', 'pending', 'pending', 'pending']
+ }
+ const data = ""
+
+ const platformId = data.buildRequests[0].platform;
+ Platform.ensureSingleton(platformId, {id: platformId, metrics: [], name: 'some platform'});
+ const request = BuildRequest.constructBuildRequestsFromData(data)[0];
+ const promise = request.findBuildRequestWithSameRoots();
+ assert.equal(requests.length, 1);
+
+ assert.equal(requests[0].url, '/api/test-groups?task=1376');
+ assert.equal(requests[0].method, 'GET');
+ requests[0].resolve(threeTestGroups(overrides));
+
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 0.5});
+
+ const result = await promise;
+ assert.equal(result, null)
+ });
+
it('should not use cache while fetching test groups under analysis task', async () => {
const overrides = {
task: '1376',
@@ -469,6 +512,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(overrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
let result = await promise;
assert.equal(result, BuildRequest.findById(16989))
@@ -480,6 +529,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(overrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
result = await promise;
assert.equal(result, BuildRequest.findById(16989))
});
@@ -502,6 +557,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(overrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, null);
});
@@ -525,6 +586,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(overrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, BuildRequest.findById(16989))
});
@@ -551,6 +618,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(secondOverrides, thirdOverrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, BuildRequest.findById(16993))
});
@@ -577,6 +650,12 @@
assert.equal(requests[0].method, 'GET');
requests[0].resolve(threeTestGroups(secondOverrides, thirdOverrides));
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert.equal(requests[1].url, '/data/manifest.json');
+ assert.equal(requests[1].method, 'GET');
+ requests[1].resolve({maxRootReuseAgeInDays: 30});
+
const result = await promise;
assert.equal(result, BuildRequest.findById(16993));
assert.ok(BuildRequest.findById(16993).createdAt() < BuildRequest.findById(16989).createdAt());
Added: trunk/Websites/perf.webkit.org/unit-tests/manifest-test.js (0 => 270607)
--- trunk/Websites/perf.webkit.org/unit-tests/manifest-test.js (rev 0)
+++ trunk/Websites/perf.webkit.org/unit-tests/manifest-test.js 2020-12-09 23:13:28 UTC (rev 270607)
@@ -0,0 +1,65 @@
+'use strict';
+
+const assert = require('assert');
+require('../tools/js/v3-models.js');
+const BrowserPrivilegedAPI = require('../public/v3/privileged-api.js').PrivilegedAPI;
+const MockRemoteAPI = require('./resources/mock-remote-api.js').MockRemoteAPI;
+const assertThrows = require('../server-tests/resources/common-operations').assertThrows;
+
+const manifestSampleResponse = {
+ siteTitle: 'some title',
+ all: {},
+ bugTrackers: {},
+ builders: {},
+ dashboard: {},
+ dashboards: {},
+ fileUploadSizeLimit: 1,
+ maxRootReuseAgeInDays: null,
+ metrics: {},
+ platformGroups: {},
+ repositories: {},
+ testAgeToleranceInHours: null,
+ tests: {},
+ triggerables: {},
+ summaryPages: [],
+ status: 'OK'
+}
+
+describe('Manifest', () => {
+ const requests = MockRemoteAPI.inject('https://perf.webkit.org', BrowserPrivilegedAPI);
+
+ describe('fetchRawResponse', () => {
+ it('should fetch from "/data/manifest.json" if the file is available', async () => {
+ const fetchingTask = Manifest.fetchRawResponse();
+ assert.equal(requests.length, 1);
+ assert(requests[0].url, '/data/manifest.json')
+ requests[0].resolve(manifestSampleResponse);
+
+ const rawResponse = await fetchingTask;
+ assert.deepEqual(rawResponse, manifestSampleResponse);
+ });
+
+ it('should fetch from api only when fetching "/data/manifest.json" returns 404', async () => {
+ const fetchingTask = Manifest.fetchRawResponse();
+ assert.equal(requests.length, 1);
+ assert(requests[0].url, '/data/manifest.json')
+ requests[0].reject(404);
+
+ await MockRemoteAPI.waitForRequest();
+ assert.equal(requests.length, 2);
+ assert(requests[1].url, '/data/manifest.json')
+ requests[1].resolve(manifestSampleResponse);
+
+ const rawResponse = await fetchingTask;
+ assert.deepEqual(rawResponse, manifestSampleResponse);
+ });
+
+ it('should not fetch from api if fetching "/data/manifest.json" returns non-404', async () => {
+ const fetchingTask = Manifest.fetchRawResponse();
+ assert.equal(requests.length, 1);
+ assert(requests[0].url, '/data/manifest.json')
+ requests[0].reject(301);
+ assertThrows('Failed to fetch manifest.json with 301', () => fetchingTask);
+ });
+ });
+});
\ No newline at end of file
