marking items as backed off and retrying with smaller subset

packages/services/src/Domain/Sync/SyncBackoffService.spec.ts

@@ -1,55 +1,115 @@
 import { AnyItemInterface } from '@standardnotes/models'
 import { SyncBackoffService } from './SyncBackoffService'
+import { InternalEventBusInterface } from '../..'
+import { Uuid } from '@standardnotes/domain-core'
 
 describe('SyncBackoffService', () => {
-  const createService = () => new SyncBackoffService()
+  let internalEventBus: InternalEventBusInterface
+
+  const createService = () => new SyncBackoffService(internalEventBus)
+
+  beforeEach(() => {
+    internalEventBus = {} as jest.Mocked<InternalEventBusInterface>
+  })
 
   it('should not be in backoff if no backoff was set', () => {
     const service = createService()
 
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(false)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
   })
 
   it('should be in backoff if backoff was set', () => {
     const service = createService()
 
-    service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
+    service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
 
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
   })
 
   it('should not be in backoff if backoff expired', () => {
     const service = createService()
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
 
-    service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
+    service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(2_000_000)
 
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(false)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
   })
 
   it('should double backoff penalty on each backoff', () => {
     const service = createService()
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
 
-    service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
+    service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
-    service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
+    service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_001_000)
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
-    service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
+    service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
 
     jest.spyOn(Date, 'now').mockReturnValueOnce(1_003_000)
-    expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
+  })
+
+  it('should remove backoff penalty on successful sync', async () => {
+    const service = createService()
+
+    jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
+
+    service.backoffItem(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<AnyItemInterface>)).toBe(true)
+
+    await service.handleEvent({
+      type: 'CompletedIncrementalSync',
+      payload: {
+        uploadedPayloads: [
+          {
+            uuid: '00000000-0000-0000-0000-000000000000',
+          },
+        ],
+      },
+    })
+
+    expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
+  })
+
+  it('should get a smaller subset of item uuids in backoff that have lesser penalty', () => {
+    const service = createService()
+
+    jest.spyOn(Date, 'now').mockReturnValueOnce(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()
+
+    expect(subset.length).toEqual(3)
+
+    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')
   })
 })

packages/services/src/Domain/Sync/SyncBackoffService.ts

@@ -1,4 +1,5 @@
 import { AnyItemInterface, ServerSyncPushContextualPayload } from '@standardnotes/models'
+import { Uuid } from '@standardnotes/domain-core'
 
 import { SyncBackoffServiceInterface } from './SyncBackoffServiceInterface'
 import { AbstractService } from '../Service/AbstractService'
@@ -21,6 +22,21 @@ export class SyncBackoffService
     this.backoffStartTimestamps = new Map<string, number>()
   }
 
