Skip to content

Commit

Permalink
fix backing off items
Browse files Browse the repository at this point in the history
  • Loading branch information
karolsojko committed Feb 2, 2024
1 parent 35645be commit 99659e7
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 35 deletions.
48 changes: 21 additions & 27 deletions packages/services/src/Domain/Sync/SyncBackoffService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AnyItemInterface>)).toBe(true)
})
Expand All @@ -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)

Expand All @@ -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<AnyItemInterface>)).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<AnyItemInterface>)).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<AnyItemInterface>)).toBe(true)
Expand All @@ -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<AnyItemInterface>)).toBe(true)
Expand All @@ -85,31 +85,25 @@ describe('SyncBackoffService', () => {
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', () => {
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])
}
})
})
43 changes: 39 additions & 4 deletions packages/services/src/Domain/Sync/SyncBackoffService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>
private backoffStartTimestamps: Map<string, number>
private itemUuidsThatHaveBeenTriedSolelyAsPayload: Set<string>

constructor(protected override internalEventBus: InternalEventBusInterface) {
super(internalEventBus)

this.backoffPenalties = new Map<string, number>()
this.backoffStartTimestamps = new Map<string, number>()
this.itemUuidsThatHaveBeenTriedSolelyAsPayload = new Set<string>()
}

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

Expand All @@ -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<void> {
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
}
7 changes: 4 additions & 3 deletions packages/snjs/lib/Services/Sync/SyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

Expand All @@ -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) {
Expand Down

0 comments on commit 99659e7

Please sign in to comment.