[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
This commit is contained in:
Jakob Ackermann
2026-03-17 12:46:30 +01:00
committed by Copybot
parent efa01e6282
commit a9c413857a
3 changed files with 50 additions and 7 deletions

View File

@@ -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
}

View File

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

View File

@@ -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', () => ({ vi.doMock('../../../app/js/LockManager', () => ({
default: { default: {
runWithLock(key, runner, callback) { runWithLock(key, runner, callback) {
@@ -782,17 +790,26 @@ describe('DockerRunner', () => {
const oneHourInSeconds = 60 * 60 const oneHourInSeconds = 60 * 60
const oneHourInMilliseconds = oneHourInSeconds * 1000 const oneHourInMilliseconds = oneHourInSeconds * 1000
const nowInSeconds = Date.now() / 1000 const nowInSeconds = Date.now() / 1000
ctx.recentlyAccessProjectId = '68494e7daa5c3680ee7182b1'
ctx.LastProjectAccess.getLastProjectAccessTime
.withArgs(ctx.recentlyAccessProjectId)
.returns(Date.now() - 1)
ctx.containers = [ ctx.containers = [
{ {
Name: '/project-old-container-name', Name: '/project-69b72c66b1a8c2f5846b24a8-container-name',
Id: 'old-container-id', Id: 'old-container-id',
Created: nowInSeconds - oneHourInSeconds - 100, Created: nowInSeconds - oneHourInSeconds - 100,
}, },
{ {
Name: '/project-new-container-name', Name: '/project-69b72c66b1a8c2f5846b24a9-container-name',
Id: 'new-container-id', Id: 'new-container-id',
Created: nowInSeconds - oneHourInSeconds + 100, 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', Name: '/totally-not-a-project-container',
Id: 'some-random-id', Id: 'some-random-id',
@@ -816,13 +833,28 @@ describe('DockerRunner', () => {
it('should destroy old containers', ctx => { it('should destroy old containers', ctx => {
ctx.DockerRunner.destroyContainer.callCount.should.equal(1) ctx.DockerRunner.destroyContainer.callCount.should.equal(1)
ctx.DockerRunner.destroyContainer ctx.DockerRunner.destroyContainer
.calledWith('project-old-container-name', 'old-container-id') .calledWith(
'project-69b72c66b1a8c2f5846b24a8-container-name',
'old-container-id'
)
.should.equal(true) .should.equal(true)
}) })
it('should not destroy new containers', ctx => { it('should not destroy new containers', ctx => {
ctx.DockerRunner.destroyContainer 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) .should.equal(false)
}) })