Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: handle back off syncing server responses #2811

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
MfaServiceInterface,
GenerateUuid,
CreateDecryptedBackupFile,
SyncBackoffServiceInterface,
} from '@standardnotes/services'
import { VaultLockServiceInterface } from './../VaultLock/VaultLockServiceInterface'
import { HistoryServiceInterface } from './../History/HistoryServiceInterface'
Expand Down Expand Up @@ -100,6 +101,7 @@ export interface ApplicationInterface {
get storage(): StorageServiceInterface
get subscriptions(): SubscriptionManagerInterface
get sync(): SyncServiceInterface
get syncBackoff(): SyncBackoffServiceInterface
get user(): UserServiceInterface
get vaultInvites(): VaultInviteServiceInterface
get vaultLocks(): VaultLockServiceInterface
Expand Down
1 change: 1 addition & 0 deletions packages/services/src/Domain/Event/ApplicationEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,5 @@ export enum ApplicationEvent {
CompletedInitialSync = 'Application:CompletedInitialSync',
DidPurchaseSubscription = 'Application:DidPurchaseSubscription',
SyncTooManyRequests = 'Application:SyncTooManyRequests',
SyncPayloadTooLarge = 'Application:SyncPayloadTooLarge',
}
1 change: 1 addition & 0 deletions packages/services/src/Domain/Event/SyncEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export enum SyncEvent {
ReceivedNotifications = 'received-user-events',
ReceivedAsymmetricMessages = 'received-asymmetric-messages',
TooManyRequests = 'too-many-requests',
PayloadTooLarge = 'payload-too-large',
}

export type SyncEventReceivedRemoteSharedVaultsData = SharedVaultServerHash[]
Expand Down
78 changes: 66 additions & 12 deletions packages/services/src/Domain/Sync/SyncBackoffService.spec.ts
Original file line number Diff line number Diff line change
@@ -1,55 +1,109 @@
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.backoffItems([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.backoffItems([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.backoffItems([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.backoffItems([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.backoffItems([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.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)

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 and smaller subset of item uuids to back off until single uuids are ruled out', () => {
const service = createService()

jest.spyOn(Date, 'now').mockReturnValue(1_000_000)

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(),
])

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.length).toBe(expectedSubsetLenghts[i])
}
})
})
94 changes: 86 additions & 8 deletions packages/services/src/Domain/Sync/SyncBackoffService.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,81 @@
import { AnyItemInterface } from '@standardnotes/models'
import { AnyItemInterface, ServerSyncPushContextualPayload } from '@standardnotes/models'
import { Uuid } from '@standardnotes/domain-core'

import { SyncBackoffServiceInterface } from './SyncBackoffServiceInterface'
import { AbstractService } from '../Service/AbstractService'
import { InternalEventHandlerInterface } from '../Internal/InternalEventHandlerInterface'
import { InternalEventBusInterface } from '../Internal/InternalEventBusInterface'
import { InternalEventInterface } from '../Internal/InternalEventInterface'
import { SyncEvent } from '../Event/SyncEvent'

export class SyncBackoffService
extends AbstractService
implements SyncBackoffServiceInterface, InternalEventHandlerInterface
{
private INITIAL_BACKOFF_PENALTY_MS = 1_000
private SUBSET_BACKOFF_PENALTY_CUTOFF_MS = 10_000

export class SyncBackoffService implements SyncBackoffServiceInterface {
private backoffPenalties: Map<string, number>
private backoffStartTimestamps: Map<string, number>
private itemUuidsThatHaveBeenTriedSolelyAsPayload: Set<string>

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

constructor() {
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 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

return penaltyA - penaltyB
})

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

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> {
if (
event.type !== SyncEvent.PaginatedSyncRequestCompleted ||
event.payload === undefined ||
(event.payload as Record<string, unknown>).uploadedPayloads === undefined
) {
return
}

for (const payload of (event.payload as Record<string, unknown>)
.uploadedPayloads as ServerSyncPushContextualPayload[]) {
this.backoffPenalties.delete(payload.uuid)
this.backoffStartTimestamps.delete(payload.uuid)
}
}

isItemInBackoff(item: AnyItemInterface): boolean {
Expand All @@ -26,12 +94,22 @@ export class SyncBackoffService implements SyncBackoffServiceInterface {
return backoffEndTimestamp > Date.now()
}

backoffItem(item: AnyItemInterface): void {
const backoffPenalty = this.backoffPenalties.get(item.uuid) || 0
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(item.uuid, newBackoffPenalty)
this.backoffStartTimestamps.set(item.uuid, Date.now())
this.backoffPenalties.set(itemUuid.value, newBackoffPenalty)
this.backoffStartTimestamps.set(itemUuid.value, Date.now())
}
}
Original file line number Diff line number Diff line change
@@ -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
backoffItems(itemUuids: Uuid[]): void
getSmallerSubsetOfItemUuidsInBackoff(): Uuid[]
}
9 changes: 9 additions & 0 deletions packages/services/src/Domain/Sync/SyncOpStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Down
13 changes: 9 additions & 4 deletions packages/snjs/lib/Application/Application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import {
CreateDecryptedBackupFile,
CreateEncryptedBackupFile,
WebSocketsService,
SyncBackoffServiceInterface,
} from '@standardnotes/services'
import {
SNNote,
Expand Down Expand Up @@ -116,7 +117,7 @@ import {
LoggerInterface,
canBlockDeinit,
} from '@standardnotes/utils'
import { UuidString, ApplicationEventPayload } from '../Types'
import { UuidString } from '../Types'
import { applicationEventForSyncEvent } from '@Lib/Application/Event'
import { BackupServiceInterface, FilesClientInterface } from '@standardnotes/files'
import { ComputePrivateUsername } from '@standardnotes/encryption'
Expand Down Expand Up @@ -273,12 +274,12 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli
}),
)

