From 3925839c7ffdeb6cbd5e9733da9c8c7f209ba2fe Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Mon, 11 May 2020 10:29:16 +0100 Subject: [PATCH] add refreshExpiryTimeout function on clsi all data lives inside of / dir dynamically reduce size of EXPIRY_TIMEOUT if disk starts to get full --- services/clsi/app.js | 8 +-- .../clsi/app/js/ProjectPersistenceManager.js | 22 +++++++ services/clsi/package-lock.json | 14 +++++ services/clsi/package.json | 1 + .../unit/js/ProjectPersistenceManagerTests.js | 62 ++++++++++++++++++- 5 files changed, 102 insertions(+), 5 deletions(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index 80b00ef5f6..6fd29b0adb 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -359,10 +359,10 @@ if (!module.parent) { module.exports = app -setInterval( - () => ProjectPersistenceManager.clearExpiredProjects(), - (tenMinutes = 10 * 60 * 1000) -) +setInterval(() => { + ProjectPersistenceManager.refreshExpiryTimeout() + ProjectPersistenceManager.clearExpiredProjects() +}, (tenMinutes = 10 * 60 * 1000)) function __guard__(value, transform) { return typeof value !== 'undefined' && value !== null diff --git a/services/clsi/app/js/ProjectPersistenceManager.js b/services/clsi/app/js/ProjectPersistenceManager.js index 46eee74771..2c89f13f81 100644 --- a/services/clsi/app/js/ProjectPersistenceManager.js +++ b/services/clsi/app/js/ProjectPersistenceManager.js @@ -20,10 +20,32 @@ const async = require('async') const logger = require('logger-sharelatex') const oneDay = 24 * 60 * 60 * 1000 const Settings = require('settings-sharelatex') +const diskusage = require('diskusage') module.exports = ProjectPersistenceManager = { EXPIRY_TIMEOUT: Settings.project_cache_length_ms || oneDay * 2.5, + refreshExpiryTimeout(callback) { + if (callback == null) { + callback = function(error) {} + } + diskusage.check('/', function(err, stats) { + if (err) { + logger.err({ err: err }, 'error getting disk usage') + return callback(err) + } + const lowDisk = stats.available / stats.total < 0.1 + const lowerExpiry = ProjectPersistenceManager.EXPIRY_TIMEOUT * 0.9 + if (lowDisk && Settings.project_cache_length_ms / 2 < lowerExpiry) { + logger.warn( + { stats: stats }, + 'disk running low on space, modifying EXPIRY_TIMEOUT' + ) + ProjectPersistenceManager.EXPIRY_TIMEOUT = lowerExpiry + } + callback() + }) + }, markProjectAsJustAccessed(project_id, callback) { if (callback == null) { callback = function(error) {} diff --git a/services/clsi/package-lock.json b/services/clsi/package-lock.json index c64e7d4485..2d27ff12ba 100644 --- a/services/clsi/package-lock.json +++ b/services/clsi/package-lock.json @@ -1883,6 +1883,15 @@ "resolved": "https://registry.npmjs.org/diff/-/diff-1.0.7.tgz", "integrity": "sha1-JLuwAcSn1VIhaefKvbLCgU7ZHPQ=" }, + "diskusage": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/diskusage/-/diskusage-1.1.3.tgz", + "integrity": "sha512-EAyaxl8hy4Ph07kzlzGTfpbZMNAAAHXSZtNEMwdlnSd1noHzvA6HsgKt4fEMSvaEXQYLSphe5rPMxN4WOj0hcQ==", + "requires": { + "es6-promise": "^4.2.5", + "nan": "^2.14.0" + } + }, "dlv": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/dlv/-/dlv-1.1.3.tgz", @@ -6925,6 +6934,11 @@ "resolved": "https://registry.npmjs.org/walkdir/-/walkdir-0.4.1.tgz", "integrity": "sha512-3eBwRyEln6E1MSzcxcVpQIhRG8Q1jLvEqRmCZqS3dsfXEDR/AhOF4d+jHg1qvDCpYaVRZjENPQyrVxAkQqxPgQ==" }, + "when": { + "version": "3.7.8", + "resolved": "https://registry.npmjs.org/when/-/when-3.7.8.tgz", + "integrity": "sha1-xxMLan6gRpPoQs3J56Hyqjmjn4I=" + }, "which": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", diff --git a/services/clsi/package.json b/services/clsi/package.json index 00440a8b1f..5c25fa1207 100644 --- a/services/clsi/package.json +++ b/services/clsi/package.json @@ -21,6 +21,7 @@ "dependencies": { "async": "3.2.0", "body-parser": "^1.19.0", + "diskusage": "^1.1.3", "dockerode": "^3.1.0", "express": "^4.17.1", "fs-extra": "^8.1.0", diff --git a/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js b/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js index 0d84fc2455..1a12cfffce 100644 --- a/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js +++ b/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js @@ -14,6 +14,7 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') require('chai').should() +const assert = require('chai').assert const modulePath = require('path').join( __dirname, '../../../app/js/ProjectPersistenceManager' @@ -26,7 +27,15 @@ describe('ProjectPersistenceManager', function() { requires: { './UrlCache': (this.UrlCache = {}), './CompileManager': (this.CompileManager = {}), - 'logger-sharelatex': (this.logger = { log: sinon.stub() }), + diskusage: (this.diskusage = { check: sinon.stub() }), + 'logger-sharelatex': (this.logger = { + log: sinon.stub(), + warn: sinon.stub(), + err: sinon.stub() + }), + 'settings-sharelatex': (this.settings = { + project_cache_length_ms: 1000 + }), './db': (this.db = {}) } }) @@ -35,6 +44,57 @@ describe('ProjectPersistenceManager', function() { return (this.user_id = '1234') }) + describe('refreshExpiryTimeout', function() { + it('should leave expiry alone if plenty of disk', function(done) { + this.diskusage.check.callsArgWith(1, null, { + available: 40, + total: 100 + }) + + this.ProjectPersistenceManager.refreshExpiryTimeout(() => { + this.ProjectPersistenceManager.EXPIRY_TIMEOUT.should.equal( + this.settings.project_cache_length_ms + ) + done() + }) + }) + + it('should drop EXPIRY_TIMEOUT 10% if low disk usage', function(done) { + this.diskusage.check.callsArgWith(1, null, { + available: 5, + total: 100 + }) + + this.ProjectPersistenceManager.refreshExpiryTimeout(() => { + this.ProjectPersistenceManager.EXPIRY_TIMEOUT.should.equal(900) + done() + }) + }) + + it('should not drop EXPIRY_TIMEOUT to below 50% of project_cache_length_ms', function(done) { + this.diskusage.check.callsArgWith(1, null, { + available: 5, + total: 100 + }) + this.ProjectPersistenceManager.EXPIRY_TIMEOUT = 500 + this.ProjectPersistenceManager.refreshExpiryTimeout(() => { + this.ProjectPersistenceManager.EXPIRY_TIMEOUT.should.equal(500) + done() + }) + }) + + it('should not modify EXPIRY_TIMEOUT if there is an error getting disk values', function(done) { + this.diskusage.check.callsArgWith(1, 'Error', { + available: 5, + total: 100 + }) + this.ProjectPersistenceManager.refreshExpiryTimeout(() => { + this.ProjectPersistenceManager.EXPIRY_TIMEOUT.should.equal(1000) + done() + }) + }) + }) + describe('clearExpiredProjects', function() { beforeEach(function() { this.project_ids = ['project-id-1', 'project-id-2']