diff --git a/packages/services/src/Domain/Sync/SyncBackoffService.spec.ts b/packages/services/src/Domain/Sync/SyncBackoffService.spec.ts index 23ffe4a924e..5786de1ad41 100644 --- a/packages/services/src/Domain/Sync/SyncBackoffService.spec.ts +++ b/packages/services/src/Domain/Sync/SyncBackoffService.spec.ts @@ -21,7 +21,7 @@ describe('SyncBackoffService', () => { it('should be in backoff if backoff was set', () => { const service = createService() - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(true) }) @@ -31,7 +31,7 @@ describe('SyncBackoffService', () => { jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) jest.spyOn(Date, 'now').mockReturnValueOnce(2_000_000) @@ -43,19 +43,19 @@ describe('SyncBackoffService', () => { jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(true) jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) jest.spyOn(Date, 'now').mockReturnValueOnce(1_001_000) expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(true) jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) jest.spyOn(Date, 'now').mockReturnValueOnce(1_003_000) expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(true) @@ -66,7 +66,7 @@ describe('SyncBackoffService', () => { jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) + service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()]) jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(true) @@ -85,31 +85,25 @@ describe('SyncBackoffService', () => { expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked)).toBe(false) }) - it('should get a smaller subset of item uuids in backoff that have lesser penalty', () => { + it('should get a smaller and smaller subset of item uuids to back off until single uuids are ruled out', () => { const service = createService() - jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000) + jest.spyOn(Date, 'now').mockReturnValue(1_000_000) - for (let i = 0; i < 5; i++) { - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue()) - } - for (let i = 0; i < 4; i++) { - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000001').getValue()) - } - for (let i = 0; i < 3; i++) { - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000002').getValue()) - } - for (let i = 0; i < 2; i++) { - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000003').getValue()) - } - service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000004').getValue()) - - const subset = service.getSmallerSubsetOfItemUuidsInBackoff() + service.backoffItems([ + Uuid.create('00000000-0000-0000-0000-000000000000').getValue(), + Uuid.create('00000000-0000-0000-0000-000000000001').getValue(), + Uuid.create('00000000-0000-0000-0000-000000000002').getValue(), + Uuid.create('00000000-0000-0000-0000-000000000003').getValue(), + Uuid.create('00000000-0000-0000-0000-000000000004').getValue(), + ]) - expect(subset.length).toEqual(3) + const expectedSubsetLenghts = [3, 3, 3, 3, 2, 1, 1, 1, 1, 1, 0] + for (let i = 0; i < expectedSubsetLenghts.length; i++) { + const subset = service.getSmallerSubsetOfItemUuidsInBackoff() + service.backoffItems(subset) - expect(subset[0].value).toBe('00000000-0000-0000-0000-000000000004') - expect(subset[1].value).toBe('00000000-0000-0000-0000-000000000003') - expect(subset[2].value).toBe('00000000-0000-0000-0000-000000000002') + expect(subset.length).toBe(expectedSubsetLenghts[i]) + } }) }) diff --git a/packages/services/src/Domain/Sync/SyncBackoffService.ts b/packages/services/src/Domain/Sync/SyncBackoffService.ts index 72b848664c6..d0fdfb664fb 100644 --- a/packages/services/src/Domain/Sync/SyncBackoffService.ts +++ b/packages/services/src/Domain/Sync/SyncBackoffService.ts @@ -12,20 +12,33 @@ export class SyncBackoffService extends AbstractService implements SyncBackoffServiceInterface, InternalEventHandlerInterface { + private INITIAL_BACKOFF_PENALTY_MS = 1_000 + private SUBSET_BACKOFF_PENALTY_CUTOFF_MS = 10_000 + private backoffPenalties: Map private backoffStartTimestamps: Map + private itemUuidsThatHaveBeenTriedSolelyAsPayload: Set constructor(protected override internalEventBus: InternalEventBusInterface) { super(internalEventBus) this.backoffPenalties = new Map() this.backoffStartTimestamps = new Map() + this.itemUuidsThatHaveBeenTriedSolelyAsPayload = new Set() } getSmallerSubsetOfItemUuidsInBackoff(): Uuid[] { const uuids = Array.from(this.backoffPenalties.keys()) - const uuidsSortedByPenaltyAscending = uuids.sort((a, b) => { + const uuidsThatHaveNotBeenTriedAsSingleItemRequests = uuids.filter( + (uuid) => !this.itemUuidsThatHaveBeenTriedSolelyAsPayload.has(uuid), + ) + + if (uuidsThatHaveNotBeenTriedAsSingleItemRequests.length === 0) { + return [] + } + + const uuidsSortedByPenaltyAscending = uuidsThatHaveNotBeenTriedAsSingleItemRequests.sort((a, b) => { const penaltyA = this.backoffPenalties.get(a) || 0 const penaltyB = this.backoffPenalties.get(b) || 0 @@ -34,7 +47,19 @@ export class SyncBackoffService const halfLength = Math.ceil(uuidsSortedByPenaltyAscending.length / 2) - return uuidsSortedByPenaltyAscending.slice(0, halfLength).map((uuid) => Uuid.create(uuid).getValue()) + const halfOfUuidsSortedByPenaltyAscendingMeetingCutoff = [] + let penaltySum = 0 + for (let i = 0; i < halfLength; i++) { + const uuid = uuidsSortedByPenaltyAscending[i] + + penaltySum += this.backoffPenalties.get(uuid) || 0 + + if (penaltySum <= this.SUBSET_BACKOFF_PENALTY_CUTOFF_MS) { + halfOfUuidsSortedByPenaltyAscendingMeetingCutoff.push(uuid) + } + } + + return halfOfUuidsSortedByPenaltyAscendingMeetingCutoff.map((uuid) => Uuid.create(uuid).getValue()) } async handleEvent(event: InternalEventInterface): Promise { @@ -63,10 +88,20 @@ export class SyncBackoffService return backoffEndTimestamp > Date.now() } - backoffItem(itemUuid: Uuid): void { + backoffItems(itemUuids: Uuid[]): void { + for (const itemUuid of itemUuids) { + this.backoffItem(itemUuid) + } + + if (itemUuids.length === 1) { + this.itemUuidsThatHaveBeenTriedSolelyAsPayload.add(itemUuids[0].value) + } + } + + private backoffItem(itemUuid: Uuid): void { const backoffPenalty = this.backoffPenalties.get(itemUuid.value) || 0 - const newBackoffPenalty = backoffPenalty === 0 ? 1_000 : backoffPenalty * 2 + const newBackoffPenalty = backoffPenalty === 0 ? this.INITIAL_BACKOFF_PENALTY_MS : backoffPenalty * 2 this.backoffPenalties.set(itemUuid.value, newBackoffPenalty) this.backoffStartTimestamps.set(itemUuid.value, Date.now()) diff --git a/packages/services/src/Domain/Sync/SyncBackoffServiceInterface.ts b/packages/services/src/Domain/Sync/SyncBackoffServiceInterface.ts index cc4a47c9c90..4b4d7f31fbf 100644 --- a/packages/services/src/Domain/Sync/SyncBackoffServiceInterface.ts +++ b/packages/services/src/Domain/Sync/SyncBackoffServiceInterface.ts @@ -4,6 +4,6 @@ import { AnyItemInterface } from '@standardnotes/models' export interface SyncBackoffServiceInterface { isItemInBackoff(item: AnyItemInterface): boolean - backoffItem(itemUuid: Uuid): void + backoffItems(itemUuids: Uuid[]): void getSmallerSubsetOfItemUuidsInBackoff(): Uuid[] } diff --git a/packages/snjs/lib/Services/Sync/SyncService.ts b/packages/snjs/lib/Services/Sync/SyncService.ts index 7afd22463a1..adfdf6e9432 100644 --- a/packages/snjs/lib/Services/Sync/SyncService.ts +++ b/packages/snjs/lib/Services/Sync/SyncService.ts @@ -1057,6 +1057,7 @@ export class SyncService } private markTooLargePayloadsAsBackedOff(operation: AccountSyncOperation): void { + const uuidsToBackOff = [] for (const payload of operation.payloads) { this.logger.debug(`Marking item ${payload.uuid} as backed off due to request payload being too large`) @@ -1066,10 +1067,10 @@ export class SyncService continue } - const uuid = uuidOrError.getValue() - - this.syncBackoffService.backoffItem(uuid) + uuidsToBackOff.push(uuidOrError.getValue()) } + + this.syncBackoffService.backoffItems(uuidsToBackOff) } private async handleSuccessServerResponse(operation: AccountSyncOperation, response: ServerSyncResponse) {