Skip to content

Commit 70317be

Browse files
karolsojkokarolsojko
committed
fix backing off items
1 parent b8eda3a commit 70317be

File tree

4 files changed

+65
-35
lines changed

4 files changed

+65
-35
lines changed

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

+21-27
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('SyncBackoffService', () => {
2121
it('should be in backoff if backoff was set', () => {
2222
const service = createService()
2323

24-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
24+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
2525

2626
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
2727
})
@@ -31,7 +31,7 @@ describe('SyncBackoffService', () => {
3131

3232
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
3333

34-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
34+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
3535

3636
jest.spyOn(Date, 'now').mockReturnValueOnce(2_000_000)
3737

@@ -43,19 +43,19 @@ describe('SyncBackoffService', () => {
4343

4444
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
4545

46-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
46+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
4747

4848
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
4949
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
5050

5151
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
52-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
52+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
5353

5454
jest.spyOn(Date, 'now').mockReturnValueOnce(1_001_000)
5555
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
5656

5757
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
58-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
58+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
5959

6060
jest.spyOn(Date, 'now').mockReturnValueOnce(1_003_000)
6161
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
@@ -66,7 +66,7 @@ describe('SyncBackoffService', () => {
6666

6767
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
6868

69-
service.backoffItem(Uuid.create('00000000-0000-0000-0000-000000000000').getValue())
69+
service.backoffItems([Uuid.create('00000000-0000-0000-0000-000000000000').getValue()])
7070

7171
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
7272
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(true)
@@ -85,31 +85,25 @@ describe('SyncBackoffService', () => {
8585
expect(service.isItemInBackoff({ uuid: '00000000-0000-0000-0000-000000000000' } as jest.Mocked<AnyItemInterface>)).toBe(false)
8686
})
8787

88-
it('should get a smaller subset of item uuids in backoff that have lesser penalty', () => {
88+
it('should get a smaller and smaller subset of item uuids to back off until single uuids are ruled out', () => {
8989
const service = createService()
9090

91-
jest.spyOn(Date, 'now').mockReturnValueOnce(1_000_000)
91+
jest.spyOn(Date, 'now').mockReturnValue(1_000_000)
9292

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()
93+
service.backoffItems([
94+
Uuid.create('00000000-0000-0000-0000-000000000000').getValue(),
95+
Uuid.create('00000000-0000-0000-0000-000000000001').getValue(),
96+
Uuid.create('00000000-0000-0000-0000-000000000002').getValue(),
97+
Uuid.create('00000000-0000-0000-0000-000000000003').getValue(),
98+
Uuid.create('00000000-0000-0000-0000-000000000004').getValue(),
99+
])
108100

109-
expect(subset.length).toEqual(3)
101+
const expectedSubsetLenghts = [3, 3, 3, 3, 2, 1, 1, 1, 1, 1, 0]
102+
for (let i = 0; i < expectedSubsetLenghts.length; i++) {
103+
const subset = service.getSmallerSubsetOfItemUuidsInBackoff()
104+
service.backoffItems(subset)
110105

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')
106+
expect(subset.length).toBe(expectedSubsetLenghts[i])
107+
}
114108
})
115109
})

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

+39-4
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,33 @@ export class SyncBackoffService
1212
extends AbstractService
1313
implements SyncBackoffServiceInterface, InternalEventHandlerInterface
1414
{
15+
private INITIAL_BACKOFF_PENALTY_MS = 1_000
16+
private SUBSET_BACKOFF_PENALTY_CUTOFF_MS = 10_000
17+
1518
private backoffPenalties: Map<string, number>
1619
private backoffStartTimestamps: Map<string, number>
20+
private itemUuidsThatHaveBeenTriedSolelyAsPayload: Set<string>
1721

1822
constructor(protected override internalEventBus: InternalEventBusInterface) {
1923
super(internalEventBus)
2024

2125
this.backoffPenalties = new Map<string, number>()
2226
this.backoffStartTimestamps = new Map<string, number>()
27+
this.itemUuidsThatHaveBeenTriedSolelyAsPayload = new Set<string>()
2328
}
2429

2530
getSmallerSubsetOfItemUuidsInBackoff(): Uuid[] {
2631
const uuids = Array.from(this.backoffPenalties.keys())
2732

28-
const uuidsSortedByPenaltyAscending = uuids.sort((a, b) => {
33+
const uuidsThatHaveNotBeenTriedAsSingleItemRequests = uuids.filter(
34+
(uuid) => !this.itemUuidsThatHaveBeenTriedSolelyAsPayload.has(uuid),
35+
)
36+
37+
if (uuidsThatHaveNotBeenTriedAsSingleItemRequests.length === 0) {
38+
return []
39+
}
40+
41+
const uuidsSortedByPenaltyAscending = uuidsThatHaveNotBeenTriedAsSingleItemRequests.sort((a, b) => {
2942
const penaltyA = this.backoffPenalties.get(a) || 0
3043
const penaltyB = this.backoffPenalties.get(b) || 0
3144

@@ -34,7 +47,19 @@ export class SyncBackoffService
3447

3548
const halfLength = Math.ceil(uuidsSortedByPenaltyAscending.length / 2)
3649

37-
return uuidsSortedByPenaltyAscending.slice(0, halfLength).map((uuid) => Uuid.create(uuid).getValue())
50+
const halfOfUuidsSortedByPenaltyAscendingMeetingCutoff = []
51+
let penaltySum = 0
52+
for (let i = 0; i < halfLength; i++) {
53+
const uuid = uuidsSortedByPenaltyAscending[i]
54+
55+
penaltySum += this.backoffPenalties.get(uuid) || 0
56+
57+
if (penaltySum <= this.SUBSET_BACKOFF_PENALTY_CUTOFF_MS) {
58+
halfOfUuidsSortedByPenaltyAscendingMeetingCutoff.push(uuid)
59+
}
60+
}
61+
62+
return halfOfUuidsSortedByPenaltyAscendingMeetingCutoff.map((uuid) => Uuid.create(uuid).getValue())
3863
}
3964

4065
async handleEvent(event: InternalEventInterface): Promise<void> {
@@ -63,10 +88,20 @@ export class SyncBackoffService
6388
return backoffEndTimestamp > Date.now()
6489
}
6590

66-
backoffItem(itemUuid: Uuid): void {
91+
backoffItems(itemUuids: Uuid[]): void {
92+
for (const itemUuid of itemUuids) {
93+
this.backoffItem(itemUuid)
94+
}
95+
96+
if (itemUuids.length === 1) {
97+
this.itemUuidsThatHaveBeenTriedSolelyAsPayload.add(itemUuids[0].value)
98+
}
99+
}
100+
101+
private backoffItem(itemUuid: Uuid): void {
67102
const backoffPenalty = this.backoffPenalties.get(itemUuid.value) || 0
68103

69-
const newBackoffPenalty = backoffPenalty === 0 ? 1_000 : backoffPenalty * 2
104+
const newBackoffPenalty = backoffPenalty === 0 ? this.INITIAL_BACKOFF_PENALTY_MS : backoffPenalty * 2
70105

71106
this.backoffPenalties.set(itemUuid.value, newBackoffPenalty)
72107
this.backoffStartTimestamps.set(itemUuid.value, Date.now())

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ import { AnyItemInterface } from '@standardnotes/models'
44

55
export interface SyncBackoffServiceInterface {
66
isItemInBackoff(item: AnyItemInterface): boolean
7-
backoffItem(itemUuid: Uuid): void
7+
backoffItems(itemUuids: Uuid[]): void
88
getSmallerSubsetOfItemUuidsInBackoff(): Uuid[]
99
}

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

+4-3
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,7 @@ export class SyncService
10571057
}
10581058

10591059
private markTooLargePayloadsAsBackedOff(operation: AccountSyncOperation): void {
1060+
const uuidsToBackOff = []
10601061
for (const payload of operation.payloads) {
10611062
this.logger.debug(`Marking item ${payload.uuid} as backed off due to request payload being too large`)
10621063

@@ -1066,10 +1067,10 @@ export class SyncService
10661067

10671068
continue
10681069
}
1069-
const uuid = uuidOrError.getValue()
1070-
1071-
this.syncBackoffService.backoffItem(uuid)
1070+
uuidsToBackOff.push(uuidOrError.getValue())
10721071
}
1072+
1073+
this.syncBackoffService.backoffItems(uuidsToBackOff)
10731074
}
10741075

10751076
private async handleSuccessServerResponse(operation: AccountSyncOperation, response: ServerSyncResponse) {

0 commit comments

Comments
 (0)