Skip to content
Permalink
Browse files
[perf.webkit.org] Do not schedule jobs to Buildbot if the last job fa…
…iled

https://bugs.webkit.org/show_bug.cgi?id=221943

Patch by Dean Johnson <dean_johnson@apple.com> on 2021-02-16
Reviewed by Dewei Zhu.

* server-tests/resources/mock-data.js:
(MockData.sampleBuildData): Allow options.results to be specified.
* server-tests/tools-buildbot-triggerable-tests.js:
(async const): Add tests for new functionality. Existing tests still pass.
* tools/js/buildbot-syncer.js:
(BuildbotBuildEntry.prototype.initialize): Track the 'results' key from a build.
(BuildbotBuildEntry.prototype.result): Make the build result available.
(BuildbotSyncer): Track the last completed build.
(BuildbotSyncer.prototype.lastCompletedBuildSuccessful): Check if the last completed
build was successful.
(BuildbotSyncer.prototype.pullBuildbot):
* tools/js/buildbot-triggerable.js: Do not schedule a job to a builder who failed
its last completedd build.
(BuildbotTriggerable.prototype._scheduleRequestIfSlaveIsAvailable):

Canonical link: https://commits.webkit.org/234174@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272971 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dean authored and webkit-commit-queue committed Feb 17, 2021
1 parent 9cdd924 commit 01b0ccad19055643b9d0b31eb115cd6b529e7b21
Showing 5 changed files with 157 additions and 3 deletions.
@@ -1,3 +1,25 @@
2021-02-16 Dean Johnson <dean_johnson@apple.com>

[perf.webkit.org] Do not schedule jobs to Buildbot if the last job failed
https://bugs.webkit.org/show_bug.cgi?id=221943

Reviewed by Dewei Zhu.

* server-tests/resources/mock-data.js:
(MockData.sampleBuildData): Allow options.results to be specified.
* server-tests/tools-buildbot-triggerable-tests.js:
(async const): Add tests for new functionality. Existing tests still pass.
* tools/js/buildbot-syncer.js:
(BuildbotBuildEntry.prototype.initialize): Track the 'results' key from a build.
(BuildbotBuildEntry.prototype.result): Make the build result available.
(BuildbotSyncer): Track the last completed build.
(BuildbotSyncer.prototype.lastCompletedBuildSuccessful): Check if the last completed
build was successful.
(BuildbotSyncer.prototype.pullBuildbot):
* tools/js/buildbot-triggerable.js: Do not schedule a job to a builder who failed
its last completedd build.
(BuildbotTriggerable.prototype._scheduleRequestIfSlaveIsAvailable):

2021-02-11 Dewei Zhu <dewei_zhu@apple.com>

'sync-commits.py' should be able to limit reporting scope to a specific branch on a Git repository.
@@ -479,7 +479,7 @@ MockData = {
"complete_at": null,
"buildid": options.buildid || 418744,
"masterid": 1,
"results": null,
"results": options.results,
"started_at": new Date('2017-12-19T23:11:49Z') / 1000,
"state_string": options.statusDescription || null,
"workerid": 41,
@@ -214,6 +214,126 @@ describe('BuildbotTriggerable', function () {
});
});

it('should not schedule the first build request on a builder whose last build failed', async () => {
const db = TestServer.database();
await MockData.addMockData(db);
await Manifest.fetch();

const config = MockData.mockTestSyncConfigWithSingleBuilder();
const logger = new MockLogger;
const slaveInfo = {name: 'sync-slave', password: 'password'};
const triggerable = new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, slaveInfo, logger);
triggerable.initSyncers().then(() => triggerable.syncOnce());
assertRequestAndResolve(MockRemoteAPI.requests[0], 'GET', MockData.buildbotBuildersURL(), MockData.mockBuildbotBuilders());
MockRemoteAPI.reset();

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 1);
assert.equal(MockRemoteAPI.requests[0].method, 'GET');
assert.equal(MockRemoteAPI.requests[0].url, MockData.pendingBuildsUrl('some-builder-1'));
MockRemoteAPI.requests[0].resolve({});

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 2);
assert.equal(MockRemoteAPI.requests[1].method, 'GET');
assert.equal(MockRemoteAPI.requests[1].url, MockData.recentBuildsUrl('some-builder-1', 2));
MockRemoteAPI.requests[1].resolve({'builds' : [MockData.finishedBuildData({results: 1, buildRequestId: 699})]});

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 3);
assert.equal(MockRemoteAPI.requests[2].method, 'GET');
assert.equal(MockRemoteAPI.requests[2].url, MockData.pendingBuildsUrl('some-builder-1'));
MockRemoteAPI.requests[2].resolve({});

