Эх сурвалжийг харах

[clsi] avoid destroying containers of recently accessed projects (#32186)

* [clsi] avoid destroying containers of recently accessed projects

Co-authored-by: Anna Claire Fields <anna.fields@overleaf.com>

* [clsi] gracefully handle missing access time during container cleanup

* [clsi] fix cyclic import

---------

Co-authored-by: Anna Claire Fields <anna.fields@overleaf.com>
GitOrigin-RevId: 8195b6fccbe26d2fd673d38356af5d44cf4042a3
Jakob Ackermann 6 сар өмнө
parent
commit
a9c413857a

+ 12 - 0
services/clsi/app/js/LastProjectAccess.js

@@ -0,0 +1,12 @@
+/**
+ * LAST_ACCESS is "owned" by ProjectPersistenceManager, but needs to be accessed
+ * by the DockerRunner. Which in turn is imported through CompileManager from
+ * ProjectPersistenceManager. Avoid this cyclic import with an extra module.
+ */
+
+// projectId -> timestamp mapping.
+export const LAST_ACCESS = new Map()
+
+export function getLastProjectAccessTime(projectId) {
+  return LAST_ACCESS.get(projectId) ?? 0
+}

+ 2 - 3
services/clsi/app/js/ProjectPersistenceManager.js

@@ -17,12 +17,11 @@ import { callbackify } from 'node:util'
 import Path from 'node:path'
 import fs from 'node:fs'
 import * as HistoryResourceWriter from './HistoryResourceWriter.js'
+import { LAST_ACCESS } from './LastProjectAccess.js'
+
 let ProjectPersistenceManager
 const oneDay = 24 * 60 * 60 * 1000
 
-// projectId -> timestamp mapping.
-const LAST_ACCESS = new Map()
-
 let ANY_DISK_LOW = false
 let ANY_DISK_CRITICAL_LOW = false
 

+ 36 - 4
services/clsi/test/unit/js/DockerRunner.test.js

@@ -57,6 +57,14 @@ describe('DockerRunner', () => {
       },
     }))
 
+    ctx.LastProjectAccess = {
+      getLastProjectAccessTime: sinon
+        .stub()
+        .returns(Date.now() - 24 * 60 * 60 * 1000),
+    }
+
+    vi.doMock('../../../app/js/LastProjectAccess', () => ctx.LastProjectAccess)
+
     vi.doMock('../../../app/js/LockManager', () => ({
       default: {
         runWithLock(key, runner, callback) {
@@ -782,17 +790,26 @@ describe('DockerRunner', () => {
         const oneHourInSeconds = 60 * 60
         const oneHourInMilliseconds = oneHourInSeconds * 1000
         const nowInSeconds = Date.now() / 1000
+        ctx.recentlyAccessProjectId = '68494e7daa5c3680ee7182b1'
+        ctx.LastProjectAccess.getLastProjectAccessTime
+          .withArgs(ctx.recentlyAccessProjectId)
+          .returns(Date.now() - 1)
         ctx.containers = [
           {
-            Name: '/project-old-container-name',
+            Name: '/project-69b72c66b1a8c2f5846b24a8-container-name',
             Id: 'old-container-id',
             Created: nowInSeconds - oneHourInSeconds - 100,
           },
           {
-            Name: '/project-new-container-name',
+            Name: '/project-69b72c66b1a8c2f5846b24a9-container-name',
             Id: 'new-container-id',
             Created: nowInSeconds - oneHourInSeconds + 100,
           },
+          {
+            Name: `/project-${ctx.recentlyAccessProjectId}-container-name`,
+            Id: 'recent-access-container-id',
+            Created: nowInSeconds - 2 * oneHourInSeconds,
+          },
           {
             Name: '/totally-not-a-project-container',
             Id: 'some-random-id',
@@ -816,13 +833,28 @@ describe('DockerRunner', () => {
     it('should destroy old containers', ctx => {
       ctx.DockerRunner.destroyContainer.callCount.should.equal(1)
       ctx.DockerRunner.destroyContainer
-        .calledWith('project-old-container-name', 'old-container-id')
+        .calledWith(
+          'project-69b72c66b1a8c2f5846b24a8-container-name',
+          'old-container-id'
+        )
         .should.equal(true)
     })
 
     it('should not destroy new containers', ctx => {
       ctx.DockerRunner.destroyContainer
-        .calledWith('project-new-container-name', 'new-container-id')
+        .calledWith(
+          'project-69b72c66b1a8c2f5846b24a9-container-name',
+          'new-container-id'
+        )
+        .should.equal(false)
+    })
+
+    it('should not destroy old containers that were recently used', ctx => {
+      ctx.DockerRunner.destroyContainer
+        .calledWith(
+          'project-68494e7daa5c3680ee7182b1-container-name',
+          'recent-access-container-id'
+        )
         .should.equal(false)
     })