Skip to content

Commit 89d25fa

Browse files
chriszaratechriszaratemaxschmeling
authored andcommitted
Restore with compaction update (#76872)
Co-authored-by: chriszarate <czarate@git.wordpress.org> Co-authored-by: maxschmeling <maxschmeling@git.wordpress.org>
1 parent 6230e95 commit 89d25fa

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

‎packages/sync/src/providers/http-polling/polling-manager.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,14 +505,30 @@ function poll(): void {
505505
MAX_ERROR_BACKOFF_IN_MS
506506
);
507507

508-
// Restore updates to queues on failure so they can be retried.
508+
// Recover from the failed request. We don't know whether the server stored
509+
// our updates before the error occurred (e.g. a network timeout after a
510+
// successful write). Re-sending the same updates via restore() would
511+
// duplicate them on the server and cause unbounded storage growth.
512+
//
513+
// Instead, for rooms that had outgoing updates, replace the queue with a
514+
// single compaction (full document state). This is idempotent: if the
515+
// server already stored the updates, the compaction safely supersedes
516+
// them; if it didn't, the compaction includes them. Updates not seen by
517+
// this client are preserved in both cases.
509518
for ( const room of payload.rooms ) {
510519
if ( ! roomStates.has( room.room ) ) {
511520
continue;
512521
}
513522

514523
const state = roomStates.get( room.room )!;
515-
state.updateQueue.restore( room.updates );
524+
525+
if ( room.updates.length > 0 && state.endCursor > 0 ) {
526+
state.updateQueue.clear();
527+
state.updateQueue.add( state.createCompactionUpdate() );
528+
} else if ( room.updates.length > 0 ) {
529+
state.updateQueue.restore( room.updates );
530+
}
531+
516532
state.log(
517533
'Error posting sync update, will retry with backoff',
518534
{

‎packages/sync/src/providers/http-polling/test/polling-manager.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,123 @@ describe( 'polling-manager', () => {
803803
} );
804804
} );
805805

806+
describe( 'error recovery', () => {
807+
it( 'replaces queued updates with a compaction after a poll error', async () => {
808+
// First poll: succeed with collaborators to resume the queue.
809+
const responseWithCollaborator = {
810+
rooms: [
811+
{
812+
room: 'test-room',
813+
end_cursor: 1,
814+
awareness: { 1: {}, 2: {} },
815+
updates: [],
816+
},
817+
],
818+
};
819+
mockPostSyncUpdate.mockResolvedValueOnce(
820+
responseWithCollaborator
821+
);
822+
823+
const doc = createMockDoc( 1 );
824+
pollingManager.registerRoom( {
825+
room: 'test-room',
826+
doc,
827+
awareness: createMockAwareness(),
828+
log: jest.fn(),
829+
onStatusChange: jest.fn(),
830+
onSync: jest.fn(),
831+
} );
832+
833+
// Flush the initial poll (queue is paused, so no updates sent).
834+
await jest.advanceTimersByTimeAsync( 0 );
835+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 1 );
836+
837+
// Add a document update (queue is now resumed due to collaborators).
838+
const onDocUpdate = getOnDocUpdate( doc );
839+
onDocUpdate( new Uint8Array( [ 1, 2, 3 ] ), 'user' );
840+
841+
// Second poll: fail with a network error.
842+
mockPostSyncUpdate.mockRejectedValueOnce( new Error( 'timeout' ) );
843+
await jest.advanceTimersByTimeAsync( 1000 );
844+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 2 );
845+
846+
// Verify the second poll included the queued updates (sync_step1 + doc update).
847+
const secondCallPayload = mockPostSyncUpdate.mock
848+
.calls[ 1 ][ 0 ] as {
849+
rooms: Array< {
850+
updates: Array< { type: string } >;
851+
} >;
852+
};
853+
expect(
854+
secondCallPayload.rooms[ 0 ].updates.length
855+
).toBeGreaterThan( 0 );
856+
857+
// Third poll: succeed — verify it sends a compaction instead of
858+
// restoring the same updates.
859+
mockPostSyncUpdate.mockResolvedValueOnce(
860+
responseWithCollaborator
861+
);
862+
863+
// Error handler doubled the interval: min(1000 * 2, 30000) = 2000.
864+
await jest.advanceTimersByTimeAsync( 2000 );
865+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 3 );
866+
867+
const thirdCallPayload = mockPostSyncUpdate.mock
868+
.calls[ 2 ][ 0 ] as {
869+
rooms: Array< {
870+
updates: Array< { type: string } >;
871+
} >;
872+
};
873+
const retryUpdates = thirdCallPayload.rooms[ 0 ].updates;
874+
expect( retryUpdates ).toHaveLength( 1 );
875+
expect( retryUpdates[ 0 ].type ).toBe( 'compaction' );
876+
} );
877+
878+
it( 'does not queue a compaction for rooms with no outgoing updates', async () => {
879+
// First poll succeeds (no collaborators, queue stays paused).
880+
mockPostSyncUpdate.mockResolvedValueOnce( syncResponse );
881+
882+
pollingManager.registerRoom( {
883+
room: 'test-room',
884+
doc: createMockDoc( 1 ),
885+
awareness: createMockAwareness(),
886+
log: jest.fn(),
887+
onStatusChange: jest.fn(),
888+
onSync: jest.fn(),
889+
} );
890+
891+
await jest.advanceTimersByTimeAsync( 0 );
892+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 1 );
893+
894+
// Second poll: fail (no updates were sent because queue is paused).
895+
mockPostSyncUpdate.mockRejectedValueOnce( new Error( 'timeout' ) );
896+
await jest.advanceTimersByTimeAsync( 4000 );
897+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 2 );
898+
899+
// Verify no updates were sent on the failed poll.
900+
const secondCallPayload = mockPostSyncUpdate.mock
901+
.calls[ 1 ][ 0 ] as {
902+
rooms: Array< {
903+
updates: Array< { type: string } >;
904+
} >;
905+
};
906+
expect( secondCallPayload.rooms[ 0 ].updates ).toHaveLength( 0 );
907+
908+
// Third poll: succeed — should still have no updates (no compaction queued).
909+
mockPostSyncUpdate.mockResolvedValueOnce( syncResponse );
910+
await jest.advanceTimersByTimeAsync( 8000 );
911+
expect( mockPostSyncUpdate ).toHaveBeenCalledTimes( 3 );
912+
913+
const thirdCallPayload = mockPostSyncUpdate.mock
914+
.calls[ 2 ][ 0 ] as {
915+
rooms: Array< {
916+
updates: Array< { type: string } >;
917+
} >;
918+
};
919+
expect( thirdCallPayload.rooms[ 0 ].updates ).toHaveLength( 0 );
920+
} );
921+
} );
922+
806923
describe( 'visibility change', () => {
807924
it( 'does not spawn a duplicate poll when a request is in-flight', () => {
808925
// Keep the first postSyncUpdate pending so we can simulate

0 commit comments

Comments
 (0)