Merge pull request #28265 from overleaf/jpa-double-compile

[web] add c2d -> c4d double compile test

GitOrigin-RevId: 186cfa867d3db5786c6d4888dfe1ca2f46a5bb0c

services/web/app/src/Features/Compile/ClsiManager.js

@@ -26,6 +26,8 @@ const Metrics = require('@overleaf/metrics')
 const Errors = require('../Errors/Errors')
 const ClsiCacheHandler = require('./ClsiCacheHandler')
 const { getFilestoreBlobURL } = require('../History/HistoryManager')
+const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const AnalyticsManager = require('../Analytics/AnalyticsManager')
 
 const VALID_COMPILERS = ['pdflatex', 'latex', 'xelatex', 'lualatex']
 const OUTPUT_FILE_TIMEOUT_MS = 60000
@@ -352,15 +354,16 @@ async function _makeRequest(
     opts
   )
     .then(result => {
-      if (result == null) {
+      if (result == null || !url.pathname.endsWith('/compile')) {
         return
       }
-      const { response: newBackendResponse } = result
-      Metrics.inc(`compile.newBackend.response.${newBackendResponse.status}`)
+      const current = json.compile
+      const {
+        body: { compile: next },
+        newCompileBackendClass,
+      } = result
       const newBackendCompileTime = new Date() - newBackendStartTime
-      const currentStatusCode = response.status
-      const newStatusCode = newBackendResponse.status
-      const statusCodeSame = newStatusCode === currentStatusCode
+      const statusCodeSame = next.status === current.status
       const timeDifference = newBackendCompileTime - currentCompileTime
       logger.debug(
         {
@@ -372,6 +375,44 @@ async function _makeRequest(
         },
         'both clsi requests returned'
       )
+      if (
+        current.status === 'success' &&
+        current.status === next.status &&
+        current.stats.isInitialCompile === next.stats.isInitialCompile &&
+        current.stats.restoredClsiCache === next.stats.restoredClsiCache
+      ) {
+        const fraction = next.timings.compileE2E / current.timings.compileE2E
+        Metrics.histogram(
+          'compile_backend_difference_v1',
+          fraction * 100,
+          [
+            // Increment the version in the metrics name when changing the buckets.
+            0,
+            10, 20, 30, 40, 45, 50, 55, 60, 65, 70, 75, 80, 85, 90, 95, 100,
+            105, 110, 115, 120,
+          ],
+          { path: compileBackendClass, method: newCompileBackendClass }
+        )
+        AnalyticsManager.recordEventForUserInBackground(
+          userId,
+          'double-compile-result',
+          {
+            projectId,
+            compileBackendClass,
+            newCompileBackendClass,
+            status: current.status,
+            compileTime: current.timings.compileE2E,
+            newCompileTime: next.timings.compileE2E,
+            clsiServerId: newClsiServerId || clsiServerId,
+            newClsiServerId: result.newClsiServerId,
+            // Successful compiles are guaranteed to have an output.pdf file.
+            pdfSize: current.outputFiles.find(f => f.path === 'output.pdf')
+              .size,
+            newPdfSize: next.outputFiles.find(f => f.path === 'output.pdf')
+              .size,
+          }
+        )
+      }
     })
     .catch(err => {
       logger.warn({ err }, 'Error making request to new CLSI backend')
@@ -387,7 +428,7 @@ async function _makeNewBackendRequest(
   projectId,
   userId,
   compileGroup,
-  compileBackendClass,
+  currentCompileBackendClass,
   url,
   opts
 ) {
@@ -398,12 +439,33 @@ async function _makeNewBackendRequest(
     url.toString().replace(Settings.apis.clsi.url, Settings.apis.clsi_new.url)
   )
 
+  // Sample x% of premium compile projects to move up one bracket.
+  if (currentCompileBackendClass !== 'c2d') return null
+  if (
+    SplitTestHandler.getPercentile(projectId, 'double-compile', 'release') >=
+    Settings.apis.clsi_new.sample
+  )
+    return null
+
+  let newCompileBackendClass
+  switch (currentCompileBackendClass) {
+    case 'n2d':
+      newCompileBackendClass = 'c2d'
+      break
+    case 'c2d':
+      newCompileBackendClass = 'c4d'
+      break
+    default:
+      throw new Error('unknown ?compileBackendClass')
+  }
+  url.searchParams.set('compileBackendClass', newCompileBackendClass)
+
   const clsiServerId =
     await NewBackendCloudClsiCookieManager.promises.getServerId(
       projectId,
       userId,
       compileGroup,
-      compileBackendClass
+      newCompileBackendClass
     )
   opts = {
     ...opts,
@@ -440,18 +502,24 @@ async function _makeNewBackendRequest(
     // Some responses are empty. Ignore JSON parsing errors
   }
   timer.done()
+  let newClsiServerId
   if (CLSI_COOKIES_ENABLED) {
-    const newClsiServerId = _getClsiServerIdFromResponse(response)
+    newClsiServerId = _getClsiServerIdFromResponse(response)
     await NewBackendCloudClsiCookieManager.promises.setServerId(
       projectId,
       userId,
       compileGroup,
-      compileBackendClass,
+      newCompileBackendClass,
       newClsiServerId,
       clsiServerId
     )
   }
-  return { response, body: json }
+  return {
+    response,
+    body: json,
+    newCompileBackendClass,
+    newClsiServerId: clsiServerId || newClsiServerId,
+  }
 }
 
 function _getCompilerUrl(

services/web/test/unit/src/Compile/ClsiCookieManagerTests.js

@@ -52,7 +52,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         '',
-        'e2'
+        'n2d'
       )
       this.redis.get
         .calledWith(`clsiserver:${this.project_id}:${this.user_id}`)
@@ -84,7 +84,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         '',
-        'e2'
+        'n2d'
       )
       this.ClsiCookieManager.promises._populateServerIdViaRequest
         .calledWith(this.project_id, this.user_id)
@@ -115,13 +115,13 @@ describe('ClsiCookieManager', function () {
           this.project_id,
           this.user_id,
           'standard',
-          'e2'
+          'n2d'
         )
         const args = this.ClsiCookieManager.promises.setServerId.args[0]
         args[0].should.equal(this.project_id)
         args[1].should.equal(this.user_id)
         args[2].should.equal('standard')
-        args[3].should.equal('e2')
+        args[3].should.equal('n2d')
         args[4].should.deep.equal(this.clsiServerId)
       })
 
@@ -131,7 +131,7 @@ describe('ClsiCookieManager', function () {
             this.project_id,
             this.user_id,
             '',
-            'e2'
+            'n2d'
           )
         serverId.should.equal(this.clsiServerId)
       })
@@ -150,7 +150,7 @@ describe('ClsiCookieManager', function () {
           this.project_id,
           this.user_id,
           'standard',
-          'e2',
+          'n2d',
           this.clsiServerId,
           null
         )
@@ -172,7 +172,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         'standard',
-        'e2',
+        'n2d',
         this.clsiServerId,
         null
       )
@@ -189,7 +189,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         'standard',
-        'e2',
+        'n2d',
         this.clsiServerId,
         null
       )
@@ -212,7 +212,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         'standard',
-        'e2',
+        'n2d',
         this.clsiServerId,
         null
       )
