Limit the number of results to be submitted in one submission.

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

Reviewed by Ryosuke Niwa.

Submitting results for a large number of builds with owned commit information may exceed the size limit of php.
Added a way to split the results into groups of certain sizes, and submit them one by one.

* server-tests/tools-os-build-fetcher-tests.js: Updated the unit tests.
* tools/js/os-build-fetcher.js: Added '_maxNumberOfResultsPerSubmit' which can be specified by a configuration but also use 20 as default value.
(prototype.fetchAndReportNewBuilds): Instead of submitting all results once, split them into groups and submit them one by one.
(prototype._fetchAvailableBuilds): 'label' is already quoted, should remove unnecessary quotes.
(prototype._addOwnedCommitsForBuild): Added logging to log the size of owned commits.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@224228 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
index de57dff5dac766cb8035fdb7dc1aa026b1bd4e09..3473f943672b5cc3579122549c58830d28391262 100644 (file)
--- a/Websites/perf.webkit.org/ChangeLog
+++ b/Websites/perf.webkit.org/ChangeLog
@@ -1,3 +1,19 @@
+2017-10-30  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Limit the number of results to be submitted in one submission.
+        https://bugs.webkit.org/show_bug.cgi?id=179045
+
+        Reviewed by Ryosuke Niwa.
+
+        Submitting results for a large number of builds with owned commit information may exceed the size limit of php.
+        Added a way to split the results into groups of certain sizes, and submit them one by one.
+
+        * server-tests/tools-os-build-fetcher-tests.js: Updated the unit tests.
+        * tools/js/os-build-fetcher.js: Added '_maxNumberOfResultsPerSubmit' which can be specified by a configuration but also use 20 as default value.
+        (prototype.fetchAndReportNewBuilds): Instead of submitting all results once, split them into groups and submit them one by one.
+        (prototype._fetchAvailableBuilds): 'label' is already quoted, should remove unnecessary quotes.
+        (prototype._addOwnedCommitsForBuild): Added logging to log the size of owned commits.
+
 2017-10-31  Dewei Zhu  <dewei_zhu@apple.com>
 
         OwnedCommitViewer should include the preceding commit.