+  getSmallerSubsetOfItemUuidsInBackoff(): Uuid[] {
+    const uuids = Array.from(this.backoffPenalties.keys())
+
+    const uuidsSortedByPenaltyAscending = uuids.sort((a, b) => {
+      const penaltyA = this.backoffPenalties.get(a) || 0
+      const penaltyB = this.backoffPenalties.get(b) || 0
+
+      return penaltyA - penaltyB
+    })
+
+    const halfLength = Math.ceil(uuidsSortedByPenaltyAscending.length / 2)
+
+    return uuidsSortedByPenaltyAscending.slice(0, halfLength).map((uuid) => Uuid.create(uuid).getValue())
+  }
+
   async handleEvent(event: InternalEventInterface): Promise<void> {
     if (event.type === ApplicationEvent.CompletedIncrementalSync) {
       for (const payload of (event.payload as Record<string, unknown>)
@@ -47,12 +63,12 @@ export class SyncBackoffService
     return backoffEndTimestamp > Date.now()
   }
 
-  backoffItem(item: AnyItemInterface): void {
-    const backoffPenalty = this.backoffPenalties.get(item.uuid) || 0
+  backoffItem(itemUuid: Uuid): void {
+    const backoffPenalty = this.backoffPenalties.get(itemUuid.value) || 0
 
     const newBackoffPenalty = backoffPenalty === 0 ? 1_000 : backoffPenalty * 2
 
-    this.backoffPenalties.set(item.uuid, newBackoffPenalty)
-    this.backoffStartTimestamps.set(item.uuid, Date.now())
+    this.backoffPenalties.set(itemUuid.value, newBackoffPenalty)
+    this.backoffStartTimestamps.set(itemUuid.value, Date.now())
   }
 }

packages/services/src/Domain/Sync/SyncBackoffServiceInterface.ts

@@ -1,6 +1,9 @@
+import { Uuid } from '@standardnotes/domain-core'
+
 import { AnyItemInterface } from '@standardnotes/models'
 
 export interface SyncBackoffServiceInterface {
   isItemInBackoff(item: AnyItemInterface): boolean
-  backoffItem(item: AnyItemInterface): void
+  backoffItem(itemUuid: Uuid): void
+  getSmallerSubsetOfItemUuidsInBackoff(): Uuid[]
 }

packages/services/src/Domain/Sync/SyncOpStatus.ts

@@ -17,6 +17,7 @@ export class SyncOpStatus {
   private databaseLoadTotal = 0
   private databaseLoadDone = false
   private syncing = false
+  private isTooLarge = false
   private syncStart!: Date
   private timingMonitor?: any
 
@@ -71,6 +72,14 @@ export class SyncOpStatus {
     this.syncing = false
   }
 
+  setIsTooLarge() {
+    this.isTooLarge = true
+  }
+
+  get isTooLargeToSync() {
+    return this.isTooLarge
+  }
+
   get syncInProgress() {
     return this.syncing === true
   }

packages/snjs/lib/Services/Sync/SyncService.ts

@@ -97,7 +97,7 @@ import {
 } from '@standardnotes/encryption'
 import { CreatePayloadFromRawServerItem } from './Account/Utilities'
 import { DecryptedServerConflictMap, TrustedServerConflictMap } from './Account/ServerConflictMap'
-import { ContentType } from '@standardnotes/domain-core'
+import { ContentType, Uuid } from '@standardnotes/domain-core'
 import { SyncFrequencyGuardInterface } from './SyncFrequencyGuardInterface'
 
 const DEFAULT_MAJOR_CHANGE_THRESHOLD = 15
@@ -459,7 +459,23 @@ export class SyncService
 
     const itemsWithoutBackoffPenalty = dirtyItems.filter((item) => !this.syncBackoffService.isItemInBackoff(item))
 
-    return itemsWithoutBackoffPenalty
+    const itemsNeedingSync = itemsWithoutBackoffPenalty
+
+    if (itemsWithoutBackoffPenalty.length === 0) {
+      const subsetOfBackoffUuids = this.syncBackoffService.getSmallerSubsetOfItemUuidsInBackoff()
+      if (subsetOfBackoffUuids.length > 0) {
+        this.logger.debug('All items are synced. Attempting to sync a subset of backoff items.')
+
+        subsetOfBackoffUuids.forEach((uuid: Uuid) => {
+          const item = dirtyItems.find((item) => item.uuid === uuid.value)
+          if (item) {
+            itemsNeedingSync.push(item)
+          }
+        })
+      }
+    }
+
+    return itemsNeedingSync
   }
 
   public async markAllItemsAsNeedingSyncAndPersist(): Promise<void> {
@@ -941,8 +957,17 @@ export class SyncService
 
     releaseLock()
 
-    const { hasError } = await this.handleSyncOperationFinish(operation, options, neverSyncedDeleted, syncMode)
+    const { hasError, isPayloadTooLarge } = await this.handleSyncOperationFinish(
+      operation,
+      options,
+      neverSyncedDeleted,
+      syncMode,
+    )
     if (hasError) {
+      if (isPayloadTooLarge && operation instanceof AccountSyncOperation) {
+        this.markTooLargePayloadsAsBackedOff(operation)
+      }
+
       return
     }
 
@@ -1023,13 +1048,30 @@ export class SyncService
 
     if (response.status === PAYLOAD_TOO_LARGE) {
       void this.notifyEvent(SyncEvent.PayloadTooLarge)
+      this.opStatus?.setIsTooLarge()
     }
 
     this.opStatus?.setError(response.error)
 
     void this.notifyEvent(SyncEvent.SyncError, response)
   }
 
+  private markTooLargePayloadsAsBackedOff(operation: AccountSyncOperation): void {
+    for (const payload of operation.payloads) {
+      this.logger.debug(`Marking item ${payload.uuid} as backed off due to request payload being too large`)
+
+      const uuidOrError = Uuid.create(payload.uuid)
+      if (uuidOrError.isFailed()) {
+        this.logger.error('Failed to create uuid from payload', payload)
+
+        continue
+      }
+      const uuid = uuidOrError.getValue()
+
+      this.syncBackoffService.backoffItem(uuid)
+    }
+  }
+
   private async handleSuccessServerResponse(operation: AccountSyncOperation, response: ServerSyncResponse) {
     if (this._simulate_latency) {
       await sleep(this._simulate_latency.latency)
@@ -1305,11 +1347,11 @@ export class SyncService
     options: SyncOptions,
     neverSyncedDeleted: DeletedItemInterface[],
     syncMode: SyncMode,
-  ) {
+  ): Promise<{ hasError: boolean; isPayloadTooLarge: boolean }> {
     this.opStatus.setDidEnd()
 
     if (this.opStatus.hasError()) {
-      return { hasError: true }
+      return { hasError: true, isPayloadTooLarge: this.opStatus.isTooLargeToSync }
     }
 
     this.opStatus.reset()
@@ -1332,7 +1374,7 @@ export class SyncService
       })
     }
 
-    return { hasError: false }
+    return { hasError: false, isPayloadTooLarge: false }
   }
 
   private async handleDownloadFirstCompletionAndSyncAgain(online: boolean, options: SyncOptions) {