Skip to content

Commit 35645be

Browse files
karolsojkokarolsojko
committed
marking items as backed off and retrying with smaller subset
1 parent 1189264 commit 35645be

File tree

5 files changed

+153
-23
lines changed

5 files changed

+153
-23
lines changed

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

+72-12
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,115 @@
11
import { AnyItemInterface } from '@standardnotes/models'
22
import { SyncBackoffService } from './SyncBackoffService'
3+
import { InternalEventBusInterface } from '../..'
4+
import { Uuid } from '@standardnotes/domain-core'
35

46
describe('SyncBackoffService', () => {
5-
const createService = () => new SyncBackoffService()
7+
let internalEventBus: InternalEventBusInterface
8+
9+
const createService = () => new SyncBackoffService(internalEventBus)
10+
11+
beforeEach(() => {
12+
internalEventBus = {} as jest.Mocked<InternalEventBusInterface>
13+
})
614

715
it('should not be in backoff if no backoff was set', () => {
816
const service = createService()
917

10-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(false)
18+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
1119
})
1220

1321
it('should be in backoff if backoff was set', () => {
1422
const service = createService()
1523

16-
service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
24+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
1725

18-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
26+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
1927
})
2028

2129
it('should not be in backoff if backoff expired', () => {
2230
const service = createService()
2331

2432
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
2533

26-
service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
34+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
2735

2836
jest.spyOn(Date, 'now').mockReturnValueOnce(2_000_000)
2937

30-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(false)
38+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
3139
})
3240

3341
it('should double backoff penalty on each backoff', () => {
3442
const service = createService()
3543

3644
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
3745

38-
service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
46+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
3947

4048
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
41-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
49+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
4250

4351
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
44-
service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
52+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
4553

4654
jest.spyOn(Date, 'now').mockReturnValueOnce(1_001_000)
47-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
55+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
4856

4957
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
50-
service.backoffItem({ uuid: '123' } as jest.Mocked<AnyItemInterface>)
58+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
5159

5260
jest.spyOn(Date, 'now').mockReturnValueOnce(1_003_000)
53-
expect(service.isItemInBackoff({ uuid: '123' } as jest.Mocked<AnyItemInterface>)).toBe(true)
61+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
62+
})
63+
64+
it('should remove backoff penalty on successful sync', async () => {
65+
const service = createService()
66+
67+
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
68+
69+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
70+
71+
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
72+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
73+
74+
await service.handleEvent({
75+
type: 'CompletedIncrementalSync',
76+
payload: {
77+
uploadedPayloads: [
78+
{
79+
uuid: '00000000-0000-0000-0000-000000000000',
80+
},
81+
],
82+
},
83+
})
84+
85+
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
86+
})
87+
88+
it('should get a smaller subset of item uuids in backoff that have lesser penalty', () => {
89+
const service = createService()
90+
91+
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
92+
93+
for (let i = 0; i < 5; i++) {
94+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
95+
}
96+
for (let i = 0; i < 4; i++) {
97+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000001').getValue())
98+
}
99+
for (let i = 0; i < 3; i++) {
100+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000002').getValue())
101+
}
102+
for (let i = 0; i < 2; i++) {
103+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000003').getValue())
104+
}
105+
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000004').getValue())
106+
107+
const subset = service.getSmallerSubsetOfItemUuidsInBackoff()
108+
109+
expect(subset.length).toEqual(3)
110+
111+
expect(subset[0].value).toBe('00000000-0000-0000-0000-000000000004')
112+
expect(subset[1].value).toBe('00000000-0000-0000-0000-000000000003')
113+
expect(subset[2].value).toBe('00000000-0000-0000-0000-000000000002')
54114
})
55115
})

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

+20-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AnyItemInterface, ServerSyncPushContextualPayload } from '@standardnotes/models'
2+
import { Uuid } from '@standardnotes/domain-core'
23