diff --git a/Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js b/Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js
index bcd3982eccf12deebe7bd0e793dd8b5d2468a036..a4c62cb09334ce225ebd01dd6dfb8139f8aa49a6 100644 (file)
--- a/Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js
+++ b/Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js
@@ -97,7 +97,7 @@ describe('OSBuildFetcher', function() {
 
     describe('OSBuilderFetcher._computeOrder', () => {
         it('should calculate the right order for a given valid revision', () => {
-            const fetcher = new OSBuildFetcher();
+            const fetcher = new OSBuildFetcher({});
             assert.equal(fetcher._computeOrder('Sierra16D32'), 1603003200);
             assert.equal(fetcher._computeOrder('16D321'), 1603032100);
             assert.equal(fetcher._computeOrder('16d321'), 1603032100);
@@ -108,7 +108,7 @@ describe('OSBuildFetcher', function() {
         });
 
         it('should throw assertion error when given a invalid revision', () => {
-            const fetcher = new OSBuildFetcher();
+            const fetcher = new OSBuildFetcher({});
             assert.throws(() => fetcher._computeOrder('invalid'), (error) => error.name == 'AssertionError');
             assert.throws(() => fetcher._computeOrder(''), (error) => error.name == 'AssertionError');
             assert.throws(() => fetcher._computeOrder('16'), (error) => error.name == 'AssertionError');
@@ -124,7 +124,7 @@ describe('OSBuildFetcher', function() {
     describe('OSBuilderFetcher._commitsForAvailableBuilds', () => {
         it('should only return commits whose orders are higher than specified order', () => {
             const logger = new MockLogger;
-            const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+            const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const fetchCommitsPromise = fetchter._commitsForAvailableBuilds('OSX', ['list', 'build1'], '^\\.*$', 1604000000);
 
@@ -144,7 +144,7 @@ describe('OSBuildFetcher', function() {
     describe('OSBuildFetcher._addOwnedCommitsForBuild', () => {
         it('should add owned-commit info for commits', () => {
             const logger = new MockLogger;
-            const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+            const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit, anotherOSXCommit], ['ownedCommit', 'for', 'revision']);
 
@@ -172,7 +172,7 @@ describe('OSBuildFetcher', function() {
 
         it('should fail if the command to get owned-commit info fails', () => {
             const logger = new MockLogger;
-            const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+            const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit], ['ownedCommit', 'for', 'revision'])
 
@@ -193,7 +193,7 @@ describe('OSBuildFetcher', function() {
 
         it('should fail if entries in owned-commits does not contain revision', () => {
             const logger = new MockLogger;
-            const fetchter = new OSBuildFetcher(null, null, null, MockSubprocess, logger);
+            const fetchter = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
             const addownedCommitPromise = fetchter._addOwnedCommitsForBuild([osxCommit], ['ownedCommit', 'for', 'revision'])
 
@@ -326,7 +326,7 @@ describe('OSBuildFetcher', function() {
 
                 assert.equal(webkitRepository.length, 1);
                 assert.equal(webkitRepository[0]['owner'], 10);
-                assert.equal(jscRepository.length, 1)
+                assert.equal(jscRepository.length, 1);
                 assert.equal(jscRepository[0]['owner'], 10);
 
                 assert.equal(osxCommit16D69.length, 1);
@@ -412,7 +412,7 @@ describe('OSBuildFetcher', function() {
                 const osxCommit16E34 = results[4];
 
                 assert.equal(webkitRepository.length, 0);
-                assert.equal(jscRepository.length, 0)
+                assert.equal(jscRepository.length, 0);
 
                 assert.equal(osxCommit16D69.length, 1);
                 assert.equal(osxCommit16D69[0]['repository'], 10);

diff --git a/Websites/perf.webkit.org/tools/js/os-build-fetcher.js b/Websites/perf.webkit.org/tools/js/os-build-fetcher.js
index 551e529805919ff287adc7e362a1810c03656c71..3b795a3f2435d7d62bae7d14ee78cba05c6cf63e 100644 (file)
--- a/Websites/perf.webkit.org/tools/js/os-build-fetcher.js
+++ b/Websites/perf.webkit.org/tools/js/os-build-fetcher.js
@@ -15,11 +15,11 @@ class OSBuildFetcher {
     constructor(osConfig, remoteAPI, slaveAuth, subprocess, logger)
     {
         this._osConfig = osConfig;
-        this._reportedRevisions = new Set();
         this._logger = logger;
         this._slaveAuth = slaveAuth;
         this._remoteAPI = remoteAPI;
         this._subprocess = subprocess;
+        this._maxSubmitCount = osConfig['maxSubmitCount'] || 20;
     }
 
     static fetchAndReportAllInOrder(fetcherList)
@@ -31,7 +31,18 @@ class OSBuildFetcher {
     {
         return this._fetchAvailableBuilds().then((results) => {
             this._logger.log(`Submitting ${results.length} builds for ${this._osConfig['name']}`);
-            return this._submitCommits(results);
+            if (results.length == 0)
+                return this._submitCommits(results);
+
+            const splittedResults = [];
+            for (let startIndex = 0; startIndex < results.length; startIndex += this._maxSubmitCount)
+                splittedResults.push(results.slice(startIndex, startIndex + this._maxSubmitCount));
+
+            return mapInSerialPromiseChain(splittedResults, this._submitCommits.bind(this)).then((responses) => {
+                assert(responses.every((response) => response['status'] == 'OK'));
+                assert(responses.length > 0);
+                return responses[0];
+            });
         });
     }
 
@@ -53,11 +64,11 @@ class OSBuildFetcher {
                 const minOrder = result['commits'].length == 1 ? parseInt(result['commits'][0]['order']) : 0;
                 return this._commitsForAvailableBuilds(repositoryName, command['command'], command['linesToIgnore'], minOrder);
             }).then((commits) => {
-                const label = 'name' in command ? `"${command['name']}"` : `"command['minRevision']" to "command['maxRevision']"`;
+                const label = 'name' in command ? `"${command['name']}"` : `"${command['minRevision']}" to "${command['maxRevision']}"`;
                 this._logger.log(`Found ${commits.length} builds for ${label}`);
 
                 if ('ownedCommitCommand' in command) {
-                    this._logger.log(`Resolving ownedCommits for "${label}"`);
+                    this._logger.log(`Resolving ownedCommits for ${label}`);
                     return this._addOwnedCommitsForBuild(commits, command['ownedCommitCommand']);
                 }
 
@@ -96,6 +107,7 @@ class OSBuildFetcher {
         return mapInSerialPromiseChain(commits, (commit) => {
             return this._subprocess.execute(command.concat(commit['revision'])).then((ownedCommitOutput) => {
                 const ownedCommits = JSON.parse(ownedCommitOutput);
+                this._logger.log(`Got ${Object.keys((ownedCommits)).length} owned commits for "${commit['revision']}"`);
                 for (let repositoryName in ownedCommits) {
                     const ownedCommit = ownedCommits[repositoryName];
                     assert.deepEqual(Object.keys(ownedCommit), ['revision']);