From 2a833aa23a007eaa9dbb3ab2d667a11999f5f800 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 10 Jun 2025 10:42:28 +0100 Subject: [PATCH] Merge pull request #26250 from overleaf/bg-history-redis-add-return-value-to-persistBuffer provide return value from persistBuffer GitOrigin-RevId: ba52ff42b91ffe9adc23ab0461fa836540735563 --- .../history-v1/storage/lib/persist_buffer.js | 14 ++- .../js/storage/persist_buffer.test.mjs | 96 +++++++++++++++++-- 2 files changed, 101 insertions(+), 9 deletions(-) diff --git a/services/history-v1/storage/lib/persist_buffer.js b/services/history-v1/storage/lib/persist_buffer.js index 1f508c43f3..9534e5834a 100644 --- a/services/history-v1/storage/lib/persist_buffer.js +++ b/services/history-v1/storage/lib/persist_buffer.js @@ -58,7 +58,17 @@ async function persistBuffer(projectId, limits) { // to match the current endVersion. This shouldn't be needed // unless a worker failed to update the persisted version. await redisBackend.setPersistedVersion(projectId, endVersion) - return + const { chunk } = await chunkStore.loadByChunkRecord( + projectId, + latestChunkMetadata + ) + // Return the result in the same format as persistChanges + // so that the caller can handle it uniformly. + return { + numberOfChangesPersisted: changesToPersist.length, + originalEndVersion: endVersion, + currentChunk: chunk, + } } logger.debug( @@ -160,6 +170,8 @@ async function persistBuffer(projectId, limits) { { projectId, finalPersistedVersion: newEndVersion }, 'persistBuffer operation completed successfully' ) + + return persistResult } module.exports = persistBuffer diff --git a/services/history-v1/test/acceptance/js/storage/persist_buffer.test.mjs b/services/history-v1/test/acceptance/js/storage/persist_buffer.test.mjs index 216399f676..138a70e626 100644 --- a/services/history-v1/test/acceptance/js/storage/persist_buffer.test.mjs +++ b/services/history-v1/test/acceptance/js/storage/persist_buffer.test.mjs @@ -92,15 +92,34 @@ describe('persistBuffer', function () { await redisBackend.setPersistedVersion(projectId, initialVersion) // Persist the changes from Redis to the chunk store - await persistBuffer(projectId, limitsToPersistImmediately) + const persistResult = await persistBuffer( + projectId, + limitsToPersistImmediately + ) - const latestChunk = await chunkStore.loadLatest(projectId) + // Check the return value of persistBuffer + expect(persistResult).to.exist + expect(persistResult).to.have.property('numberOfChangesPersisted') + expect(persistResult).to.have.property('originalEndVersion') + expect(persistResult).to.have.property('currentChunk') + expect(persistResult).to.have.property('resyncNeeded') + expect(persistResult.numberOfChangesPersisted).to.equal( + changesToQueue.length + ) + expect(persistResult.originalEndVersion).to.equal(initialVersion + 1) + expect(persistResult.resyncNeeded).to.be.false + + const latestChunk = await chunkStore.loadLatest(projectId, { + persistedOnly: true, + }) expect(latestChunk).to.exist expect(latestChunk.getStartVersion()).to.equal(initialVersion) expect(latestChunk.getEndVersion()).to.equal(finalHeadVersion) expect(latestChunk.getChanges().length).to.equal( changesToQueue.length + 1 ) + // Check that chunk returned by persistBuffer matches the latest chunk + expect(latestChunk).to.deep.equal(persistResult.currentChunk) const chunkSnapshot = latestChunk.getSnapshot() expect(Object.keys(chunkSnapshot.getFileMap()).length).to.equal(1) @@ -196,9 +215,28 @@ describe('persistBuffer', function () { persistedChunkEndVersion ) - await persistBuffer(projectId, limitsToPersistImmediately) + const persistResult = await persistBuffer( + projectId, + limitsToPersistImmediately + ) - const latestChunk = await chunkStore.loadLatest(projectId) + // Check the return value of persistBuffer + expect(persistResult).to.exist + expect(persistResult).to.have.property('numberOfChangesPersisted') + expect(persistResult).to.have.property('originalEndVersion') + expect(persistResult).to.have.property('currentChunk') + expect(persistResult).to.have.property('resyncNeeded') + expect(persistResult.numberOfChangesPersisted).to.equal( + redisChangesToPush.length + ) + expect(persistResult.originalEndVersion).to.equal( + persistedChunkEndVersion + ) + expect(persistResult.resyncNeeded).to.be.false + + const latestChunk = await chunkStore.loadLatest(projectId, { + persistedOnly: true, + }) expect(latestChunk).to.exist expect(latestChunk.getStartVersion()).to.equal(0) expect(latestChunk.getEndVersion()).to.equal( @@ -215,6 +253,9 @@ describe('persistBuffer', function () { finalHeadVersionAfterRedisPush ) + // Check that chunk returned by persistBuffer matches the latest chunk + expect(persistResult.currentChunk).to.deep.equal(latestChunk) + const nonPersisted = await redisBackend.getNonPersistedChanges( projectId, finalHeadVersionAfterRedisPush @@ -287,8 +328,19 @@ describe('persistBuffer', function () { const chunksBefore = await chunkStore.getProjectChunks(projectId) - // Persist buffer (which should do nothing as there are no new changes) - await persistBuffer(projectId, limitsToPersistImmediately) + const persistResult = await persistBuffer( + projectId, + limitsToPersistImmediately + ) + + const currentChunk = await chunkStore.loadLatest(projectId, { + persistedOnly: true, + }) + expect(persistResult).to.deep.equal({ + numberOfChangesPersisted: 0, + originalEndVersion: persistedChunkEndVersion, + currentChunk, + }) const chunksAfter = await chunkStore.getProjectChunks(projectId) expect(chunksAfter.length).to.equal(chunksBefore.length) @@ -324,7 +376,20 @@ describe('persistBuffer', function () { const chunksBefore = await chunkStore.getProjectChunks(projectId) // Persist buffer (which should do nothing as there are no new changes) - await persistBuffer(projectId, limitsToPersistImmediately) + const persistResult = await persistBuffer( + projectId, + limitsToPersistImmediately + ) + + // Check the return value + const currentChunk = await chunkStore.loadLatest(projectId, { + persistedOnly: true, + }) + expect(persistResult).to.deep.equal({ + numberOfChangesPersisted: 0, + originalEndVersion: persistedChunkEndVersion, + currentChunk, + }) const chunksAfter = await chunkStore.getProjectChunks(projectId) expect(chunksAfter.length).to.equal(chunksBefore.length) @@ -411,7 +476,19 @@ describe('persistBuffer', function () { maxChangeTimestamp: new Date(twoHoursAgo), // they will be persisted if any change is older than 2 hours } - await persistBuffer(projectId, restrictiveLimits) + const persistResult = await persistBuffer(projectId, restrictiveLimits) + + // Check the return value of persistBuffer + expect(persistResult).to.exist + expect(persistResult).to.have.property('numberOfChangesPersisted') + expect(persistResult).to.have.property('originalEndVersion') + expect(persistResult).to.have.property('currentChunk') + expect(persistResult).to.have.property('resyncNeeded') + expect(persistResult.numberOfChangesPersisted).to.equal(2) // change1 + change2 + expect(persistResult.originalEndVersion).to.equal( + versionAfterInitialSetup + ) + expect(persistResult.resyncNeeded).to.be.false // Check the latest persisted chunk, it should only have the initial file and the first two changes const latestChunk = await chunkStore.loadLatest(projectId, { @@ -423,6 +500,9 @@ describe('persistBuffer', function () { const expectedEndVersion = versionAfterInitialSetup + 2 // Persisted two changes from the queue expect(latestChunk.getEndVersion()).to.equal(expectedEndVersion) + // Check that chunk returned by persistBuffer matches the latest chunk + expect(persistResult.currentChunk).to.deep.equal(latestChunk) + // Check persisted version in Redis const state = await redisBackend.getState(projectId) expect(state.persistedVersion).to.equal(expectedEndVersion)