@@ -240,7 +240,7 @@ describe('ClsiCookieManager', function () {
         this.project_id,
         this.user_id,
         'standard',
-        'e2',
+        'n2d',
         this.clsiServerId,
         null
       )
@@ -259,14 +259,14 @@ describe('ClsiCookieManager', function () {
             this.project_id,
             this.user_id,
             'standard',
-            'e2',
+            'n2d',
             this.clsiServerId,
             'previous-clsi-server-id'
           )
           expect(
             this.fetchUtils.fetchStringWithResponse
           ).to.have.been.calledWith(
-            `${this.settings.apis.clsi.url}/instance-state?clsiserverid=previous-clsi-server-id&compileGroup=standard&compileBackendClass=e2`,
+            `${this.settings.apis.clsi.url}/instance-state?clsiserverid=previous-clsi-server-id&compileGroup=standard&compileBackendClass=n2d`,
             { method: 'GET', signal: sinon.match.instanceOf(AbortSignal) }
           )
         }

services/web/test/unit/src/Compile/ClsiManagerTests.js

@@ -59,7 +59,21 @@ describe('ClsiManager', function () {
     this.newClsiServerId = 'newserver'
     this.rawOutputFiles = {}
     this.responseBody = {
-      compile: { status: 'success' },
+      compile: {
+        status: 'success',
+        stats: {
+          isInitialCompile: 1,
+          restoredClsiCache: 1,
+        },
+        timings: { compileE2E: 1337 },
+        outputFiles: [
+          {
+            path: 'output.pdf',
+            size: 42,
+            url: 'http://localhost:3013/snip/output.pdf',
+          },
+        ],
+      },
     }
     this.response = {
       ok: true,
@@ -128,6 +142,7 @@ describe('ClsiManager', function () {
       },
       inc: sinon.stub(),
       count: sinon.stub(),
+      histogram: sinon.stub(),
     }
     this.Settings = {
       apis: {
@@ -139,8 +154,8 @@ describe('ClsiManager', function () {
           url: `http://${CLSI_HOST}`,
           submissionBackendClass: 'n2d',
         },
-        clsi_priority: {
-          url: 'https://clsipremium.example.com',
+        clsi_new: {
+          sample: 100,
         },
       },
       enablePdfCaching: true,
@@ -157,10 +172,17 @@ describe('ClsiManager', function () {
         return `${FILESTORE_URL}/history/project/${historyId}/hash/${hash}`
       }),
     }
+    this.SplitTestHandler = {
+      getPercentile: sinon.stub().returns(42),
+    }
+    this.AnalyticsManager = {
+      recordEventForUserInBackground: sinon.stub(),
+    }
 
     this.ClsiManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '@overleaf/settings': this.Settings,
+        '../SplitTests/SplitTestHandler': this.SplitTestHandler,
         '../../models/Project': {
           Project: this.Project,
         },
@@ -175,6 +197,7 @@ describe('ClsiManager', function () {
         './ClsiFormatChecker': this.ClsiFormatChecker,
         '@overleaf/metrics': this.Metrics,
         '../History/HistoryManager': this.HistoryManager,
+        '../Analytics/AnalyticsManager': this.AnalyticsManager,
       },
     })
   })
@@ -214,7 +237,7 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           {
-            compileBackendClass: 'e2',
+            compileBackendClass: 'n2d',
             compileGroup: 'standard',
             timeout: this.timeout,
           }
@@ -228,7 +251,7 @@ describe('ClsiManager', function () {
               url.host === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}/compile` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
               url.searchParams.get('compileGroup') === 'standard'
           ),
           {
@@ -308,7 +331,7 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           'standard',
-          'e2',
+          'n2d',
           this.newClsiServerId
         )
       })
@@ -351,7 +374,7 @@ describe('ClsiManager', function () {
         this.result = await this.ClsiManager.promises.sendRequest(
           this.project._id,
           this.user_id,
-          { compileBackendClass: 'e2', compileGroup: 'standard' }
+          { compileBackendClass: 'n2d', compileGroup: 'standard' }
         )
       })
 
@@ -386,7 +409,7 @@ describe('ClsiManager', function () {
           {
             timeout: 100,
             incrementalCompilesEnabled: true,
-            compileBackendClass: 'e2',
+            compileBackendClass: 'n2d',
             compileGroup: 'priority',
             compileFromClsiCache: true,
             populateClsiCache: true,
@@ -433,7 +456,7 @@ describe('ClsiManager', function () {
               url.hostname === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}/compile` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
               url.searchParams.get('compileGroup') === 'priority'
           ),
           {
@@ -790,8 +813,8 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           {
-            compileBackendClass: 'e2',
-            compileGroup: 'standard',
+            compileBackendClass: 'c2d',
+            compileGroup: 'priority',
           }
         )
         // wait for the background task to finish
@@ -806,18 +829,34 @@ describe('ClsiManager', function () {
               url.host === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}/compile` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
-              url.searchParams.get('compileGroup') === 'standard'
+              url.searchParams.get('compileBackendClass') === 'c2d' &&
+              url.searchParams.get('compileGroup') === 'priority'
           )
         )
         expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith(
           sinon.match(
             url =>
               url.toString() ===
-              `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard`
+              `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=c4d&compileGroup=priority`
           )
         )
       })
+      it('should record an event', function () {
+        expect(
+          this.AnalyticsManager.recordEventForUserInBackground
+        ).to.have.been.calledWith(this.user_id, 'double-compile-result', {
+          projectId: 'project-id',
+          compileBackendClass: 'c2d',
+          newCompileBackendClass: 'c4d',
+          status: 'success',
+          compileTime: 1337,
+          newCompileTime: 1337,
+          clsiServerId: 'newserver',
+          newClsiServerId: 'clsi-server-id',
+          pdfSize: 42,
+          newPdfSize: 42,
+        })
+      })
     })
   })
 
@@ -852,7 +891,7 @@ describe('ClsiManager', function () {
         this.result = await this.ClsiManager.promises.sendExternalRequest(
           this.submissionId,
           this.clsiRequest,
-          { compileBackendClass: 'e2', compileGroup: 'standard' }
+          { compileBackendClass: 'n2d', compileGroup: 'standard' }
         )
       })
 
@@ -862,7 +901,7 @@ describe('ClsiManager', function () {
             url =>
               url.host === CLSI_HOST &&
               url.pathname === `/project/${this.submissionId}/compile` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
               url.searchParams.get('compileGroup') === 'standard'
           ),
           {
@@ -927,7 +966,7 @@ describe('ClsiManager', function () {
         await this.ClsiManager.promises.deleteAuxFiles(
           this.project._id,
           this.user_id,
-          { compileBackendClass: 'e2', compileGroup: 'standard' },
+          { compileBackendClass: 'n2d', compileGroup: 'standard' },
           'node-1'
         )
       })
@@ -939,7 +978,7 @@ describe('ClsiManager', function () {
               url.host === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
               url.searchParams.get('compileGroup') === 'standard' &&
               url.searchParams.get('clsiserverid') === 'node-1'
           ),
@@ -977,7 +1016,7 @@ describe('ClsiManager', function () {
         await this.ClsiManager.promises.deleteAuxFiles(
           this.project._id,
           this.user_id,
-          { compileBackendClass: 'n2d', compileGroup: 'standard' },
+          { compileBackendClass: 'c2d', compileGroup: 'priority' },
           'node-1'
         )
         // wait for the background task to finish
@@ -991,8 +1030,8 @@ describe('ClsiManager', function () {
               url.host === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}` &&
-              url.searchParams.get('compileBackendClass') === 'n2d' &&
-              url.searchParams.get('compileGroup') === 'standard' &&
+              url.searchParams.get('compileBackendClass') === 'c2d' &&
+              url.searchParams.get('compileGroup') === 'priority' &&
               url.searchParams.get('clsiserverid') === 'node-1'
           ),
           { method: 'DELETE' }
@@ -1003,8 +1042,8 @@ describe('ClsiManager', function () {
               url.host === 'compiles.somewhere.test' &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}` &&
-              url.searchParams.get('compileBackendClass') === 'n2d' &&
-              url.searchParams.get('compileGroup') === 'standard' &&
+              url.searchParams.get('compileBackendClass') === 'c4d' &&
+              url.searchParams.get('compileGroup') === 'priority' &&
               !url.searchParams.has('clsiserverid')
           ),
           sinon.match({ method: 'DELETE' })
@@ -1020,7 +1059,7 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           false,
-          { compileBackendClass: 'e2', compileGroup: 'standard' },
+          { compileBackendClass: 'n2d', compileGroup: 'standard' },
           'node-1'
         )
       })
@@ -1030,7 +1069,7 @@ describe('ClsiManager', function () {
           sinon.match(
             url =>
               url.toString() ===
-              `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1`
+              `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=n2d&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1`
           )
         )
       })
@@ -1047,7 +1086,7 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           'other.tex',
-          { compileBackendClass: 'e2', compileGroup: 'standard' },
+          { compileBackendClass: 'n2d', compileGroup: 'standard' },
           'node-2'
         )
       })
@@ -1059,7 +1098,7 @@ describe('ClsiManager', function () {
               url.host === CLSI_HOST &&
               url.pathname ===
                 `/project/${this.project._id}/user/${this.user_id}/wordcount` &&
-              url.searchParams.get('compileBackendClass') === 'e2' &&
+              url.searchParams.get('compileBackendClass') === 'n2d' &&
               url.searchParams.get('compileGroup') === 'standard' &&
               url.searchParams.get('clsiserverid') === 'node-2' &&
               url.searchParams.get('file') === 'other.tex' &&
@@ -1081,7 +1120,7 @@ describe('ClsiManager', function () {
           this.project._id,
           this.user_id,
           false,
-          { compileBackendClass: 'n2d', compileGroup: 'standard' },
+          { compileBackendClass: 'c2d', compileGroup: 'priority' },
           'node-1'
         )
         // wait for the background task to finish
@@ -1093,14 +1132,14 @@ describe('ClsiManager', function () {
           sinon.match(
             url =>
               url.toString() ===
-              `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=n2d&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1`
+              `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=c2d&compileGroup=priority&file=main.tex&image=mock-image-name&clsiserverid=node-1`
           )
         )
         expect(this.FetchUtils.fetchStringWithResponse).to.have.been.calledWith(
           sinon.match(
             url =>
               url.toString() ===
-              `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=n2d&compileGroup=standard&file=main.tex&image=mock-image-name`
+              `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=c4d&compileGroup=priority&file=main.tex&image=mock-image-name`
           )
         )
       })

services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -712,7 +712,7 @@ describe('CompileController', function () {
             .stub()
             .resolves({
               compileGroup: 'standard',
-              compileBackendClass: 'e2',
+              compileBackendClass: 'n2d',
             })
           await this.CompileController._proxyToClsi(
             this.projectId,
@@ -727,7 +727,7 @@ describe('CompileController', function () {
 
         it('should open a request to the CLSI', function () {
           this.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=e2&query=foo`
+            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=n2d&query=foo`
           )
         })
 
@@ -769,7 +769,7 @@ describe('CompileController', function () {
             .stub()
             .resolves({
               compileGroup: 'standard',
-              compileBackendClass: 'e2',
+              compileBackendClass: 'n2d',
             })
           await this.CompileController._proxyToClsi(
             this.projectId,
@@ -784,7 +784,7 @@ describe('CompileController', function () {
 
         it('should open a request to the CLSI', function () {
           this.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=e2`
+            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=n2d`
           )
         })
 
@@ -800,7 +800,7 @@ describe('CompileController', function () {
             .stub()
             .resolves({
               compileGroup: 'standard',
-              compileBackendClass: 'e2',
+              compileBackendClass: 'n2d',
             })
           await this.CompileController._proxyToClsi(
             this.projectId,
@@ -815,7 +815,7 @@ describe('CompileController', function () {
 
         it('should proxy to the standard url', function () {
           this.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=e2`
+            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=n2d`
           )
         })
       })
@@ -826,7 +826,7 @@ describe('CompileController', function () {
             .stub()
             .resolves({
               compileGroup: 'standard',
-              compileBackendClass: 'e2',
+              compileBackendClass: 'n2d',
             })
           this.req.query = { build: 1234 }
           await this.CompileController._proxyToClsi(
@@ -842,7 +842,7 @@ describe('CompileController', function () {
 
         it('should proxy to the standard url without the build parameter', function () {
           this.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith(
-            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=e2`
+            `${this.settings.apis.clsi.url}${this.url}?compileGroup=standard&compileBackendClass=n2d`
           )
         })
       })