Pruning old file logic should not stop after removing 10 files if there are more...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 May 2019 01:56:17 +0000 (01:56 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 May 2019 01:56:17 +0000 (01:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197870

Reviewed by Ryosuke Niwa.

Pruning old file logic should keep removing removable files to make enough space for a new file upload.
It should only attempt to remve the files that have not been removed.

* public/include/uploaded-file-helpers.php: Modified 'prune_old_files' to allow to remove more than 10 files.
Fixed a bug that a removed file keep getting removed but never free up new space.
* server-tests/privileged-api-upload-file-tests.js:
(makeRandomAlnumStringForLength): Helper function to generate random content for a given length.
* server-tests/resources/test-server.js: Update total file size limit for this change.
(TestServer.prototype.testConfig):

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/uploaded-file-helpers.php
Websites/perf.webkit.org/server-tests/privileged-api-upload-file-tests.js
Websites/perf.webkit.org/server-tests/resources/test-server.js

index 5a7a581..6c174c9 100644 (file)
@@ -1,3 +1,20 @@
+2019-05-14  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Pruning old file logic should not stop after removing 10 files if there are more files to be removed.
+        https://bugs.webkit.org/show_bug.cgi?id=197870
+
+        Reviewed by Ryosuke Niwa.
+
+        Pruning old file logic should keep removing removable files to make enough space for a new file upload.
+        It should only attempt to remve the files that have not been removed.
+
+        * public/include/uploaded-file-helpers.php: Modified 'prune_old_files' to allow to remove more than 10 files.
+        Fixed a bug that a removed file keep getting removed but never free up new space.
+        * server-tests/privileged-api-upload-file-tests.js:
+        (makeRandomAlnumStringForLength): Helper function to generate random content for a given length.
+        * server-tests/resources/test-server.js: Update total file size limit for this change.
+        (TestServer.prototype.testConfig):
+
 2019-03-25  Dewei Zhu  <dewei_zhu@apple.com>
 
         Primary cluster of measurement set should always contain latest point.
index 2f36a92..3832c44 100644 (file)
@@ -162,7 +162,7 @@ function prune_old_files($db, $size_needed, $remote_user)
         WHERE file_id = commitset_root_file AND commitset_patch_file IS NOT NULL AND file_deleted_at IS NULL
             AND NOT EXISTS (SELECT request_id FROM build_requests WHERE request_commit_set = commitset_set AND request_status <= 'running')
             $user_filter
-        ORDER BY file_created_at LIMIT 10", $params);
+        ORDER BY file_created_at", $params);
     if (!$build_product_query)
         return FALSE;
     while ($row = $db->fetch_next_row($build_product_query)) {
@@ -178,8 +178,9 @@ function prune_old_files($db, $size_needed, $remote_user)
         WHERE NOT EXISTS (SELECT request_id FROM build_requests, commit_set_items
             WHERE (commitset_root_file = file_id OR commitset_patch_file = file_id)
                 AND request_commit_set = commitset_set AND request_status <= 'running')
+            AND file_deleted_at IS NULL
             $user_filter
-        ORDER BY file_created_at LIMIT 10", $params);
+        ORDER BY file_created_at", $params);
     if (!$unused_file_query)
         return FALSE;
     while ($row = $db->fetch_next_row($unused_file_query)) {
index 1b902fa..e0f8a28 100644 (file)
@@ -13,6 +13,15 @@ const prepareServerTest = require('./resources/common-operations.js').prepareSer
 describe('/privileged-api/upload-file', function () {
     prepareServerTest(this);
     TemporaryFile.inject();
+    function makeRandomAlnumStringForLength(length)
+    {
+        let string = '';
+        const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
+        const charactersLength = characters.length;
+        for (let i = 0; i < length; i++)
+            string += characters.charAt(Math.floor(Math.random() * charactersLength));
+        return string;
+    }
 
     it('should return "NotFileSpecified" when newFile not is specified', () => {
         return PrivilegedAPI.sendRequest('upload-file', {}, {useFormData: true}).then(() => {
@@ -196,6 +205,57 @@ describe('/privileged-api/upload-file', function () {
         })
     });
 
+    it('should prune all removable files to get space for a file upload', async () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+        const userQuotaInMB = TestServer.testConfig().uploadUserQuotaInMB;
+        const splitCount = 40;
+        const contentLength = userQuotaInMB * 1024 * 1024 / splitCount;
+        for (let i = 0; i < splitCount; i += 1) {
+            const content = makeRandomAlnumStringForLength(contentLength);
+            const stream = await TemporaryFile.makeTemporaryFile('file-' + i, content);
+            await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }
+        let rows = await db.selectAll('uploaded_files');
+        assert.equal(rows.length, splitCount);
+
+        const stream = await TemporaryFile.makeTemporaryFileOfSizeInMB('limit.dat', limitInMB, 'a');
+        await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        rows = await db.selectAll('uploaded_files');
+        assert.equal(rows.length, splitCount + 1);
+        const deletedFiles = rows.filter((row) => row['deleted_at']);
+        assert.equal(deletedFiles.length, 16);
+    });
+
+    it('should not prune files that have been deleted', async () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+
+        let stream = await TemporaryFile.makeTemporaryFileOfSizeInMB('limit.dat', limitInMB, 'a');
+        let response = await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        const fileA = response.uploadedFile;
+        stream = await TemporaryFile.makeTemporaryFileOfSizeInMB('limit.dat', limitInMB, 'b');
+        response = await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        const fileB = response.uploadedFile;
+
+        await MockData.addMockData(db, ['completed', 'completed', 'completed', 'completed']);
+        await db.query('UPDATE commit_set_items SET commitset_root_file=$1 WHERE commitset_set=401 AND commitset_commit=87832', [fileA.id]);
+        await db.query('UPDATE commit_set_items SET commitset_root_file=$1 WHERE commitset_set=401 AND commitset_commit=93116', [fileB.id]);
+
+        stream = await TemporaryFile.makeTemporaryFileOfSizeInMB('limit.dat', limitInMB, 'c');
+        response = await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        const fileC = response.uploadedFile;
+        await db.query('UPDATE commit_set_items SET commitset_root_file=$1 WHERE commitset_set=402 AND commitset_commit=87832', [fileC.id]);
+
+        const deletedFileA = await db.selectFirstRow('uploaded_files', {id: fileA.id});
+        assert.ok(deletedFileA.deleted_at);
+
+        stream = await TemporaryFile.makeTemporaryFileOfSizeInMB('limit.dat', limitInMB, 'd');
+        await PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        const deletedFileB = await db.selectFirstRow('uploaded_files', {id: fileB.id});
+        assert.ok(deletedFileB.deleted_at);
+    });
+
     it('should return "FileSizeQuotaExceeded" when there is no file to delete', () => {
         const db = TestServer.database();
         const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
index ad60c46..7273006 100644 (file)
@@ -80,7 +80,7 @@ class TestServer {
             },
             'uploadFileLimitInMB': 2,
             'uploadUserQuotaInMB': 5,
-            'uploadTotalQuotaInMB': 6,
+            'uploadTotalQuotaInMB': 7,
             'uploadDirectory': Config.value('dataDirectory') + '/uploaded',
             'universalSlavePassword': null,
             'maintenanceMode': false,