From a8b4f2dc5526b0d31b4aec2f30d6b98bcf2c75ff Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 23 Jun 2026 14:04:58 +0200 Subject: [PATCH] [web] reduce mongo lookups for compile requests (#34890) * [web] _getUserCompileLimits: use owner.analyticsId directly * [web] deduplicate project lookups for compile requests * [web] add test case for prefetched project GitOrigin-RevId: 0e140619c874accf8673153ed90af2ce70cf9234 --- .../app/src/Features/Compile/ClsiManager.mjs | 41 ++++--- .../src/Features/Compile/CompileManager.mjs | 38 ++++--- .../unit/src/Compile/ClsiManager.test.mjs | 106 +++++++++++++++++- .../unit/src/Compile/CompileManager.test.mjs | 78 ++++++++++--- 4 files changed, 210 insertions(+), 53 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiManager.mjs b/services/web/app/src/Features/Compile/ClsiManager.mjs index effad9f39f..a659796fce 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.mjs +++ b/services/web/app/src/Features/Compile/ClsiManager.mjs @@ -150,25 +150,22 @@ function collectMetricsOnBlgFiles(outputFiles) { Metrics.count('blg_output_file', nested, 1, { path: 'nested' }) } -async function sendRequest(projectId, userId, options) { - if (options == null) { - options = {} - } - let result = await sendRequestOnce(projectId, userId, options) +async function sendRequest(project, projectId, userId, options) { + let result = await sendRequestOnce(project, projectId, userId, options) if (result.status === 'missing-updates') { // try again with updated baseline - result = await sendRequestOnce(projectId, userId, { + result = await sendRequestOnce(project, projectId, userId, { ...options, baseHistoryVersion: result.baseHistoryVersion, }) } else if (result.status === 'conflict') { // Try again, with a full compile - result = await sendRequestOnce(projectId, userId, { + result = await sendRequestOnce(null, projectId, userId, { ...options, syncType: 'full', }) } else if (result.status === 'unavailable') { - result = await sendRequestOnce(projectId, userId, { + result = await sendRequestOnce(null, projectId, userId, { ...options, syncType: 'full', forceNewClsiServer: true, @@ -177,10 +174,10 @@ async function sendRequest(projectId, userId, options) { return result } -async function sendRequestOnce(projectId, userId, options) { +async function sendRequestOnce(project, projectId, userId, options) { let req try { - req = await _buildRequest(projectId, userId, options) + req = await _buildRequest(project, projectId, userId, options) } catch (err) { if (err.message === 'no main file specified') { return { @@ -735,13 +732,15 @@ function _parseOutputFiles(projectId, rawOutputFiles = []) { return outputFiles } -async function _buildRequest(projectId, userId, options) { - const project = await ProjectGetter.promises.getProject(projectId, { - compiler: 1, - imageName: 1, - 'overleaf.history.id': 1, - ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }), - }) +async function _buildRequest(project, projectId, userId, options) { + if (project === null) { + project = await ProjectGetter.promises.getProject(projectId, { + compiler: 1, + imageName: 1, + 'overleaf.history.id': 1, + ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }), + }) + } if (project == null) { throw new Errors.NotFoundError(`project does not exist: ${projectId}`) } @@ -770,7 +769,7 @@ async function _buildRequest(projectId, userId, options) { 'failed to compose history-full request' ) // fall back to old compile mode - return await _buildRequest(projectId, userId, { + return await _buildRequest(null, projectId, userId, { ...options, compileFromHistory: false, }) @@ -791,7 +790,7 @@ async function _buildRequest(projectId, userId, options) { 'failed to compose history-incremental request' ) // fall back to old compile mode - return await _buildRequest(projectId, userId, { + return await _buildRequest(null, projectId, userId, { ...options, compileFromHistory: false, }) @@ -1175,7 +1174,7 @@ function _finaliseRequest(projectId, options, project, docs, files) { } async function buildDocumentConversionRequest(projectId, userId, options) { - return await _buildRequest(projectId, userId, { + return await _buildRequest(null, projectId, userId, { ...options, // Use the history snapshot as populated on clsi-cache. populateClsiCache: true, @@ -1186,7 +1185,7 @@ async function buildDocumentConversionRequest(projectId, userId, options) { async function wordCount(projectId, userId, file, limits, clsiserverid) { const { compileBackendClass, compileGroup } = limits - const req = await _buildRequest(projectId, userId, limits) + const req = await _buildRequest(null, projectId, userId, limits) const filename = file || req.compile.rootResourcePath const url = _getCompilerUrl( compileBackendClass, diff --git a/services/web/app/src/Features/Compile/CompileManager.mjs b/services/web/app/src/Features/Compile/CompileManager.mjs index 3cc9259a87..b6cfd45b4d 100644 --- a/services/web/app/src/Features/Compile/CompileManager.mjs +++ b/services/web/app/src/Features/Compile/CompileManager.mjs @@ -7,8 +7,8 @@ import UserGetter from '../User/UserGetter.mjs' import ClsiManager from './ClsiManager.mjs' import Metrics from '@overleaf/metrics' import { RateLimiter } from '../../infrastructure/RateLimiter.mjs' -import UserAnalyticsDataCache from '../Analytics/UserAnalyticsDataCache.mjs' import { callbackify, callbackifyMultiResult } from '@overleaf/promise-utils' +import Errors from '../Errors/Errors.js' let CompileManager const rclient = RedisWrapper.client('clsi_recently_compiled') @@ -63,8 +63,23 @@ async function compile(projectId, userId, options = {}) { } } - const limits = - await CompileManager.promises.getProjectCompileLimits(projectId) + // Generate the buildId ahead of fetching the project content from redis/mongo so that the buildId's timestamp is before any lastUpdated date. + options.buildId = generateBuildId() + + const project = await ProjectGetter.promises.getProject(projectId, { + // _getProjectCompileLimits + owner_ref: 1, + fromV1TemplateId: 1, + // _build_request + compiler: 1, + imageName: 1, + 'overleaf.history.id': 1, + ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }), + }) + if (project == null) { + throw new Errors.NotFoundError(`project does not exist: ${projectId}`) + } + const limits = await _getProjectCompileLimits(project) for (const key in limits) { const value = limits[key] options[key] = value @@ -82,9 +97,6 @@ async function compile(projectId, userId, options = {}) { return { message: 'autocompile-backoff', outputFiles: [] } } - // Generate the buildId ahead of fetching the project content from redis/mongo so that the buildId's timestamp is before any lastUpdated date. - options.buildId = generateBuildId() - // only pass userId down to clsi if this is a per-user compile const compileAsUser = Settings.disablePerUserCompiles ? undefined : userId const { @@ -99,7 +111,12 @@ async function compile(projectId, userId, options = {}) { clsiCacheShard, baseHistoryVersion, instanceType, - } = await ClsiManager.promises.sendRequest(projectId, compileAsUser, options) + } = await ClsiManager.promises.sendRequest( + project, + projectId, + compileAsUser, + options + ) return { status, @@ -153,11 +170,6 @@ async function _getUserCompileLimits(userId) { ownerFeatures.compileGroup = 'alpha' } - const analyticsId = await UserAnalyticsDataCache.getAnalyticsId( - owner._id, - '_getUserCompileLimits' - ) - const compileGroup = ownerFeatures.compileGroup || Settings.defaultFeatures.compileGroup const limits = { @@ -168,7 +180,7 @@ async function _getUserCompileLimits(userId) { compileGroup === 'standard' ? Settings.apis.clsi.standardCompileBackendClass : Settings.apis.clsi.priorityCompileBackendClass, - ownerAnalyticsId: analyticsId, + ownerAnalyticsId: owner.analyticsId, } return limits diff --git a/services/web/test/unit/src/Compile/ClsiManager.test.mjs b/services/web/test/unit/src/Compile/ClsiManager.test.mjs index 95c96f2cc7..edc9c1ea84 100644 --- a/services/web/test/unit/src/Compile/ClsiManager.test.mjs +++ b/services/web/test/unit/src/Compile/ClsiManager.test.mjs @@ -332,6 +332,7 @@ describe('ClsiManager', function () { ctx.responseBody.compile.buildId = buildId ctx.timeout = 100 ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -435,6 +436,87 @@ describe('ClsiManager', function () { }) }) + describe('with the project prefetched', function () { + const buildId = '18fbe9e7564-30dcb2f71250c690' + + beforeEach(async function (ctx) { + ctx.outputFiles = [ + { + url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.pdf`, + path: 'output.pdf', + type: 'pdf', + build: buildId, + }, + { + url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.log`, + path: 'output.log', + type: 'log', + build: buildId, + }, + ] + ctx.responseBody.compile.outputFiles = ctx.outputFiles.map( + outputFile => ({ + ...outputFile, + url: `http://${CLSI_HOST}${outputFile.url}`, + }) + ) + ctx.responseBody.compile.buildId = buildId + ctx.timeout = 100 + ctx.result = await ctx.ClsiManager.promises.sendRequest( + ctx.project, + ctx.project._id, + ctx.user_id, + { + compileBackendClass: 'free', + compileGroup: 'standard', + timeout: ctx.timeout, + } + ) + }) + + it('should send the request to the CLSI', function (ctx) { + ctx.FetchUtils.fetchStringWithResponse.should.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === + `/project/${ctx.project._id}/user/${ctx.user_id}/compile` && + url.searchParams.get('compileBackendClass') === 'free' && + url.searchParams.get('compileGroup') === 'standard' + ), + { + method: 'POST', + json: sinon.match({ + compile: { + options: { + compiler: ctx.project.compiler, + imageName: ctx.project.imageName, + timeout: ctx.timeout, + draft: false, + compileGroup: 'standard', + metricsMethod: 'standard', + stopOnFirstError: false, + syncType: undefined, + }, + rootResourcePath: 'main.tex', + resources: _makeResources(ctx.project, ctx.docs, ctx.files), + }, + }), + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + Cookie: `${ctx.clsiCookieKey}=${ctx.clsiServerId}`, + }, + signal: sinon.match.instanceOf(AbortSignal), + } + ) + }) + + it('should get the project with the required fields', function (ctx) { + ctx.ProjectGetter.promises.getProject.should.not.have.been.called + }) + }) + describe('with compile from history fallback to incremental', function () { const buildId = '18fbe9e7564-30dcb2f71250c690' @@ -470,6 +552,7 @@ describe('ClsiManager', function () { 'mock-doc-id-1': 'main.tex', }) ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -619,6 +702,7 @@ describe('ClsiManager', function () { ctx.responseBody.compile.stats = ctx.stats ctx.responseBody.compile.timings = ctx.timings ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { compileBackendClass: 'free', compileGroup: 'standard' } @@ -651,6 +735,7 @@ describe('ClsiManager', function () { 'mock-doc-id-1': 'main.tex', }) ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -762,6 +847,7 @@ describe('ClsiManager', function () { 'mock-doc-id-2': '/chapters/chapter1.tex', }) await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -785,6 +871,7 @@ describe('ClsiManager', function () { describe('when root doc override is valid', function () { beforeEach(async function (ctx) { await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { rootDoc_id: 'mock-doc-id-2' } @@ -804,6 +891,7 @@ describe('ClsiManager', function () { describe('when root doc override is invalid', function () { beforeEach(async function (ctx) { await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { rootDoc_id: 'invalid-id' } @@ -824,6 +912,7 @@ describe('ClsiManager', function () { beforeEach(async function (ctx) { ctx.project.compiler = 'context' await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -844,6 +933,7 @@ describe('ClsiManager', function () { beforeEach(async function (ctx) { ctx.project.rootDoc_id = 'not-valid' await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -877,6 +967,7 @@ describe('ClsiManager', function () { } ctx.ProjectEntityHandler.promises.getAllDocs.resolves(ctx.docs) ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -900,6 +991,7 @@ describe('ClsiManager', function () { } ctx.ProjectEntityHandler.promises.getAllDocs.resolves(ctx.docs) await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -919,6 +1011,7 @@ describe('ClsiManager', function () { describe('with the draft option', function () { beforeEach(async function (ctx) { await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -942,6 +1035,7 @@ describe('ClsiManager', function () { beforeEach(async function (ctx) { ctx.responseBody.compile.status = 'failure' ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -970,6 +1064,7 @@ describe('ClsiManager', function () { response: ctx.response, }) ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, {} @@ -1006,6 +1101,7 @@ describe('ClsiManager', function () { response: ctx.response, }) ctx.result = await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { compileBackendClass: 'free' } @@ -1050,7 +1146,12 @@ describe('ClsiManager', function () { it('should throw an error', async function (ctx) { await expect( - ctx.ClsiManager.promises.sendRequest(ctx.project._id, ctx.user_id, {}) + ctx.ClsiManager.promises.sendRequest( + null, + ctx.project._id, + ctx.user_id, + {} + ) ).to.be.rejected }) }) @@ -1059,6 +1160,7 @@ describe('ClsiManager', function () { beforeEach(async function (ctx) { ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test' await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -1113,6 +1215,7 @@ describe('ClsiManager', function () { ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test' ctx.Settings.apis.clsi_new.doubleCompileFree.sample = 0 await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { @@ -1143,6 +1246,7 @@ describe('ClsiManager', function () { beforeEach(async function (ctx) { ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test' await ctx.ClsiManager.promises.sendRequest( + null, ctx.project._id, ctx.user_id, { diff --git a/services/web/test/unit/src/Compile/CompileManager.test.mjs b/services/web/test/unit/src/Compile/CompileManager.test.mjs index 4f226ff31c..1ac6d1f099 100644 --- a/services/web/test/unit/src/Compile/CompileManager.test.mjs +++ b/services/web/test/unit/src/Compile/CompileManager.test.mjs @@ -1,5 +1,6 @@ import { vi, expect } from 'vitest' import sinon from 'sinon' +import _ from 'lodash' const MODULE_PATH = '../../../../app/src/Features/Compile/CompileManager.mjs' @@ -16,6 +17,45 @@ describe('CompileManager', function () { inc: sinon.stub(), } + ctx.project = { + _id: 'project-id', + owner_ref: 'owner-id', + compiler: 'latex', + rootDoc_id: 'mock-doc-id-1', + imageName: 'mock-image-name', + overleaf: { history: { id: 42 } }, + fromV1TemplateId: 1337, + rootFolder: [ + { + docs: [], + files: [], + folders: [], + }, + ], + } + + ctx.user = { + _id: 'owner-id', + features: { compileTimeout: 42, compileGroup: 'standard' }, + analyticsId: 'abc', + } + + ctx.ProjectGetter = { + promises: { + getProject: sinon.stub().callsFake((projectId, projection) => { + const result = { _id: ctx.project._id } + for (const [field, v] of Object.entries(projection)) { + if (v) { + _.set(result, field, _.get(ctx.project, field)) + } else { + _.unset(result, field) + } + } + return result + }), + }, + } + vi.doMock('@overleaf/settings', () => ({ default: (ctx.settings = { apis: { @@ -49,11 +89,15 @@ describe('CompileManager', function () { ) vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({ - default: (ctx.ProjectGetter = { promises: {} }), + default: ctx.ProjectGetter, })) vi.doMock('../../../../app/src/Features/User/UserGetter', () => ({ - default: (ctx.UserGetter = { promises: {} }), + default: (ctx.UserGetter = { + promises: { + getUser: sinon.stub().resolves(ctx.user), + }, + }), })) vi.doMock('../../../../app/src/Features/Compile/ClsiManager', () => ({ @@ -105,9 +149,6 @@ describe('CompileManager', function () { rootDocId: 'mock-root-doc-id-123', rootResourcePath: '/main.tex', }) - ctx.CompileManager.promises.getProjectCompileLimits = sinon - .stub() - .resolves(ctx.limits) ctx.ClsiManager.promises.sendRequest = sinon.stub().resolves({ status: (ctx.status = 'mock-status'), outputFiles: (ctx.outputFiles = []), @@ -122,17 +163,6 @@ describe('CompileManager', function () { isAutoCompile, compileGroup ) => true - ctx.ProjectGetter.promises.getProject = sinon - .stub() - .resolves( - (ctx.project = { owner_ref: (ctx.owner_id = 'owner-id-123') }) - ) - ctx.UserGetter.promises.getUser = sinon.stub().resolves( - (ctx.user = { - features: { compileTimeout: '20s', compileGroup: 'standard' }, - analyticsId: 'abc', - }) - ) result = await ctx.CompileManager.promises.compile( ctx.project_id, ctx.user_id, @@ -153,13 +183,23 @@ describe('CompileManager', function () { }) it('should get the project compile limits', function (ctx) { - ctx.CompileManager.promises.getProjectCompileLimits - .calledWith(ctx.project_id) + ctx.UserGetter.promises.getUser + .calledWith(ctx.project.owner_ref) .should.equal(true) }) it('should run the compile with the compile limits', function (ctx) { ctx.ClsiManager.promises.sendRequest.should.have.been.calledWith( + { + _id: 'project-id', + compiler: 'latex', + fromV1TemplateId: 1337, + imageName: 'mock-image-name', + overleaf: { history: { id: 42 } }, + owner_ref: 'owner-id', + rootDoc_id: 'mock-doc-id-1', + rootFolder: [{ docs: [], files: [], folders: [] }], + }, ctx.project_id, ctx.user_id, { @@ -168,6 +208,8 @@ describe('CompileManager', function () { buildId: sinon.match(/[a-f0-9]+-[a-f0-9]+/), rootResourcePath: 'main.tex', rootDoc_id: 'mock-root-doc-id-123', + compileBackendClass: 'free', + ownerAnalyticsId: 'abc', } ) })