await MockRemoteAPI.waitForRequest();

assert.equal(BuildRequest.all().length, 4);
assert.equal(BuildRequest.findById(700).status(), 'pending');
assert.equal(BuildRequest.findById(700).statusUrl(), null);
});

it('should schedule the first build request to the first available non-failing queue', async () => {
const db = TestServer.database();
await MockData.addMockData(db);
await Manifest.fetch();

const config = MockData.mockTestSyncConfigWithTwoBuilders();
const logger = new MockLogger;
const slaveInfo = {name: 'sync-slave', password: 'password'};
const triggerable = new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, slaveInfo, logger);
const syncOnce = triggerable.initSyncers().then(() => triggerable.syncOnce());
assertRequestAndResolve(MockRemoteAPI.requests[0], 'GET', MockData.buildbotBuildersURL(), MockData.mockBuildbotBuilders());
MockRemoteAPI.reset();

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 2);
assert.equal(MockRemoteAPI.requests[0].method, 'GET');
assert.equal(MockRemoteAPI.requests[0].url, MockData.pendingBuildsUrl('some-builder-1'));
MockRemoteAPI.requests[0].resolve({});
assert.equal(MockRemoteAPI.requests[1].method, 'GET');
assert.equal(MockRemoteAPI.requests[1].url, MockData.pendingBuildsUrl('some builder 2'));
MockRemoteAPI.requests[1].resolve({});

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 4);
assert.equal(MockRemoteAPI.requests[2].method, 'GET');
assert.equal(MockRemoteAPI.requests[2].url, MockData.recentBuildsUrl('some-builder-1', 2));
MockRemoteAPI.requests[2].resolve({'builds' : [MockData.finishedBuildData({results: 1, buildRequestId: 699})]});
assert.equal(MockRemoteAPI.requests[3].method, 'GET');
assert.equal(MockRemoteAPI.requests[3].url, MockData.recentBuildsUrl('some builder 2', 2));
MockRemoteAPI.requests[3].resolve({});

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 5);
assert.equal(MockRemoteAPI.requests[4].method, 'POST');
assert.equal(MockRemoteAPI.requests[4].url, '/api/v2/forceschedulers/force-some-builder-2');
assert.deepEqual(MockRemoteAPI.requests[4].data, {'id': '700', 'jsonrpc': '2.0', 'method': 'force', 'params':
{'wk': '191622', 'os': '10.11 15A284', 'build-request-id': '700', 'forcescheduler': 'force-some-builder-2'}});
MockRemoteAPI.requests[4].resolve('OK');

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 7);
assert.equal(MockRemoteAPI.requests[5].method, 'GET');
assert.equal(MockRemoteAPI.requests[5].url, MockData.pendingBuildsUrl('some-builder-1'));
MockRemoteAPI.requests[5].resolve({});
assert.equal(MockRemoteAPI.requests[6].method, 'GET');
assert.equal(MockRemoteAPI.requests[6].url, MockData.pendingBuildsUrl('some builder 2'));
MockRemoteAPI.requests[6].resolve(MockData.pendingBuild({builderId: MockData.builderIDForName('some builder 2'), buildRequestId: 700, buildbotBuildRequestId: 17}));

await MockRemoteAPI.waitForRequest();

