Merge pull request #14836 from overleaf/ab-split-test-cache-refactoring

[web] Fetch all active split test into cache at once

GitOrigin-RevId: b477b88bf281349433af2cf692a0e9ea5b036588
This commit is contained in:
Alexandre Bourdin 2023-10-16 14:01:16 +02:00 committed by Copybot
parent 719da5fbd8
commit b8a5eca1d0
4 changed files with 60 additions and 33 deletions

View file

@ -9,15 +9,12 @@ class SplitTestCache extends CacheLoader {
}) })
} }
async load(name) { async load() {
Metrics.inc('split_test_get_split_test_from_mongo', 1, { Metrics.inc('split_test_get_split_test_from_mongo', 1, {})
path: name, const splitTests = await SplitTestManager.getRuntimeTests()
}) return new Map(
const splitTest = await SplitTestManager.getSplitTest({ splitTests.map(splitTest => [splitTest.name, splitTest.toObject()])
name, )
archived: { $ne: true },
})
return splitTest?.toObject()
} }
serialize(value) { serialize(value) {

View file

@ -225,7 +225,7 @@ async function sessionMaintenance(req, user) {
* @private * @private
*/ */
async function _getVariantNames(splitTestName) { async function _getVariantNames(splitTestName) {
const splitTest = await SplitTestCache.get(splitTestName) const splitTest = await _getSplitTest(splitTestName)
const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
if (currentVersion?.active) { if (currentVersion?.active) {
return currentVersion.variants.map(v => v.name).concat([DEFAULT_VARIANT]) return currentVersion.variants.map(v => v.name).concat([DEFAULT_VARIANT])
@ -242,7 +242,7 @@ async function _getAssignment(
return DEFAULT_ASSIGNMENT return DEFAULT_ASSIGNMENT
} }
const splitTest = await SplitTestCache.get(splitTestName) const splitTest = await _getSplitTest(splitTestName)
const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
if (!currentVersion?.active) { if (!currentVersion?.active) {
return DEFAULT_ASSIGNMENT return DEFAULT_ASSIGNMENT
@ -493,9 +493,14 @@ async function _getUser(id, splitTestName) {
} }
async function _loadSplitTestInfoInLocals(locals, splitTestName) { async function _loadSplitTestInfoInLocals(locals, splitTestName) {
const splitTest = await SplitTestCache.get(splitTestName) const splitTest = await _getSplitTest(splitTestName)
if (splitTest) { if (splitTest) {
const phase = SplitTestUtils.getCurrentVersion(splitTest).phase const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
if (!currentVersion.active) {
return
}
const phase = currentVersion.phase
LocalsHelper.setSplitTestInfo(locals, splitTestName, { LocalsHelper.setSplitTestInfo(locals, splitTestName, {
phase, phase,
badgeInfo: splitTest.badgeInfo?.[phase], badgeInfo: splitTest.badgeInfo?.[phase],
@ -538,6 +543,11 @@ function _collectSessionStats(session) {
} }
} }
async function _getSplitTest(name) {
const splitTests = await SplitTestCache.get('')
return splitTests?.get(name)
}
module.exports = { module.exports = {
getPercentile, getPercentile,
getAssignment: callbackify(getAssignment), getAssignment: callbackify(getAssignment),

View file

@ -57,6 +57,16 @@ async function getSplitTests({ name, phase, type, active, archived }) {
} }
} }
async function getRuntimeTests() {
try {
return await SplitTest.find({
archived: { $ne: true },
}).exec()
} catch (error) {
throw OError.tag(error, 'Failed to get active split tests list')
}
}
async function getSplitTest(query) { async function getSplitTest(query) {
try { try {
return await SplitTest.findOne(query) return await SplitTest.findOne(query)
@ -439,6 +449,7 @@ function _mergeFlags(incomingTests, baseTests) {
module.exports = { module.exports = {
getSplitTest, getSplitTest,
getSplitTests, getSplitTests,
getRuntimeTests,
createSplitTest, createSplitTest,
updateSplitTestConfig, updateSplitTestConfig,
updateSplitTestInfo, updateSplitTestInfo,

View file

@ -22,6 +22,10 @@ describe('SplitTestHandler', function () {
versionNumber: 2, versionNumber: 2,
}), }),
] ]
this.cachedSplitTests = new Map()
for (const splitTest of this.splitTests) {
this.cachedSplitTests.set(splitTest.name, splitTest)
}
this.UserGetter = { this.UserGetter = {
promises: { promises: {
@ -36,11 +40,9 @@ describe('SplitTestHandler', function () {
} }
this.SplitTestCache = { this.SplitTestCache = {
get: sinon.stub().resolves(null), get: sinon.stub().resolves({}),
}
for (const splitTest of this.splitTests) {
this.SplitTestCache.get.withArgs(splitTest.name).resolves(splitTest)
} }
this.SplitTestCache.get.resolves(this.cachedSplitTests)
this.Settings = { this.Settings = {
moduleImportSequence: [], moduleImportSequence: [],
overleaf: {}, overleaf: {},
@ -203,22 +205,29 @@ describe('SplitTestHandler', function () {
this.AnalyticsManager.getIdsFromSession.returns({ this.AnalyticsManager.getIdsFromSession.returns({
userId: 'abc123abc123', userId: 'abc123abc123',
}) })
this.SplitTestCache.get.returns({ this.SplitTestCache.get.resolves(
name: 'my-test-name', new Map([
versions: [ [
{ 'my-test-name',
versionNumber: 0, {
active: true, name: 'my-test-name',
variants: [ versions: [
{ {
name: '100-percent-variant', versionNumber: 0,
rolloutPercent: 100, active: true,
rolloutStripes: [{ start: 0, end: 100 }], variants: [
}, {
], name: '100-percent-variant',
}, rolloutPercent: 100,
], rolloutStripes: [{ start: 0, end: 100 }],
}) },
],
},
],
},
],
])
)
const assignment = await this.SplitTestHandler.promises.getAssignment( const assignment = await this.SplitTestHandler.promises.getAssignment(
this.req, this.req,