Merge pull request #24966 from overleaf/bg-history-buffer-optimised-get

add getCurrentChunkIfValid function

GitOrigin-RevId: e947a99ac928b58048a87cea0be1da34fcf3a9f8

services/history-v1/storage/lib/chunk_buffer/index.js

@@ -14,13 +14,12 @@ const metrics = require('@overleaf/metrics')
  * @return {Promise.<Chunk>}
  */
 async function loadLatest(projectId) {
-  const cachedChunk = await redisBackend.getCurrentChunk(projectId)
   const chunkRecord = await chunkStore.loadLatestRaw(projectId)
-  const cachedChunkIsValid = redisBackend.checkCacheValidityWithMetadata(
-    cachedChunk,
+  const cachedChunk = await redisBackend.getCurrentChunkIfValid(
+    projectId,
     chunkRecord
   )
-  if (cachedChunkIsValid) {
+  if (cachedChunk) {
     metrics.inc('chunk_buffer.loadLatest', 1, {
       status: 'cache-hit',
     })

services/history-v1/storage/lib/chunk_store/redis.js

@@ -62,6 +62,62 @@ async function getCurrentChunk(projectId) {
   }
 }
 
+rclient.defineCommand('get_current_chunk_if_valid', {
+  numberOfKeys: 3,
+  lua: `
+      local expectedStartVersion = ARGV[1]
+      local expectedChangesCount = tonumber(ARGV[2])
+      local startVersionValue = redis.call('GET', KEYS[2])
+      if not startVersionValue then
+        return nil -- this is a cache-miss
+      end
+      if startVersionValue ~= expectedStartVersion then
+        return nil -- this is a cache-miss
+      end
+      local changesCount = redis.call('LLEN', KEYS[3])
+      if changesCount ~= expectedChangesCount then
+        return nil -- this is a cache-miss
+      end
+      local snapshotValue = redis.call('GET', KEYS[1])
+      local changesValues = redis.call('LRANGE', KEYS[3], 0, -1)
+      return {snapshotValue, startVersionValue, changesValues}
+    `,
+})
+
+async function getCurrentChunkIfValid(projectId, chunkRecord) {
+  try {
+    const changesCount = chunkRecord.endVersion - chunkRecord.startVersion
+    const result = await rclient.get_current_chunk_if_valid(
+      keySchema.snapshot({ projectId }),
+      keySchema.startVersion({ projectId }),
+      keySchema.changes({ projectId }),
+      chunkRecord.startVersion,
+      changesCount
+    )
+    if (!result) {
+      return null // cache-miss
+    }
+    const snapshot = Snapshot.fromRaw(JSON.parse(result[0]))
+    const startVersion = parseInt(result[1], 10)
+    const changes = result[2].map(c => Change.fromRaw(JSON.parse(c)))
+    const history = new History(snapshot, changes)
+    const chunk = new Chunk(history, startVersion)
+    metrics.inc('chunk_store.redis.get_current_chunk_if_valid', 1, {
+      status: 'success',
+    })
+    return chunk
+  } catch (err) {
+    logger.error(
+      { err, projectId, chunkRecord },
+      'error getting current chunk from redis'
+    )
+    metrics.inc('chunk_store.redis.get_current_chunk_if_valid', 1, {
+      status: 'error',
+    })
+    return null
+  }
+}
+
 rclient.defineCommand('get_current_chunk_metadata', {
   numberOfKeys: 2,
   lua: `
@@ -245,6 +301,7 @@ async function clearCache(projectId) {
 
 module.exports = {
   getCurrentChunk,
+  getCurrentChunkIfValid,
   setCurrentChunk,
   getCurrentChunkMetadata,
   checkCacheValidity,

services/history-v1/test/acceptance/js/storage/chunk_store_redis_backend.test.js

@@ -603,4 +603,132 @@ describe('chunk store Redis backend', function () {
       expect(chunkMetadata.changesCount).to.equal(3)
     })
   })
+
+  describe('getCurrentChunkIfValid', function () {
+    it('should return the chunk when versions and changes count match', async function () {
+      // Create and cache a sample chunk
+      const snapshot = new Snapshot()
+      const changes = [
+        new Change(
+          [new AddFileOperation('test.tex', File.fromString('Valid content'))],
+          new Date(),
+          []
+        ),
+      ]
+      const history = new History(snapshot, changes)
+      const chunk = new Chunk(history, 7) // startVersion 7, endVersion 8
+      await redisBackend.setCurrentChunk(projectId, chunk)
+
+      // Prepare chunkRecord matching the cached chunk
+      const chunkRecord = { startVersion: 7, endVersion: 8 }
+
+      // Retrieve using getCurrentChunkIfValid
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+
+      expect(validChunk).to.not.be.null
+      expect(validChunk.getStartVersion()).to.equal(7)
+      expect(validChunk.getEndVersion()).to.equal(8)
+      expect(validChunk).to.deep.equal(chunk)
+    })
+
+    it('should return null when no chunk is cached', async function () {
+      // No chunk is cached for this projectId yet
+      const chunkRecord = { startVersion: 1, endVersion: 2 }
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+      expect(validChunk).to.be.null
+    })
+
+    it('should return null when start version mismatches', async function () {
+      // Cache a chunk with startVersion 10
+      const snapshot = new Snapshot()
+      const changes = [
+        new Change(
+          [new AddFileOperation('test.tex', File.fromString('Content'))],
+          new Date(),
+          []
+        ),
+      ]
+      const history = new History(snapshot, changes)
+      const chunk = new Chunk(history, 10) // startVersion 10, endVersion 11
+      await redisBackend.setCurrentChunk(projectId, chunk)
+
+      // Attempt to retrieve with a different startVersion
+      const chunkRecord = { startVersion: 9, endVersion: 10 } // Incorrect startVersion
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+      expect(validChunk).to.be.null
+    })
+
+    it('should return null when changes count mismatches', async function () {
+      // Cache a chunk with one change (startVersion 15, endVersion 16)
+      const snapshot = new Snapshot()
+      const changes = [
+        new Change(
+          [new AddFileOperation('test.tex', File.fromString('Content'))],
+          new Date(),
+          []
+        ),
+      ]
+      const history = new History(snapshot, changes)
+      const chunk = new Chunk(history, 15)
+      await redisBackend.setCurrentChunk(projectId, chunk)
+
+      // Attempt to retrieve with correct startVersion but incorrect endVersion (implying wrong changes count)
+      const chunkRecord = { startVersion: 15, endVersion: 17 } // Incorrect endVersion (implies 2 changes)
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+      expect(validChunk).to.be.null
+    })
+
+    it('should return the chunk when versions and changes count match for a zero-change chunk', async function () {
+      // Cache a chunk with zero changes
+      const snapshot = new Snapshot()
+      const changes = []
+      const history = new History(snapshot, changes)
+      const chunk = new Chunk(history, 20) // startVersion 20, endVersion 20
+      await redisBackend.setCurrentChunk(projectId, chunk)
+
+      // Prepare chunkRecord matching the zero-change chunk
+      const chunkRecord = { startVersion: 20, endVersion: 20 }
+
+      // Retrieve using getCurrentChunkIfValid
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+
+      expect(validChunk).to.not.be.null
+      expect(validChunk.getStartVersion()).to.equal(20)
+      expect(validChunk.getEndVersion()).to.equal(20)
+      expect(validChunk.history.changes.length).to.equal(0)
+      expect(validChunk).to.deep.equal(chunk)
+    })
+
+    it('should return null when start version matches but changes count is wrong for zero-change chunk', async function () {
+      // Cache a chunk with zero changes
+      const snapshot = new Snapshot()
+      const changes = []
+      const history = new History(snapshot, changes)
+      const chunk = new Chunk(history, 25) // startVersion 25, endVersion 25
+      await redisBackend.setCurrentChunk(projectId, chunk)
+
+      // Attempt to retrieve with correct startVersion but incorrect endVersion
+      const chunkRecord = { startVersion: 25, endVersion: 26 } // Incorrect endVersion (implies 1 change)
+      const validChunk = await redisBackend.getCurrentChunkIfValid(
+        projectId,
+        chunkRecord
+      )
+      expect(validChunk).to.be.null
+    })
+  })
 })