From fa62529d82b04f2f6dd46198b2e6d3471198271a Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Thu, 17 Apr 2025 08:46:21 +0200 Subject: [PATCH] [clsi] Replace `diskusage` by `fs` (#24789) * Replace `diskusage` by `fs` in clsi * Replace `diskusage` by `fs` in clsi-cache * Update disk space calculations to include block size in bytes Co-authored-by: Jakob Ackermann * Add warning comments about Docker-for-Mac fs stats being off by a factor --------- Co-authored-by: Jakob Ackermann GitOrigin-RevId: 02ea07e531b89bb3d10ddfe780348b19cbddad1f --- package-lock.json | 26 +++-------------- .../clsi/app/js/ProjectPersistenceManager.js | 9 ++++-- services/clsi/package.json | 1 - .../unit/js/ProjectPersistenceManagerTests.js | 29 ++++++++++++------- 4 files changed, 29 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index e007d34307..986630ccc3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19782,16 +19782,6 @@ "node": ">=8" } }, - "node_modules/diskusage": { - "version": "1.1.3", - "resolved": "https://registry.npmjs.org/diskusage/-/diskusage-1.1.3.tgz", - "integrity": "sha512-EAyaxl8hy4Ph07kzlzGTfpbZMNAAAHXSZtNEMwdlnSd1noHzvA6HsgKt4fEMSvaEXQYLSphe5rPMxN4WOj0hcQ==", - "hasInstallScript": true, - "dependencies": { - "es6-promise": "^4.2.5", - "nan": "^2.14.0" - } - }, "node_modules/dnd-core": { "version": "16.0.1", "resolved": "https://registry.npmjs.org/dnd-core/-/dnd-core-16.0.1.tgz", @@ -30316,9 +30306,10 @@ "dev": true }, "node_modules/nan": { - "version": "2.17.0", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.17.0.tgz", - "integrity": "sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ==" + "version": "2.22.2", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.22.2.tgz", + "integrity": "sha512-DANghxFkS1plDdRsX0X9pm0Z6SJNN6gBdtXfanwoZ8hooC5gosGFSBGRYHUVPz1asKA/kMRqDRdHrluZ61SpBQ==", + "license": "MIT" }, "node_modules/nanoclone": { "version": "0.2.1", @@ -41995,7 +41986,6 @@ "async": "^3.2.5", "body-parser": "^1.20.3", "bunyan": "^1.8.15", - "diskusage": "^1.1.3", "dockerode": "^4.0.5", "express": "^4.21.2", "lodash": "^4.17.21", @@ -42031,7 +42021,6 @@ "body-parser": "^1.20.3", "bunyan": "^1.8.15", "celebrate": "^15.0.3", - "diskusage": "^1.1.3", "express": "^4.21.2" }, "devDependencies": { @@ -42150,13 +42139,6 @@ "tar-stream": "^2.1.4" } }, - "services/clsi/node_modules/nan": { - "version": "2.22.2", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.22.2.tgz", - "integrity": "sha512-DANghxFkS1plDdRsX0X9pm0Z6SJNN6gBdtXfanwoZ8hooC5gosGFSBGRYHUVPz1asKA/kMRqDRdHrluZ61SpBQ==", - "license": "MIT", - "optional": true - }, "services/clsi/node_modules/protobufjs": { "version": "7.4.0", "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-7.4.0.tgz", diff --git a/services/clsi/app/js/ProjectPersistenceManager.js b/services/clsi/app/js/ProjectPersistenceManager.js index 7d4f071d2c..e96a4591c3 100644 --- a/services/clsi/app/js/ProjectPersistenceManager.js +++ b/services/clsi/app/js/ProjectPersistenceManager.js @@ -15,7 +15,6 @@ const logger = require('@overleaf/logger') const oneDay = 24 * 60 * 60 * 1000 const Metrics = require('@overleaf/metrics') const Settings = require('@overleaf/settings') -const diskusage = require('diskusage') const { callbackify } = require('node:util') const Path = require('node:path') const fs = require('node:fs') @@ -33,7 +32,13 @@ async function collectDiskStats() { const diskStats = {} for (const path of paths) { try { - const stats = await diskusage.check(path) + const { blocks, bavail, bsize } = await fs.promises.statfs(path) + const stats = { + // Warning: these values will be wrong by a factor in Docker-for-Mac. + // See https://github.com/docker/for-mac/issues/2136 + total: blocks * bsize, // Total size of the file system in bytes + available: bavail * bsize, // Free space available to unprivileged users. + } const diskAvailablePercent = (stats.available / stats.total) * 100 Metrics.gauge('disk_available_percent', diskAvailablePercent, 1, { path, diff --git a/services/clsi/package.json b/services/clsi/package.json index b3a64e35df..86566e0f59 100644 --- a/services/clsi/package.json +++ b/services/clsi/package.json @@ -27,7 +27,6 @@ "async": "^3.2.5", "body-parser": "^1.20.3", "bunyan": "^1.8.15", - "diskusage": "^1.1.3", "dockerode": "^4.0.5", "express": "^4.21.2", "lodash": "^4.17.21", diff --git a/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js b/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js index 2504d266ca..4f42411fba 100644 --- a/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js +++ b/services/clsi/test/unit/js/ProjectPersistenceManagerTests.js @@ -21,12 +21,16 @@ const tk = require('timekeeper') describe('ProjectPersistenceManager', function () { beforeEach(function () { + this.fsPromises = { + statfs: sinon.stub(), + } + this.ProjectPersistenceManager = SandboxedModule.require(modulePath, { requires: { '@overleaf/metrics': (this.Metrics = { gauge: sinon.stub() }), './UrlCache': (this.UrlCache = {}), './CompileManager': (this.CompileManager = {}), - diskusage: (this.diskusage = { check: sinon.stub() }), + fs: { promises: this.fsPromises }, '@overleaf/settings': (this.settings = { project_cache_length_ms: 1000, path: { @@ -44,9 +48,10 @@ describe('ProjectPersistenceManager', function () { describe('refreshExpiryTimeout', function () { it('should leave expiry alone if plenty of disk', function (done) { - this.diskusage.check.resolves({ - available: 40, - total: 100, + this.fsPromises.statfs.resolves({ + blocks: 100, + bsize: 1, + bavail: 40, }) this.ProjectPersistenceManager.refreshExpiryTimeout(() => { @@ -62,9 +67,10 @@ describe('ProjectPersistenceManager', function () { }) it('should drop EXPIRY_TIMEOUT 10% if low disk usage', function (done) { - this.diskusage.check.resolves({ - available: 5, - total: 100, + this.fsPromises.statfs.resolves({ + blocks: 100, + bsize: 1, + bavail: 5, }) this.ProjectPersistenceManager.refreshExpiryTimeout(() => { @@ -78,9 +84,10 @@ describe('ProjectPersistenceManager', function () { }) it('should not drop EXPIRY_TIMEOUT to below 50% of project_cache_length_ms', function (done) { - this.diskusage.check.resolves({ - available: 5, - total: 100, + this.fsPromises.statfs.resolves({ + blocks: 100, + bsize: 1, + bavail: 5, }) this.ProjectPersistenceManager.EXPIRY_TIMEOUT = 500 this.ProjectPersistenceManager.refreshExpiryTimeout(() => { @@ -94,7 +101,7 @@ describe('ProjectPersistenceManager', function () { }) it('should not modify EXPIRY_TIMEOUT if there is an error getting disk values', function (done) { - this.diskusage.check.throws(new Error()) + this.fsPromises.statfs.rejects(new Error()) this.ProjectPersistenceManager.refreshExpiryTimeout(() => { this.ProjectPersistenceManager.EXPIRY_TIMEOUT.should.equal(1000) done()