Prune unused uploaded files when the file quota is reached
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 21:13:42 +0000 (21:13 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 21:13:42 +0000 (21:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174086

Reviewed by Andreas Kling.

Made /privileged-api/uploaded-file and /api/upload-root automatically delete old uploaded files when
uploading a new file results in the file quota to be exceeded. Also added the notion of the total quota
to avoid running out of a disk when there are hundreds of users each uploading near their quota.

* config.json: Added a sample total disk quota of 100GB.
* public/include/uploaded-file-helpers.php:
(query_file_usage_for_user): Renamed from query_total_file_size.
(query_total_file_usage): Added.
(upload_file_in_transaction):
(delete_file): Added.
(prune_old_files): Added.
* server-tests/privileged-api-upload-file-tests.js: Added tests for deleting old uploaded files as well as
tests for the total quota.
* server-tests/resources/test-server.js:
(TestServer.prototype.testConfig): Added uploadTotalQuotaInMB to the test configuration.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/config.json
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 298bdef..4a4f026 100644 (file)
@@ -1,3 +1,26 @@
+2017-07-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Prune unused uploaded files when the file quota is reached
+        https://bugs.webkit.org/show_bug.cgi?id=174086
+
+        Reviewed by Andreas Kling.
+
+        Made /privileged-api/uploaded-file and /api/upload-root automatically delete old uploaded files when
+        uploading a new file results in the file quota to be exceeded. Also added the notion of the total quota
+        to avoid running out of a disk when there are hundreds of users each uploading near their quota.
+
+        * config.json: Added a sample total disk quota of 100GB.
+        * public/include/uploaded-file-helpers.php:
+        (query_file_usage_for_user): Renamed from query_total_file_size.
+        (query_total_file_usage): Added.
+        (upload_file_in_transaction):
+        (delete_file): Added.
+        (prune_old_files): Added.
+        * server-tests/privileged-api-upload-file-tests.js: Added tests for deleting old uploaded files as well as
+        tests for the total quota.
+        * server-tests/resources/test-server.js:
+        (TestServer.prototype.testConfig): Added uploadTotalQuotaInMB to the test configuration.
+
 2017-06-29  Ryosuke Niwa  <rniwa@webkit.org>
 
         UploadedFile should include the file extension in its url
index e6d99a2..6c21011 100644 (file)
@@ -6,6 +6,7 @@
     "uploadDirectory": "uploaded",
     "uploadFileLimitInMB": 800,
     "uploadUserQuotaInMB": 8192,
+    "uploadTotalQuotaInMB": 102400,
     "database": {
         "host": "localhost",
         "port": "5432",
index 34341c2..9949afa 100644 (file)
@@ -44,14 +44,22 @@ function validate_uploaded_file($field_name)
     return $input_file;
 }
 
-function query_total_file_size($db, $user)
+function query_file_usage_for_user($db, $user)
 {
     if ($user)
         $count_result = $db->query_and_fetch_all('SELECT sum(file_size) as "sum" FROM uploaded_files WHERE file_deleted_at IS NULL AND file_author = $1', array($user));
     else
         $count_result = $db->query_and_fetch_all('SELECT sum(file_size) as "sum" FROM uploaded_files WHERE file_deleted_at IS NULL AND file_author IS NULL');
     if (!$count_result)
-        return FALSE;
+        exit_with_error('FailedToQueryDiskUsagePerUser');
+    return intval($count_result[0]["sum"]);
+}
+
+function query_total_file_usage($db)
+{
+    $count_result = $db->query_and_fetch_all('SELECT sum(file_size) as "sum" FROM uploaded_files WHERE file_deleted_at IS NULL');
+    if (!$count_result)
+        exit_with_error('FailedToQueryTotalDiskUsage');
     return intval($count_result[0]["sum"]);
 }
 
@@ -80,10 +88,14 @@ function create_uploaded_file_from_form_data($input_file)
 
 function upload_file_in_transaction($db, $input_file, $remote_user, $additional_work = NULL)
 {
-    // FIXME: Cleanup old files.
-
-    if (config('uploadUserQuotaInMB') * MEGABYTES - query_total_file_size($db, $remote_user) < $input_file['size'])
-        exit_with_error('FileSizeQuotaExceeded');
+    $new_file_size = $input_file['size'];
+    if (config('uploadUserQuotaInMB') * MEGABYTES - query_file_usage_for_user($db, $remote_user) < $new_file_size
+        || config('uploadTotalQuotaInMB') * MEGABYTES - query_total_file_usage($db) < $new_file_size) {
+        // Instead of <quota> - <used> - <new file size>, just ask for <new file size>
+        // since finding files to delete is an expensive operation.
+        if (!prune_old_files($db, $new_file_size, $remote_user))
+            exit_with_error('FileSizeQuotaExceeded');
+    }
 
     $uploaded_file = create_uploaded_file_from_form_data($input_file);
 
@@ -94,7 +106,8 @@ function upload_file_in_transaction($db, $input_file, $remote_user, $additional_
         exit_with_error('FailedToInsertFileData');
 
     // A concurrent session may have inserted another file.
-    if (config('uploadUserQuotaInMB') * MEGABYTES < query_total_file_size($db, $remote_user)) {
+    if (config('uploadUserQuotaInMB') * MEGABYTES < query_file_usage_for_user($db, $remote_user)
+        || config('uploadTotalQuotaInMB') * MEGABYTES < query_total_file_usage($db)) {
         $db->rollback_transaction();
         exit_with_error('FileSizeQuotaExceeded');
     }
@@ -118,4 +131,65 @@ function upload_file_in_transaction($db, $input_file, $remote_user, $additional_
     return format_uploaded_file($file_row);
 }
 
+function delete_file($db, $file_row)
+{
+    $db->begin_transaction();
+
+    if (!$db->query_and_get_affected_rows("UPDATE uploaded_files SET file_deleted_at = CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
+        WHERE file_id = $1", array($file_row['file_id']))) {
+        $db->rollback_transaction();
+        return FALSE;
+    }
+
+    $file_path = uploaded_file_path_for_row($file_row);
+    // The file may have been deleted by a concurrent session by the time we get here.
+    if (file_exists($file_path) && !unlink($file_path)) {
+        $db->rollback_transaction();
+        return FALSE;
+    }
+
+    $db->commit_transaction();
+    return TRUE;
+}
+
+function prune_old_files($db, $size_needed, $remote_user)
+{
+    $user_filter = $remote_user ? 'AND file_author = $1' : 'AND file_author IS NULL';
+    $params = $remote_user ? array($remote_user) : array();
+
+    // 1. Delete old build products created for a patch not associated with any pending or in-progress builds.
+    $build_product_query = $db->query("SELECT file_id, file_extension, file_size FROM uploaded_files, commit_set_items
+        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);
+    if (!$build_product_query)
+        return FALSE;
+    while ($row = $db->fetch_next_row($build_product_query)) {
+        if (!$row || !delete_file($db, $row))
+            return FALSE;
+        $size_needed -= $row['file_size'];
+        if ($size_needed <= 0)
+            return TRUE;
+    }
+
+    // 2. Delete any uploaded file not associated with any pending or in-progress builds.
+    $unused_file_query = $db->query("SELECT file_id, file_extension, file_size FROM uploaded_files
+        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')
+            $user_filter
+        ORDER BY file_created_at LIMIT 10", $params);
+    if (!$unused_file_query)
+        return FALSE;
+    while ($row = $db->fetch_next_row($unused_file_query)) {
+        if (!$row || !delete_file($db, $row))
+            return FALSE;
+        $size_needed -= $row['file_size'];
+        if ($size_needed <= 0)
+            return TRUE;
+    }
+    return FALSE;
+}
+
 ?>
index 6927785..7e7f2c8 100644 (file)
@@ -5,13 +5,13 @@ require('../tools/js/v3-models.js');
 const assert = require('assert');
 global.FormData = require('form-data');
 
+const MockData = require('./resources/mock-data.js');
 const TestServer = require('./resources/test-server.js');
 const TemporaryFile = require('./resources/temporary-file.js').TemporaryFile;
+const prepareServerTest = require('./resources/common-operations.js').prepareServerTest;
 
 describe('/privileged-api/upload-file', function () {
-    this.timeout(5000);
-    TestServer.inject();
-
+    prepareServerTest(this);
     TemporaryFile.inject();
 
     it('should return "NotFileSpecified" when newFile not is specified', () => {
@@ -40,7 +40,7 @@ describe('/privileged-api/upload-file', function () {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then((response) => {
             uploadedFile = response['uploadedFile'];
-            return db.connect().then(() => db.selectAll('uploaded_files', 'id'));
+            return db.selectAll('uploaded_files', 'id');
         }).then((rows) => {
             assert.equal(rows.length, 1);
             assert.equal(rows[0].id, uploadedFile.id);
@@ -68,7 +68,7 @@ describe('/privileged-api/upload-file', function () {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then((response) => {
             uploadedFile2 = response['uploadedFile'];
-            return db.connect().then(() => db.selectAll('uploaded_files', 'id'));
+            return db.selectAll('uploaded_files', 'id');
         }).then((rows) => {
             assert.deepEqual(uploadedFile1, uploadedFile2);
             assert.equal(rows.length, 1);
@@ -97,7 +97,7 @@ describe('/privileged-api/upload-file', function () {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then((response) => {
             uploadedFile2 = response['uploadedFile'];
-            return db.connect().then(() => db.selectAll('uploaded_files', 'id'));
+            return db.selectAll('uploaded_files', 'id');
         }).then((rows) => {
             assert.deepEqual(uploadedFile1, uploadedFile2);
             assert.equal(rows.length, 1);
@@ -121,14 +121,14 @@ describe('/privileged-api/upload-file', function () {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then((response) => {
             uploadedFile1 = response['uploadedFile'];
-            return db.connect().then(() => db.query(`UPDATE uploaded_files SET file_deleted_at = now() at time zone 'utc'`));
+            return db.query(`UPDATE uploaded_files SET file_deleted_at = now() at time zone 'utc'`);
         }).then(() => {
             return TemporaryFile.makeTemporaryFileOfSizeInMB('other.dat', limitInMB);
         }).then((stream) => {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then((response) => {
             uploadedFile2 = response['uploadedFile'];
-            return db.connect().then(() => db.selectAll('uploaded_files', 'id'));
+            return db.selectAll('uploaded_files', 'id');
         }).then((rows) => {
             assert.notEqual(uploadedFile1.id, uploadedFile2.id);
             assert.equal(rows.length, 2);
@@ -159,7 +159,7 @@ describe('/privileged-api/upload-file', function () {
         return TemporaryFile.makeTemporaryFileOfSizeInMB('some.other.tar.gz', limitInMB).then((stream) => {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then(() => {
-            return db.connect().then(() => db.selectAll('uploaded_files', 'id'))
+            return db.selectAll('uploaded_files', 'id');
         }).then((rows) => {
             assert.equal(rows.length, 1);
             assert.equal(rows[0].size, limitInMB * 1024 * 1024);
@@ -170,7 +170,7 @@ describe('/privileged-api/upload-file', function () {
         });
     });
 
-    it('should return "FileSizeQuotaExceeded" when the total file size exceeds the quota allowed per user', () => {
+    it('should delete an old file when uploading the file would result in the quota being exceeded', () => {
         const db = TestServer.database();
         const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
         return TemporaryFile.makeTemporaryFileOfSizeInMB('some.dat', limitInMB, 'a').then((stream) => {
@@ -180,6 +180,143 @@ describe('/privileged-api/upload-file', function () {
         }).then((stream) => {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
         }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('another.dat', limitInMB, 'c');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return db.selectAll('uploaded_files', 'id');
+        }).then((rows) => {
+            assert.equal(rows.length, 3);
+            assert.equal(rows[0].filename, 'some.dat');
+            assert.notEqual(rows[0].deleted_at, null);
+            assert.equal(rows[1].filename, 'other.dat');
+            assert.equal(rows[1].deleted_at, null);
+            assert.equal(rows[2].filename, 'another.dat');
+            assert.equal(rows[2].deleted_at, null);
+        })
+    });
+
+    it('should return "FileSizeQuotaExceeded" when there is no file to delete', () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+        let fileA;
+        return MockData.addMockData(db).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('some.patch', limitInMB, 'a');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            fileA = result.uploadedFile;
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('other.patch', limitInMB, 'b');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            const fileB = result.uploadedFile;
+            return Promise.all([
+                db.query('UPDATE commit_set_items SET commitset_patch_file = $1 WHERE commitset_set = 402 AND commitset_commit = 87832', [fileA.id]),
+                db.query('UPDATE commit_set_items SET commitset_patch_file = $1 WHERE commitset_set = 402 AND commitset_commit = 96336', [fileB.id])
+            ]);
+        }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('other.dat', limitInMB, 'c');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true}).then(() => {
+                assert(false, 'should never be reached');
+            }, (error) => {
+                assert.equal(error, 'FileSizeQuotaExceeded');
+            });
+        });
+    });
+
+    it('should delete old patches that belong to finished build requests', () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+        let fileA;
+        return MockData.addMockData(db, ['completed', 'completed', 'failed', 'canceled']).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('some.patch', limitInMB, 'a');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            fileA = result.uploadedFile;
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('other.patch', limitInMB, 'b');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            const fileB = result.uploadedFile;
+            return Promise.all([
+                db.query('UPDATE commit_set_items SET commitset_patch_file = $1 WHERE commitset_set = 402 AND commitset_commit = 87832', [fileA.id]),
+                db.query('UPDATE commit_set_items SET commitset_patch_file = $1 WHERE commitset_set = 402 AND commitset_commit = 96336', [fileB.id])
+            ]);
+        }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('another.dat', limitInMB, 'c');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return db.selectAll('uploaded_files', 'id');
+        }).then((rows) => {
+            assert.equal(rows.length, 3);
+            assert.equal(rows[0].filename, 'some.patch');
+            assert.notEqual(rows[0].deleted_at, null);
+            assert.equal(rows[1].filename, 'other.patch');
+            assert.equal(rows[1].deleted_at, null);
+            assert.equal(rows[2].filename, 'another.dat');
+            assert.equal(rows[2].deleted_at, null);
+        });
+    });
+
+    it('should delete old build products that belong to finished build requests before deleting patches', () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+        let fileA;
+        return MockData.addMockData(db, ['completed', 'completed', 'failed', 'canceled']).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('some.patch', limitInMB, 'a');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            fileA = result.uploadedFile;
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('root.tar.gz', limitInMB, 'b');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((result) => {
+            const fileB = result.uploadedFile;
+            return db.query(`UPDATE commit_set_items SET (commitset_patch_file, commitset_root_file) = ($1, $2)
+                WHERE commitset_set = 402 AND commitset_commit = 87832`, [fileA.id, fileB.id]);
+        }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('another.dat', limitInMB, 'c');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return db.selectAll('uploaded_files', 'id');
+        }).then((rows) => {
+            assert.equal(rows.length, 3);
+            assert.equal(rows[0].filename, 'some.patch');
+            assert.equal(rows[0].deleted_at, null);
+            assert.equal(rows[1].filename, 'root.tar.gz');
+            assert.notEqual(rows[1].deleted_at, null);
+            assert.equal(rows[2].filename, 'another.dat');
+            assert.equal(rows[2].deleted_at, null);
+        });
+    });
+
+    it('should return "FileSizeQuotaExceeded" when the total quota is exceeded due to files uploaded by other users', () => {
+        const db = TestServer.database();
+        const limitInMB = TestServer.testConfig().uploadFileLimitInMB;
+        let fileA;
+        return MockData.addMockData(db).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('some.dat', limitInMB, 'a');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('other.dat', limitInMB, 'b');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return db.query('UPDATE uploaded_files SET file_author = $1', ['someUser']);
+        }).then(() => {
+            return TemporaryFile.makeTemporaryFileOfSizeInMB('another.dat', limitInMB, 'c');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then(() => {
+            return db.query('UPDATE uploaded_files SET file_author = $1 WHERE file_author IS NULL', ['anotherUser']);
+        }).then(() => {
             return TemporaryFile.makeTemporaryFileOfSizeInMB('other.dat', limitInMB, 'c');
         }).then((stream) => {
             return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true}).then(() => {
index 6c3a175..988285d 100644 (file)
@@ -78,6 +78,7 @@ class TestServer {
             },
             'uploadFileLimitInMB': 2,
             'uploadUserQuotaInMB': 5,
+            'uploadTotalQuotaInMB': 6,
             'uploadDirectory': Config.value('dataDirectory') + '/uploaded',
             'universalSlavePassword': null,
             'maintenanceMode': false,