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

Merge pull request #15169 from overleaf/tm-compile-timeout-20s-phase-two

Decide new user cutoff using baseline (original) n2d assignment

GitOrigin-RevId: 7ea263cc551f87a4d9fed70450c32d7dea6b1e58
Thomas 2 лет назад
Родитель
Сommit
03aaabd5cb

+ 18 - 3
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
                         }
                       }

+ 21 - 1
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 {

+ 62 - 4
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)
+          })
+        })
+      })
     })
   })
 

+ 2 - 0
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,
       },
     })
   })