const syncEventCallback = async (eventName: SyncEvent) => {
const syncEventCallback = async (eventName: SyncEvent, data?: unknown) => {
const appEvent = applicationEventForSyncEvent(eventName)
if (appEvent) {
await encryptionService.onSyncEvent(eventName)

await this.notifyEvent(appEvent)
await this.notifyEvent(appEvent, data)

if (appEvent === ApplicationEvent.CompletedFullSync) {
if (!this.handledFullSyncStage) {
Expand Down Expand Up @@ -529,7 +530,7 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli
return this.addEventObserver(filteredCallback, event)
}

private async notifyEvent(event: ApplicationEvent, data?: ApplicationEventPayload) {
private async notifyEvent(event: ApplicationEvent, data?: unknown) {
if (event === ApplicationEvent.Started) {
this.onStart()
} else if (event === ApplicationEvent.Launched) {
Expand Down Expand Up @@ -1016,6 +1017,10 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli
return this.dependencies.get<SyncServiceInterface>(TYPES.SyncService)
}

public get syncBackoff(): SyncBackoffServiceInterface {
return this.dependencies.get<SyncBackoffServiceInterface>(TYPES.SyncBackoffService)
}

public get user(): UserServiceInterface {
return this.dependencies.get<UserServiceInterface>(TYPES.UserService)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/snjs/lib/Application/Dependencies/Dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,7 @@ export class Dependencies {
})

this.factory.set(TYPES.SyncBackoffService, () => {
return new SyncBackoffService()
return new SyncBackoffService(this.get<InternalEventBus>(TYPES.InternalEventBus))
})

this.factory.set(TYPES.SyncService, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function RegisterApplicationServicesEvents(container: Dependencies, event
events.addEventHandler(container.get(TYPES.VaultInviteService), ApplicationEvent.Launched)
events.addEventHandler(container.get(TYPES.VaultInviteService), SyncEvent.ReceivedSharedVaultInvites)
events.addEventHandler(container.get(TYPES.VaultInviteService), WebSocketsServiceEvent.UserInvitedToSharedVault)
events.addEventHandler(container.get(TYPES.SyncBackoffService), SyncEvent.PaginatedSyncRequestCompleted)

if (container.get(TYPES.FilesBackupService)) {
events.addEventHandler(container.get(TYPES.FilesBackupService), ApplicationEvent.ApplicationStageChanged)
Expand Down
1 change: 1 addition & 0 deletions packages/snjs/lib/Application/Event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const map: Record<string, ApplicationEvent> = {
[SyncEvent.DatabaseWriteError]: ApplicationEvent.LocalDatabaseWriteError,
[SyncEvent.DownloadFirstSyncCompleted]: ApplicationEvent.CompletedInitialSync,
[SyncEvent.TooManyRequests]: ApplicationEvent.SyncTooManyRequests,
[SyncEvent.PayloadTooLarge]: ApplicationEvent.SyncPayloadTooLarge,
}

export function applicationEventForSyncEvent(syncEvent: SyncEvent): ApplicationEvent | undefined {
Expand Down
Loading
Loading