34
import { SyncBackoffServiceInterface } from './SyncBackoffServiceInterface'
45
import { AbstractService } from '../Service/AbstractService'
@@ -21,6 +22,21 @@ export class SyncBackoffService
2122
this.backoffStartTimestamps = new Map<string, number>()
2223
}
2324

25+
getSmallerSubsetOfItemUuidsInBackoff(): Uuid[] {
26+
const uuids = Array.from(this.backoffPenalties.keys())
27+
28+
const uuidsSortedByPenaltyAscending = uuids.sort((a, b) => {
29+
const penaltyA = this.backoffPenalties.get(a) || 0
30+
const penaltyB = this.backoffPenalties.get(b) || 0
31+
32+
return penaltyA - penaltyB
33+
})
34+
35+
const halfLength = Math.ceil(uuidsSortedByPenaltyAscending.length / 2)
36+
37+
return uuidsSortedByPenaltyAscending.slice(0, halfLength).map((uuid) => Uuid.create(uuid).getValue())
38+
}
39+
2440
async handleEvent(event: InternalEventInterface): Promise<void> {
2541
if (event.type === ApplicationEvent.CompletedIncrementalSync) {
2642
for (const payload of (event.payload as Record<string, unknown>)
@@ -47,12 +63,12 @@ export class SyncBackoffService
4763
return backoffEndTimestamp > Date.now()
4864
}
4965

50-
backoffItem(item: AnyItemInterface): void {
51-
const backoffPenalty = this.backoffPenalties.get(item.uuid) || 0
66+
backoffItem(itemUuid: Uuid): void {
67+
const backoffPenalty = this.backoffPenalties.get(itemUuid.value) || 0
5268

5369
const newBackoffPenalty = backoffPenalty === 0 ? 1_000 : backoffPenalty * 2
5470

55-
this.backoffPenalties.set(item.uuid, newBackoffPenalty)
56-
this.backoffStartTimestamps.set(item.uuid, Date.now())
71+
this.backoffPenalties.set(itemUuid.value, newBackoffPenalty)
72+
this.backoffStartTimestamps.set(itemUuid.value, Date.now())
5773
}
5874
}

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import { Uuid } from '@standardnotes/domain-core'
2+
13
import { AnyItemInterface } from '@standardnotes/models'
24

35
export interface SyncBackoffServiceInterface {
46
isItemInBackoff(item: AnyItemInterface): boolean
5-
backoffItem(item: AnyItemInterface): void
7+
backoffItem(itemUuid: Uuid): void
8+
getSmallerSubsetOfItemUuidsInBackoff(): Uuid[]
69
}

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

+9
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export class SyncOpStatus {
1717
private databaseLoadTotal = 0
1818
private databaseLoadDone = false
1919
private syncing = false
20+
private isTooLarge = false
2021
private syncStart!: Date
2122
private timingMonitor?: any
2223

@@ -71,6 +72,14 @@ export class SyncOpStatus {
7172
this.syncing = false
7273
}
7374

75+
setIsTooLarge() {
76+
this.isTooLarge = true
77+
}
78+
79+
get isTooLargeToSync() {
80+
return this.isTooLarge
81+
}
82+
7483
get syncInProgress() {
7584
return this.syncing === true
7685
}

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

+48-6
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ import {
9797
} from '@standardnotes/encryption'
9898
import { CreatePayloadFromRawServerItem } from './Account/Utilities'
9999
import { DecryptedServerConflictMap, TrustedServerConflictMap } from './Account/ServerConflictMap'
100-
import { ContentType } from '@standardnotes/domain-core'
100+
import { ContentType, Uuid } from '@standardnotes/domain-core'
101101
import { SyncFrequencyGuardInterface } from './SyncFrequencyGuardInterface'
102102

103103
const DEFAULT_MAJOR_CHANGE_THRESHOLD = 15
@@ -459,7 +459,23 @@ export class SyncService
459459

460460
const itemsWithoutBackoffPenalty = dirtyItems.filter((item) => !this.syncBackoffService.isItemInBackoff(item))
461461

462-
return itemsWithoutBackoffPenalty
462+
const itemsNeedingSync = itemsWithoutBackoffPenalty
463+
464+
if (itemsWithoutBackoffPenalty.length === 0) {
465+
const subsetOfBackoffUuids = this.syncBackoffService.getSmallerSubsetOfItemUuidsInBackoff()
466+
if (subsetOfBackoffUuids.length > 0) {
467+
this.logger.debug('All items are synced. Attempting to sync a subset of backoff items.')
468+
469+
subsetOfBackoffUuids.forEach((uuid: Uuid) => {
470+
const item = dirtyItems.find((item) => item.uuid === uuid.value)
471+
if (item) {
472+
itemsNeedingSync.push(item)
473+
}
474+
})
475+
}
476+
}
477+
478+
return itemsNeedingSync
463479
}
464480

465481
public async markAllItemsAsNeedingSyncAndPersist(): Promise<void> {
@@ -941,8 +957,17 @@ export class SyncService
941957

942958
releaseLock()
943959

944-
const { hasError } = await this.handleSyncOperationFinish(operation, options, neverSyncedDeleted, syncMode)
960+
const { hasError, isPayloadTooLarge } = await this.handleSyncOperationFinish(
961+
operation,
962+
options,
963+
neverSyncedDeleted,
964+
syncMode,
965+
)
945966
if (hasError) {
967+
if (isPayloadTooLarge && operation instanceof AccountSyncOperation) {
968+
this.markTooLargePayloadsAsBackedOff(operation)
969+
}
970+
946971
return
947972
}
948973

@@ -1023,13 +1048,30 @@ export class SyncService
10231048

10241049
if (response.status === PAYLOAD_TOO_LARGE) {
10251050
void this.notifyEvent(SyncEvent.PayloadTooLarge)
1051+
this.opStatus?.setIsTooLarge()
10261052
}
10271053

10281054
this.opStatus?.setError(response.error)
10291055

10301056
void this.notifyEvent(SyncEvent.SyncError, response)
10311057
}
10321058

1059+
private markTooLargePayloadsAsBackedOff(operation: AccountSyncOperation): void {
1060+
for (const payload of operation.payloads) {
1061+
this.logger.debug(`Marking item ${payload.uuid} as backed off due to request payload being too large`)
1062+
1063+
const uuidOrError = Uuid.create(payload.uuid)
1064+
if (uuidOrError.isFailed()) {
1065+
this.logger.error('Failed to create uuid from payload', payload)
1066+
1067+
continue
1068+
}
1069+
const uuid = uuidOrError.getValue()
1070+
1071+
this.syncBackoffService.backoffItem(uuid)
1072+
}
1073+
}
1074+
10331075
private async handleSuccessServerResponse(operation: AccountSyncOperation, response: ServerSyncResponse) {
10341076
if (this._simulate_latency) {
10351077
await sleep(this._simulate_latency.latency)
@@ -1305,11 +1347,11 @@ export class SyncService
13051347
options: SyncOptions,
13061348
neverSyncedDeleted: DeletedItemInterface[],
13071349
syncMode: SyncMode,
1308-
) {
1350+
): Promise<{ hasError: boolean; isPayloadTooLarge: boolean }> {
13091351
this.opStatus.setDidEnd()
13101352

13111353
if (this.opStatus.hasError()) {
1312-
return { hasError: true }
1354+
return { hasError: true, isPayloadTooLarge: this.opStatus.isTooLargeToSync }
13131355
}
13141356

13151357
this.opStatus.reset()
@@ -1332,7 +1374,7 @@ export class SyncService
13321374
})
13331375
}
13341376

1335-
return { hasError: false }
1377+
return { hasError: false, isPayloadTooLarge: false }
13361378
}
13371379

13381380
private async handleDownloadFirstCompletionAndSyncAgain(online: boolean, options: SyncOptions) {

0 commit comments

Comments
 (0)