Merge pull request #28377 from overleaf/bg-inefficient-query-during-history-backup

Fix inefficient query during history backup verification

GitOrigin-RevId: c33246dca2436e82056506a2dceb79c465cd8301

services/history-v1/storage/lib/backupVerifier.mjs

@@ -147,31 +147,40 @@ export async function loadChunk(
 export async function verifyProject(historyId, endTimestamp) {
   const backend = chunkStore.getBackend(historyId)
   const [first, last] = await Promise.all([
-    backend.getFirstChunkBeforeTimestamp(historyId, endTimestamp),
-    backend.getLastActiveChunkBeforeTimestamp(historyId, endTimestamp),
+    backend.getChunkForVersion(historyId, 0),
+    backend.getChunkForTimestamp(historyId, endTimestamp),
   ])
 
   const chunksRecordsToVerify = [
     {
       chunkId: first.id,
       chunkLabel: 'first',
+      ...first,
     },
   ]
   if (first.startVersion !== last.startVersion) {
     chunksRecordsToVerify.push({
       chunkId: last.id,
       chunkLabel: 'last before RPO',
+      ...last,
     })
   }
 
   const projectCache = await getProjectPersistor(historyId)
-
   const chunks = await Promise.all(
     chunksRecordsToVerify.map(async chunk => {
       try {
-        return History.fromRaw(
-          await loadChunk(historyId, chunk.startVersion, projectCache)
+        const chunkContents = await loadChunk(
+          historyId,
+          chunk.startVersion,
+          projectCache
+        )
+        // filter the raw changes to only those that are <= endTimestamp
+        // to simulate the state of the project at endTimestamp
+        chunkContents.changes = chunkContents.changes.filter(
+          change => new Date(change.timestamp) <= endTimestamp
         )
+        return History.fromRaw(chunkContents)
       } catch (err) {
         if (err instanceof Chunk.NotPersistedError) {
           throw new BackupRPOViolationChunkNotBackedUpError(

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

@@ -70,35 +70,6 @@ async function getChunkForVersion(projectId, version, opts = {}) {
   return chunkFromRecord(record)
 }
 
-/**
- * Get the metadata for the chunk that contains the given version before the endTime.
- */
-async function getFirstChunkBeforeTimestamp(projectId, timestamp) {
-  assert.mongoId(projectId, 'bad projectId')
-  assert.date(timestamp, 'bad timestamp')
-
-  const recordActive = await getChunkForVersion(projectId, 0)
-  if (recordActive && recordActive.endTimestamp <= timestamp) {
-    return recordActive
-  }
-
-  // fallback to deleted chunk
-  const recordDeleted = await mongodb.chunks.findOne(
-    {
-      projectId: new ObjectId(projectId),
-      state: 'deleted',
-      startVersion: 0,
-      updatedAt: { $lte: timestamp }, // indexed for state=deleted
-      endTimestamp: { $lte: timestamp },
-    },
-    { sort: { updatedAt: -1 } }
-  )
-  if (recordDeleted) {
-    return chunkFromRecord(recordDeleted)
-  }
-  throw new Chunk.BeforeTimestampNotFoundError(projectId, timestamp)
-}
-
 /**
  * Get the metadata for the chunk that contains the version that was current at
  * the given timestamp.
@@ -131,39 +102,6 @@ async function getChunkForTimestamp(projectId, timestamp) {
   return chunkFromRecord(record)
 }
 
-/**
- * Get the metadata for the chunk that contains the version that was current before
- * the given timestamp.
- */
-async function getLastActiveChunkBeforeTimestamp(projectId, timestamp) {
-  assert.mongoId(projectId, 'bad projectId')
-  assert.date(timestamp, 'bad timestamp')
-
-  const record = await mongodb.chunks.findOne(
-    {
-      projectId: new ObjectId(projectId),
-      state: { $in: ['active', 'closed'] },
-      $or: [
-        {
-          endTimestamp: {
-            $lte: timestamp,
-          },
-        },
-        {
-          endTimestamp: null,
-        },
-      ],
-    },
-    // We use the index on the startVersion for sorting records. This assumes
-    // that timestamps go up with each version.
-    { sort: { startVersion: -1 } }
-  )
-  if (record == null) {
-    throw new Chunk.BeforeTimestampNotFoundError(projectId, timestamp)
-  }
-  return chunkFromRecord(record)
-}
-
 /**
  * Get all of a project's chunk ids
  */
@@ -540,8 +478,6 @@ function chunkFromRecord(record) {
 
 module.exports = {
   getLatestChunk,
-  getFirstChunkBeforeTimestamp,
-  getLastActiveChunkBeforeTimestamp,
   getChunkForVersion,
   getChunkForTimestamp,
   getProjectChunkIds,

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

@@ -60,64 +60,6 @@ async function getChunkForVersion(projectId, version, opts = {}) {
   return chunkFromRecord(record)
 }
 
-/**
- * Get the metadata for the chunk that contains the given version.
- *
- * @param {string} projectId
- * @param {Date} timestamp
- */
-async function getFirstChunkBeforeTimestamp(projectId, timestamp) {
-  assert.date(timestamp, 'bad timestamp')
-
-  const recordActive = await getChunkForVersion(projectId, 0)
-
-  // projectId must be valid if getChunkForVersion did not throw
-  if (recordActive && recordActive.endTimestamp <= timestamp) {
-    return recordActive
-  }
-
-  // fallback to deleted chunk
-  const recordDeleted = await knex('old_chunks')
-    .where('doc_id', parseInt(projectId, 10))
-    .where('start_version', '=', 0)
-    .where('end_timestamp', '<=', timestamp)
-    .orderBy('end_version', 'desc')
-    .first()
-  if (recordDeleted) {
-    return chunkFromRecord(recordDeleted)
-  }
-  throw new Chunk.BeforeTimestampNotFoundError(projectId, timestamp)
-}
-
-/**
- * Get the metadata for the chunk that contains the version that was current at
- * the given timestamp.
- *
- * @param {string} projectId
- * @param {Date} timestamp
- */
-async function getLastActiveChunkBeforeTimestamp(projectId, timestamp) {
-  assert.date(timestamp, 'bad timestamp')
-  assert.postgresId(projectId, 'bad projectId')
-
-  const query = knex('chunks')
-    .where('doc_id', parseInt(projectId, 10))
-    .where(function () {
-      this.where('end_timestamp', '<=', timestamp).orWhere(
-        'end_timestamp',
-        null
-      )
-    })
-    .orderBy('end_version', 'desc', 'last')
-
-  const record = await query.first()
-
-  if (!record) {
-    throw new Chunk.BeforeTimestampNotFoundError(projectId, timestamp)
-  }
-  return chunkFromRecord(record)
-}
-
 /**
  * Get the metadata for the chunk that contains the version that was current at
  * the given timestamp.
@@ -481,8 +423,6 @@ async function resolveHistoryIdToMongoProjectId(projectId) {
 
 module.exports = {
   getLatestChunk,
-  getFirstChunkBeforeTimestamp,
-  getLastActiveChunkBeforeTimestamp,
   getChunkForVersion,
   getChunkForTimestamp,
   getProjectChunkIds,

services/history-v1/test/acceptance/js/api/backupVerifier.test.mjs

@@ -19,7 +19,14 @@ import { promisify } from 'node:util'
 import { execFile } from 'node:child_process'
 import { NotFoundError } from '@overleaf/object-persistor/src/Errors.js'
 import { chunkStore } from '../../../../storage/index.js'
-import { Change, File, Operation } from 'overleaf-editor-core'
+import {
+  Change,
+  File,
+  Operation,
+  Snapshot,
+  History,
+  Chunk,
+} from 'overleaf-editor-core'
 import Crypto from 'node:crypto'
 import path from 'node:path'
 import projectKey from '../../../../storage/lib/project_key.js'
@@ -150,10 +157,7 @@ async function addFileInNewChunk(
   { creationDate = new Date() }
 ) {
   const chunk = await chunkStore.loadLatest(historyId, { persistedOnly: true })
-  const operation = Operation.addFile(
-    `${historyId}.txt`,
-    File.fromString(fileContents)
-  )
+  const operation = Operation.addFile(filePath, File.fromString(fileContents))
   const changes = [new Change([operation], creationDate, [])]
   chunk.pushChanges(changes)
   await chunkStore.update(historyId, chunk)
@@ -166,37 +170,86 @@ async function addFileInNewChunk(
  */
 async function prepareProjectAndBlob(
   historyId,
-  { shouldBackupBlob, shouldBackupChunk, shouldCreateChunk } = {
-    shouldBackupBlob: true,
-    shouldBackupChunk: true,
-    shouldCreateChunk: true,
-  }
+  {
+    shouldBackupBlob = true,
+    shouldBackupChunk = true,
+    shouldCreateChunk = true,
+    extraChunks = 0, // number of additional chunks to create after the first one
+    skipBackupForLastChunk = false, // when true, do not back up the last created chunk (simulates missing last chunk backup)
+  } = {}
 ) {
   await testProjects.createEmptyProject(historyId)
   const blobStore = new BlobStore(historyId)
   const fileContents = historyId
-  const blob = await blobStore.putString(fileContents)
+  const initialBlob = await blobStore.putString(fileContents)
+  const now = Date.now()
+  const creationTime = now - FIFTEEN_MINUTES_IN_MS
+
+  // Create first (updated) chunk if requested
   if (shouldCreateChunk) {
     await addFileInNewChunk(fileContents, `${historyId}.txt`, historyId, {
-      creationDate: new Date(new Date().getTime() - FIFTEEN_MINUTES_IN_MS),
+      creationDate: new Date(creationTime),
     })
   }
 
+  // Backup the initial blob if requested
   if (shouldBackupBlob) {
-    const gzipped = zlib.gzipSync(Buffer.from(historyId))
+    const gzipped = zlib.gzipSync(Buffer.from(fileContents))
     await backupPersistor.sendStream(
       projectBlobsBucket,
-      makeProjectKey(historyId, blob.getHash()),
+      makeProjectKey(historyId, initialBlob.getHash()),
       Stream.Readable.from([gzipped]),
       { contentLength: gzipped.byteLength, contentEncoding: 'gzip' }
     )
     await checkDEKExists(historyId)
   }
+  // Backup first chunk if requested
   if (shouldCreateChunk && shouldBackupChunk) {
     await backupChunk(historyId)
   }
 
-  return blob.getHash()
+  for (let index = 0; index < extraChunks; index++) {
+    // Create an additional chunk starting at current endVersion
+    const latestMeta = await chunkStore.getLatestChunkMetadata(historyId)
+    const startVersion = latestMeta.endVersion
+    const snapshot = Snapshot.fromRaw({ files: {} })
+    const history = new History(snapshot, [])
+    const newChunk = new Chunk(history, startVersion)
+    const extraContent = `${fileContents}-extra-${index + 1}`
+    const extraBlob = await blobStore.putString(extraContent)
+    // ensure strictly increasing timestamps
+    const changeTimestamp = new Date(creationTime + (index + 1) * 60_000)
+    const change = new Change(
+      [
+        Operation.addFile(
+          `${historyId}-extra-${index + 1}.txt`,
+          File.createLazyFromBlobs(extraBlob)
+        ),
+      ],
+      changeTimestamp,
+      []
+    )
+    newChunk.pushChanges([change])
+    await chunkStore.create(historyId, newChunk)
+
+    // Backup blob for this chunk if requested
+    if (shouldBackupBlob) {
+      const gzipped = zlib.gzipSync(Buffer.from(extraContent))
+      await backupPersistor.sendStream(
+        projectBlobsBucket,
+        makeProjectKey(historyId, extraBlob.getHash()),
+        Stream.Readable.from([gzipped]),
+        { contentLength: gzipped.byteLength, contentEncoding: 'gzip' }
+      )
+    }
+    // Backup the chunk unless we're intentionally skipping the last one
+    const isLast = index === extraChunks - 1
+    if (shouldBackupChunk && (!isLast || !skipBackupForLastChunk)) {
+      await backupChunk(historyId)
+    }
+  }
+
+  return initialBlob.getHash()
 }
 
 /**
@@ -281,6 +334,50 @@ describe('backupVerifier', function () {
         )
       })
     })
+    describe('for a project with multiple chunks', function () {
+      const multiHistoryIdOk = '100'
+      const multiHistoryIdMissingLast = '101'
+      let okResponse
+      let missingLastResponse
+
+      describe('when multiple chunks are fully backed up', function () {
+        beforeEach(async function () {
+          await prepareProjectAndBlob(multiHistoryIdOk, {
+            shouldBackupBlob: true,
+            shouldBackupChunk: true,
+            shouldCreateChunk: true,
+            extraChunks: 2, // total 3 chunks including first
+          })
+          okResponse = await verifyProjectScript(multiHistoryIdOk, false)
+        })
+        it('returns 0', function () {
+          expect(okResponse.status).to.equal(0)
+        })
+      })
+
+      describe('when the last chunk backup is missing', function () {
+        beforeEach(async function () {
+          await prepareProjectAndBlob(multiHistoryIdMissingLast, {
+            shouldBackupBlob: true,
+            shouldBackupChunk: true,
+            shouldCreateChunk: true,
+            extraChunks: 2,
+            skipBackupForLastChunk: true, // simulate missing backup for last chunk
+          })
+          missingLastResponse = await verifyProjectScript(
+            multiHistoryIdMissingLast
+          )
+        })
+        it('returns 1', function () {
+          expect(missingLastResponse.status).to.equal(1)
+        })
+        it('emits a BackupRPOViolationChunkNotBackedUpError', function () {
+          expect(missingLastResponse.stderr).to.include(
+            'BackupRPOViolationChunkNotBackedUpError'
+          )
+        })
+      })
+    })
   })
   describe('storage/scripts/verify_backup_blob.mjs', function () {
     it('throws and does not create DEK if missing', async function () {