From a2ad326b30ee25de8ef172f59eec214cc2319bfc Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 10 Oct 2023 11:42:32 +0200 Subject: [PATCH] Merge pull request #15169 from overleaf/tm-compile-timeout-20s-phase-two Decide new user cutoff using baseline (original) n2d assignment GitOrigin-RevId: 7ea263cc551f87a4d9fed70450c32d7dea6b1e58 --- .../src/Features/Compile/CompileManager.js | 21 +++++- .../Features/Editor/EditorHttpController.js | 22 ++++++- .../unit/src/Compile/CompileManagerTests.js | 66 +++++++++++++++++-- .../src/Editor/EditorHttpControllerTests.js | 2 + 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/services/web/app/src/Features/Compile/CompileManager.js b/services/web/app/src/Features/Compile/CompileManager.js index 29a2d2edc0..9b2c43d569 100644 --- a/services/web/app/src/Features/Compile/CompileManager.js +++ b/services/web/app/src/Features/Compile/CompileManager.js @@ -12,9 +12,13 @@ const SplitTestHandler = require('../SplitTests/SplitTestHandler') const { getAnalyticsIdFromMongoUser } = require('../Analytics/AnalyticsHelper') const NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF = new Date('2023-09-18T11:00:00.000Z') +const NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE = new Date( + '2023-10-08T11:00:00.000Z' +) module.exports = CompileManager = { NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF, + NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE, compile(projectId, userId, options = {}, _callback) { const timer = new Metrics.Timer('editor.compile') @@ -200,10 +204,21 @@ module.exports = CompileManager = { 'compile-timeout-20s', (err, assignment) => { if (err) return callback(err) + // users who were on the 'default' servers at time of original rollout + // will have a later cutoff date for the 20s timeout in the next phase + // we check the backend class at version 8 (baseline) + const backendClassHistory = + owner.splitTests?.['compile-backend-class-n2d'] || [] + const backendClassBaselineVariant = + backendClassHistory.find(version => { + return version.versionNumber === 8 + })?.variantName + const timeoutEnforcedCutoff = + backendClassBaselineVariant === 'default' + ? NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE + : NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF if (assignment?.variant === '20s') { - if ( - owner.signUpDate > NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF - ) { + if (owner.signUpDate > timeoutEnforcedCutoff) { limits.timeout = 20 } } diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index e0555acff9..1ac47d08fe 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -17,7 +17,9 @@ const { expressify } = require('../../util/promises') const SplitTestHandler = require('../SplitTests/SplitTestHandler') const { NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF, + NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE, } = require('../Compile/CompileManager') +const UserGetter = require('../User/UserGetter') module.exports = { joinProject: expressify(joinProject), @@ -85,7 +87,25 @@ async function joinProject(req, res, next) { 'compile-timeout-20s' ) if (timeoutAssignment?.variant === '20s') { - if (project.owner.signUpDate > NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF) { + // users who were on the 'default' servers at time of original rollout + // will have a later cutoff date for the 20s timeout in the next phase + // we check the backend class at version 8 (baseline) + const owner = await UserGetter.promises.getUser(project.owner._id, { + _id: 1, + 'splitTests.compile-backend-class-n2d': 1, + }) + const backendClassHistory = + owner.splitTests?.['compile-backend-class-n2d'] || [] + const backendClassBaselineVariant = backendClassHistory.find( + version => { + return version.versionNumber === 8 + } + )?.variantName + const timeoutEnforcedCutoff = + backendClassBaselineVariant === 'default' + ? NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE + : NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF + if (project.owner.signUpDate > timeoutEnforcedCutoff) { // New users will see a 10s warning and compile fail at 20s project.showNewCompileTimeoutUI = 'active' } else { diff --git a/services/web/test/unit/src/Compile/CompileManagerTests.js b/services/web/test/unit/src/Compile/CompileManagerTests.js index 0ef54edc5d..892dc871bc 100644 --- a/services/web/test/unit/src/Compile/CompileManagerTests.js +++ b/services/web/test/unit/src/Compile/CompileManagerTests.js @@ -261,10 +261,6 @@ describe('CompileManager', function () { null, (this.user = { features: this.features, analyticsId: 'abc' }) ) - this.CompileManager.getProjectCompileLimits( - this.project_id, - this.callback - ) }) describe('user is in the n2d group and compile-timeout-20s split test variant', function () { @@ -320,6 +316,68 @@ describe('CompileManager', function () { .should.equal(true) }) }) + + describe('user was in the default n2d variant at the baseline test version', function () { + beforeEach(function () { + this.UserGetter.getUser = sinon.stub().callsArgWith( + 2, + null, + (this.user = { + features: this.features, + analyticsId: 'abc', + splitTests: { + 'compile-backend-class-n2d': [ + { + variantName: 'default', + versionNumber: 8, + phase: 'release', + }, + ], + }, + }) + ) + }) + + describe('user signed up after the original rollout but before the second phase rollout', function () { + beforeEach(function () { + const signUpDate = new Date( + this.CompileManager.NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF + ) + signUpDate.setDate(signUpDate.getDate() + 1) + this.user.signUpDate = signUpDate + }) + + it('should keep the users compile timeout', function () { + this.CompileManager.getProjectCompileLimits( + this.project_id, + this.callback + ) + this.callback + .calledWith(null, sinon.match({ timeout: 60 })) + .should.equal(true) + }) + }) + + describe('user signed up after the second phase rollout', function () { + beforeEach(function () { + const signUpDate = new Date( + this.CompileManager.NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE + ) + signUpDate.setDate(signUpDate.getDate() + 1) + this.user.signUpDate = signUpDate + }) + + it('should reduce compile timeout to 20s', function () { + this.CompileManager.getProjectCompileLimits( + this.project_id, + this.callback + ) + this.callback + .calledWith(null, sinon.match({ timeout: 20 })) + .should.equal(true) + }) + }) + }) }) }) diff --git a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js index 5c935d39db..6c9fde45f2 100644 --- a/services/web/test/unit/src/Editor/EditorHttpControllerTests.js +++ b/services/web/test/unit/src/Editor/EditorHttpControllerTests.js @@ -138,6 +138,7 @@ describe('EditorHttpController', function () { .resolves({ variant: 'default' }), }, } + this.UserGetter = { promises: { getUser: sinon.stub().resolves(null, {}) } } this.EditorHttpController = SandboxedModule.require(MODULE_PATH, { requires: { '../Project/ProjectDeleter': this.ProjectDeleter, @@ -159,6 +160,7 @@ describe('EditorHttpController', function () { '../Errors/HttpErrorHandler': this.HttpErrorHandler, '../SplitTests/SplitTestHandler': this.SplitTestHandler, '../Compile/CompileManager': {}, + '../User/UserGetter': this.UserGetter, }, }) })