assert.equal(MockRemoteAPI.requests.length, 9);
assert.equal(MockRemoteAPI.requests[7].method, 'GET');
assert.equal(MockRemoteAPI.requests[7].url, MockData.recentBuildsUrl('some-builder-1', 2));
MockRemoteAPI.requests[7].resolve({});
assert.equal(MockRemoteAPI.requests[8].method, 'GET');
assert.equal(MockRemoteAPI.requests[8].url, MockData.recentBuildsUrl('some builder 2', 2));
MockRemoteAPI.requests[8].resolve({});

await syncOnce;

assert.equal(BuildRequest.all().length, 4);
assert.equal(BuildRequest.findById(700).status(), 'pending');
assert.equal(BuildRequest.findById(700).statusUrl(), null);
assert.equal(BuildRequest.findById(701).status(), 'pending');
assert.equal(BuildRequest.findById(701).statusUrl(), null);

await BuildRequest.fetchForTriggerable(MockData.mockTestSyncConfigWithTwoBuilders().triggerableName);

assert.equal(BuildRequest.all().length, 4);
assert.equal(BuildRequest.findById(700).status(), 'scheduled');
assert.equal(BuildRequest.findById(700).statusUrl(), 'http://build.webkit.org/#/buildrequests/17');
assert.equal(BuildRequest.findById(701).status(), 'pending');
assert.equal(BuildRequest.findById(701).statusUrl(), null);
});

it('should not schedule a build request on a different builder than the one the first build request is pending', () => {
const db = TestServer.database();
let syncPromise;
@@ -20,6 +20,7 @@ class BuildbotBuildEntry {
this._isPending = 'claimed' in rawData && !rawData['claimed'];
this._isInProgress = !this._isPending && !this._hasFinished;
this._buildTag = rawData['number'];
this._result = rawData['results'];
this._workerName = rawData['properties'] && rawData['properties']['workername'] ? rawData['properties']['workername'][0] : null;
this._buildRequestId = rawData['properties'] && rawData['properties'][syncer._buildRequestPropertyName]
? rawData['properties'][syncer._buildRequestPropertyName][0] : null;
@@ -34,6 +35,7 @@ class BuildbotBuildEntry {
isPending() { return this._isPending; }
isInProgress() { return this._isInProgress; }
hasFinished() { return this._hasFinished; }
result() { return this._result; }
statusDescription() { return this._statusDescription; }
url() { return this.isPending() ? this._syncer.urlForPendingBuild(this._buildbotBuildRequestId) : this._syncer.urlForBuildSerial(this._buildTag); }

@@ -72,11 +74,14 @@ class BuildbotSyncer {
this._builderID = object.builderID;
this._slaveList = object.slaveList;
this._entryList = null;
this._lastCompletedBuild = null;
this._slavesWithNewRequests = new Set;
}

builderName() { return this._builderName; }
builderID() { return this._builderID; }
// Buildbot result codes: https://docs.buildbot.net/latest/developer/results.html
lastCompletedBuildSuccessful() { return !this._lastCompletedBuild || !this._lastCompletedBuild.result() }

addTestConfiguration(test, platform, propertiesTemplate)
{
@@ -183,9 +188,12 @@ class BuildbotSyncer {
for (let entry of pendingEntries)
entryByRequest[entry.buildRequestId()] = entry;

for (let entry of entries)
for (let entry of entries) {
entryByRequest[entry.buildRequestId()] = entry;

if ((!this._lastCompletedBuild || this._lastCompletedBuild.buildTag() < entry.buildTag()) && entry.hasFinished())
this._lastCompletedBuild = entry;
}

let entryList = [];
for (let id in entryByRequest)
entryList.push(entryByRequest[id]);
@@ -251,6 +251,10 @@ class BuildbotTriggerable {

// Pick a new syncer for the first test.
for (const syncer of this._syncers) {
// Only schedule A/B tests to queues whose last job was successful.
if (syncer.isTester() && !nextRequest.order() && !syncer.lastCompletedBuildSuccessful())
continue;

const promise = this._scheduleRequestWithLog(syncer, nextRequest, requestsInGroup, null);
if (promise)
return promise;

0 comments on commit 01b0cca

Please sign in to comment.