Просмотр исходного кода

[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 <jakob.ackermann@overleaf.com>

* Add warning comments about Docker-for-Mac fs stats being off by a factor

---------

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>
GitOrigin-RevId: 02ea07e531b89bb3d10ddfe780348b19cbddad1f
Antoine Clausse 1 год назад
Родитель
Сommit
fa62529d82

+ 4 - 22
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",

+ 7 - 2
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,

+ 0 - 1
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",

+ 18 - 11
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()