diff --git a/.changeset/cuddly-trainers-sparkle.md b/.changeset/cuddly-trainers-sparkle.md new file mode 100644 index 00000000..33e1baf7 --- /dev/null +++ b/.changeset/cuddly-trainers-sparkle.md @@ -0,0 +1,5 @@ +--- +'@l2beat/uif': minor +--- + +Add retries diff --git a/packages/example/src/Application.ts b/packages/example/src/Application.ts index 80a993e3..b7dde5d7 100644 --- a/packages/example/src/Application.ts +++ b/packages/example/src/Application.ts @@ -1,13 +1,14 @@ import { Logger } from '@l2beat/backend-tools' +import { BaseIndexer, Retries } from '@l2beat/uif' import { Config } from './Config' import { BalanceIndexer } from './indexers/BalanceIndexer' import { BlockNumberIndexer } from './indexers/BlockNumberIndexer' import { FakeClockIndexer } from './indexers/FakeClockIndexer' +import { TvlIndexer } from './indexers/TvlIndexer' import { BalanceRepository } from './repositories/BalanceRepository' import { BlockNumberRepository } from './repositories/BlockNumberRepository' import { TvlRepository } from './repositories/TvlRepository' -import { TvlIndexer } from './indexers/TvlIndexer' export class Application { start: () => Promise @@ -24,6 +25,12 @@ export class Application { const balanceRepository = new BalanceRepository() const tvlRepository = new TvlRepository() + BaseIndexer.DEFAULT_RETRY_STRATEGY = Retries.exponentialBackOff({ + initialTimeoutMs: 100, + maxAttempts: 10, + maxTimeoutMs: 60 * 1000, + }) + const fakeClockIndexer = new FakeClockIndexer(logger) const blockNumberIndexer = new BlockNumberIndexer( logger, diff --git a/packages/example/src/indexers/BlockNumberIndexer.ts b/packages/example/src/indexers/BlockNumberIndexer.ts index 036805b8..685c0264 100644 --- a/packages/example/src/indexers/BlockNumberIndexer.ts +++ b/packages/example/src/indexers/BlockNumberIndexer.ts @@ -1,9 +1,9 @@ -import { ChildIndexer } from '@l2beat/uif' import { Logger } from '@l2beat/backend-tools' +import { ChildIndexer, Retries } from '@l2beat/uif' +import { setTimeout } from 'timers/promises' import { BlockNumberRepository } from '../repositories/BlockNumberRepository' import { FakeClockIndexer } from './FakeClockIndexer' -import { setTimeout } from 'timers/promises' export class BlockNumberIndexer extends ChildIndexer { constructor( @@ -11,7 +11,12 @@ export class BlockNumberIndexer extends ChildIndexer { fakeClockIndexer: FakeClockIndexer, private readonly blockNumberRepository: BlockNumberRepository, ) { - super(logger, [fakeClockIndexer]) + super(logger, [fakeClockIndexer], { + updateRetryStrategy: Retries.exponentialBackOff({ + initialTimeoutMs: 100, + maxAttempts: 10, + }), + }) } override async update(from: number, to: number): Promise { diff --git a/packages/uif/package.json b/packages/uif/package.json index fbef6266..56d21791 100644 --- a/packages/uif/package.json +++ b/packages/uif/package.json @@ -33,7 +33,7 @@ "@l2beat/backend-tools": "*" }, "devDependencies": { - "@sinonjs/fake-timers": "^10.2.0", + "@sinonjs/fake-timers": "^11.1.0", "@types/sinonjs__fake-timers": "^8.1.2", "wait-for-expect": "^3.0.2" } diff --git a/packages/uif/src/BaseIndexer.test.ts b/packages/uif/src/BaseIndexer.test.ts index 280ff4b6..29a57dd3 100644 --- a/packages/uif/src/BaseIndexer.test.ts +++ b/packages/uif/src/BaseIndexer.test.ts @@ -1,8 +1,10 @@ import { Logger } from '@l2beat/backend-tools' -import { expect } from 'earl' +import { install } from '@sinonjs/fake-timers' +import { expect, mockFn } from 'earl' import { BaseIndexer, ChildIndexer, RootIndexer } from './BaseIndexer' import { IndexerAction } from './reducer/types/IndexerAction' +import { RetryStrategy } from './Retries' describe(BaseIndexer.name, () => { describe('correctly informs about updates', () => { @@ -15,6 +17,7 @@ describe(BaseIndexer.name, () => { await child.finishInvalidate() await parent.doTick(1) + await parent.finishTick(1) await child.finishUpdate(1) @@ -29,6 +32,7 @@ describe(BaseIndexer.name, () => { await child.start() await parent.doTick(1) + await parent.finishTick(1) await child.finishInvalidate() await child.finishUpdate(1) @@ -42,7 +46,7 @@ describe(BaseIndexer.name, () => { // Pi - parent indexer // Mi - middle indexer // Ci - child indexer - it('When Mi updating, passes correcty safeHeight', async () => { + it('When Mi updating, passes correctly safeHeight', async () => { const parent = new TestRootIndexer(0) const middle = new TestChildIndexer([parent], 0, 'Middle') const child = new TestChildIndexer([middle], 0, 'Child') @@ -55,15 +59,159 @@ describe(BaseIndexer.name, () => { await child.finishInvalidate() await parent.doTick(10) + await parent.finishTick(10) await middle.finishUpdate(10) expect(child.getState().status).toEqual('updating') await parent.doTick(5) + await parent.finishTick(5) expect(middle.getState().waiting).toEqual(true) }) }) + + describe('retries on error', () => { + it('invalidates and retries update', async () => { + const clock = install({ shouldAdvanceTime: true, advanceTimeDelta: 1 }) + + const parent = new TestRootIndexer(0) + + const shouldRetry = mockFn(() => true) + const markAttempt = mockFn(() => {}) + const clear = mockFn(() => {}) + + const child = new TestChildIndexer([parent], 0, '', { + updateRetryStrategy: { + shouldRetry, + markAttempt, + timeoutMs: () => 1000, + clear, + }, + }) + + await parent.start() + await child.start() + + await child.finishInvalidate() + + await parent.doTick(1) + await parent.finishTick(1) + + await child.finishUpdate(new Error('test error')) + expect(child.updating).toBeFalsy() + expect(child.invalidating).toBeTruthy() + expect(shouldRetry).toHaveBeenCalledTimes(1) + expect(markAttempt).toHaveBeenCalledTimes(1) + + await child.finishInvalidate() + expect(child.getState().status).toEqual('idle') + + await clock.tickAsync(1000) + + expect(child.getState().status).toEqual('updating') + await child.finishUpdate(1) + + expect(clear).toHaveBeenCalledTimes(1) + expect(child.getState().status).toEqual('idle') + + clock.uninstall() + }) + + it('retries invalidate', async () => { + const clock = install({ shouldAdvanceTime: true, advanceTimeDelta: 1 }) + const parent = new TestRootIndexer(0) + const invalidateShouldRetry = mockFn(() => true) + const invalidateMarkAttempt = mockFn(() => {}) + const invalidateClear = mockFn(() => {}) + + const updateShouldRetry = mockFn(() => true) + const updateMarkAttempt = mockFn(() => {}) + const updateClear = mockFn(() => {}) + + const child = new TestChildIndexer([parent], 0, '', { + invalidateRetryStrategy: { + shouldRetry: invalidateShouldRetry, + markAttempt: invalidateMarkAttempt, + timeoutMs: () => 1000, + clear: invalidateClear, + }, + updateRetryStrategy: { + shouldRetry: updateShouldRetry, + markAttempt: updateMarkAttempt, + timeoutMs: () => 1000, + clear: updateClear, + }, + }) + + await parent.start() + await child.start() + + await child.finishInvalidate() + expect(invalidateClear).toHaveBeenCalledTimes(1) + + await parent.doTick(1) + await parent.finishTick(1) + + await child.finishUpdate(new Error('test error')) + expect(updateShouldRetry).toHaveBeenCalledTimes(1) + expect(updateMarkAttempt).toHaveBeenCalledTimes(1) + + await child.finishInvalidate(new Error('test error')) + expect(invalidateMarkAttempt).toHaveBeenCalledTimes(1) + expect(invalidateShouldRetry).toHaveBeenCalledTimes(1) + expect(child.getState().status).toEqual('idle') + + await clock.tickAsync(1000) + + expect(child.getState().status).toEqual('invalidating') + expect(child.invalidating).toBeTruthy() + + await child.finishInvalidate() + expect(invalidateClear).toHaveBeenCalledTimes(2) + expect(child.getState().status).toEqual('updating') + expect(child.updating).toBeTruthy() + + await child.finishUpdate(1) + expect(updateClear).toHaveBeenCalledTimes(1) + expect(child.getState().status).toEqual('idle') + clock.uninstall() + }) + + it('invalidates and retries tick', async () => { + const clock = install({ shouldAdvanceTime: true, advanceTimeDelta: 1 }) + const shouldRetry = mockFn(() => true) + const markAttempt = mockFn(() => {}) + const clear = mockFn(() => {}) + + const root = new TestRootIndexer(0, '', { + tickRetryStrategy: { + shouldRetry, + markAttempt, + timeoutMs: () => 1000, + clear, + }, + }) + + await root.start() + + await root.doTick(1) + await root.finishTick(new Error('test error')) + expect(markAttempt).toHaveBeenCalledTimes(1) + expect(shouldRetry).toHaveBeenCalledTimes(1) + expect(root.getState().status).toEqual('idle') + + await clock.tickAsync(1000) + + expect(root.getState().status).toEqual('ticking') + + await root.finishTick(1) + expect(clear).toHaveBeenCalledTimes(1) + expect(root.getState().status).toEqual('idle') + + clock.uninstall() + }) + }) }) async function waitUntil(predicate: () => boolean): Promise { @@ -78,10 +226,18 @@ async function waitUntil(predicate: () => boolean): Promise { } class TestRootIndexer extends RootIndexer { + public resolveTick: (height: number) => void = () => {} + public rejectTick: (error: unknown) => void = () => {} + dispatchCounter = 0 + ticking = false - constructor(private safeHeight: number, name?: string) { - super(Logger.SILENT.tag(name)) + constructor( + private safeHeight: number, + name?: string, + retryStrategy?: { tickRetryStrategy?: RetryStrategy }, + ) { + super(Logger.SILENT.tag(name), retryStrategy ?? {}) const oldDispatch = Reflect.get(this, 'dispatch') Reflect.set(this, 'dispatch', (action: IndexerAction) => { @@ -98,8 +254,33 @@ class TestRootIndexer extends RootIndexer { await waitUntil(() => this.dispatchCounter > counter) } + async finishTick(result: number | Error): Promise { + await waitUntil(() => this.ticking) + const counter = this.dispatchCounter + if (typeof result === 'number') { + this.resolveTick(result) + } else { + this.rejectTick(result) + } + await waitUntil(() => this.dispatchCounter > counter) + } + override tick(): Promise { - return Promise.resolve(this.safeHeight) + this.ticking = true + + return new Promise((resolve, reject) => { + this.resolveTick = resolve + this.rejectTick = reject + }).finally(() => { + this.ticking = false + }) + } + + override async getSafeHeight(): Promise { + const promise = this.tick() + this.resolveTick(this.safeHeight) + await promise + return this.safeHeight } } @@ -144,8 +325,12 @@ class TestChildIndexer extends ChildIndexer { parents: BaseIndexer[], private safeHeight: number, name?: string, + retryStrategy?: { + invalidateRetryStrategy?: RetryStrategy + updateRetryStrategy?: RetryStrategy + }, ) { - super(Logger.SILENT.tag(name), parents) + super(Logger.SILENT.tag(name), parents, retryStrategy ?? {}) const oldDispatch = Reflect.get(this, 'dispatch') Reflect.set(this, 'dispatch', (action: IndexerAction) => { diff --git a/packages/uif/src/BaseIndexer.ts b/packages/uif/src/BaseIndexer.ts index 3098ade7..bcb6eeeb 100644 --- a/packages/uif/src/BaseIndexer.ts +++ b/packages/uif/src/BaseIndexer.ts @@ -14,10 +14,17 @@ import { UpdateEffect, } from './reducer/types/IndexerEffect' import { IndexerState } from './reducer/types/IndexerState' +import { Retries, RetryStrategy } from './Retries' export abstract class BaseIndexer implements Indexer { private readonly children: Indexer[] = [] + static DEFAULT_RETRY_STRATEGY = Retries.exponentialBackOff({ + initialTimeoutMs: 1000, + maxAttempts: 10, + maxTimeoutMs: 60 * 1000, + }) + /** * Should read the height from the database. It must return a height, so * if database is empty a fallback value has to be chosen. @@ -53,9 +60,19 @@ export abstract class BaseIndexer implements Indexer { private state: IndexerState private started = false - private readonly retryTimeout = 1000 // TODO: make configurable, can be a function - - constructor(protected logger: Logger, public readonly parents: Indexer[]) { + private readonly tickRetryStrategy: RetryStrategy + private readonly updateRetryStrategy: RetryStrategy + private readonly invalidateRetryStrategy: RetryStrategy + + constructor( + protected logger: Logger, + public readonly parents: Indexer[], + opts?: { + tickRetryStrategy?: RetryStrategy + updateRetryStrategy?: RetryStrategy + invalidateRetryStrategy?: RetryStrategy + }, + ) { this.logger = this.logger.for(this) this.state = getInitialState(parents.length) this.parents.forEach((parent) => { @@ -64,6 +81,13 @@ export abstract class BaseIndexer implements Indexer { }) parent.subscribe(this) }) + + this.tickRetryStrategy = + opts?.tickRetryStrategy ?? BaseIndexer.DEFAULT_RETRY_STRATEGY + this.updateRetryStrategy = + opts?.updateRetryStrategy ?? BaseIndexer.DEFAULT_RETRY_STRATEGY + this.invalidateRetryStrategy = + opts?.invalidateRetryStrategy ?? BaseIndexer.DEFAULT_RETRY_STRATEGY } async start(): Promise { @@ -143,24 +167,58 @@ export abstract class BaseIndexer implements Indexer { try { const to = await this.update(from, effect.targetHeight) if (to > effect.targetHeight) { - this.logger.error('Update returned invalid height', { + this.logger.critical('Update returned invalid height', { returned: to, max: effect.targetHeight, }) - this.dispatch({ type: 'UpdateFailed' }) + this.dispatch({ type: 'UpdateFailed', fatal: true }) } else { this.dispatch({ type: 'UpdateSucceeded', from, targetHeight: to }) + this.updateRetryStrategy.clear() } } catch (e) { - this.logger.error('Update failed', e) - this.dispatch({ type: 'UpdateFailed' }) + this.updateRetryStrategy.markAttempt() + const fatal = !this.updateRetryStrategy.shouldRetry() + if (fatal) { + this.logger.critical('Update failed', e) + } else { + this.logger.error('Update failed', e) + } + this.dispatch({ type: 'UpdateFailed', fatal }) } } private executeScheduleRetryUpdate(): void { setTimeout(() => { this.dispatch({ type: 'RetryUpdate' }) - }, this.retryTimeout) + }, this.updateRetryStrategy.timeoutMs()) + } + + private async executeInvalidate(effect: InvalidateEffect): Promise { + this.logger.info('Invalidating', { to: effect.targetHeight }) + try { + await this.invalidate(effect.targetHeight) + this.dispatch({ + type: 'InvalidateSucceeded', + targetHeight: effect.targetHeight, + }) + this.invalidateRetryStrategy.clear() + } catch (e) { + this.invalidateRetryStrategy.markAttempt() + const fatal = !this.invalidateRetryStrategy.shouldRetry() + if (fatal) { + this.logger.critical('Invalidate failed', e) + } else { + this.logger.error('Invalidate failed', e) + } + this.dispatch({ type: 'InvalidateFailed', fatal }) + } + } + + private executeScheduleRetryInvalidate(): void { + setTimeout(() => { + this.dispatch({ type: 'RetryInvalidate' }) + }, this.invalidateRetryStrategy.timeoutMs()) } private executeNotifyReady(effect: NotifyReadyEffect): void { @@ -179,16 +237,23 @@ export abstract class BaseIndexer implements Indexer { try { const safeHeight = await this.tick() this.dispatch({ type: 'TickSucceeded', safeHeight }) + this.tickRetryStrategy.clear() } catch (e) { - this.logger.error('Tick failed', e) - this.dispatch({ type: 'TickFailed' }) + this.tickRetryStrategy.markAttempt() + const fatal = !this.tickRetryStrategy.shouldRetry() + if (fatal) { + this.logger.critical('Tick failed', e) + } else { + this.logger.error('Tick failed', e) + } + this.dispatch({ type: 'TickFailed', fatal }) } } private executeScheduleRetryTick(): void { setTimeout(() => { this.dispatch({ type: 'RetryTick' }) - }, this.retryTimeout) + }, this.tickRetryStrategy.timeoutMs()) } /** @@ -204,28 +269,6 @@ export abstract class BaseIndexer implements Indexer { // #endregion // #region Common methods - private async executeInvalidate(effect: InvalidateEffect): Promise { - this.logger.info('Invalidating', { to: effect.targetHeight }) - try { - await this.invalidate(effect.targetHeight) - this.dispatch({ - type: 'InvalidateSucceeded', - targetHeight: effect.targetHeight, - }) - } catch (e) { - this.logger.error('Invalidate failed', e) - this.dispatch({ - type: 'InvalidateFailed', - }) - } - } - - private executeScheduleRetryInvalidate(): void { - setTimeout(() => { - this.dispatch({ type: 'RetryInvalidate' }) - }, this.retryTimeout) - } - private async executeSetSafeHeight( effect: SetSafeHeightEffect, ): Promise { @@ -240,8 +283,8 @@ export abstract class BaseIndexer implements Indexer { } export abstract class RootIndexer extends BaseIndexer { - constructor(logger: Logger) { - super(logger, []) + constructor(logger: Logger, opts?: { tickRetryStrategy?: RetryStrategy }) { + super(logger, [], opts) } // eslint-disable-next-line @typescript-eslint/require-await @@ -264,6 +307,18 @@ export abstract class RootIndexer extends BaseIndexer { } export abstract class ChildIndexer extends BaseIndexer { + // eslint-disable-next-line @typescript-eslint/no-useless-constructor + constructor( + logger: Logger, + parents: Indexer[], + opts?: { + updateRetryStrategy?: RetryStrategy + invalidateRetryStrategy?: RetryStrategy + }, + ) { + super(logger, parents, opts) + } + // eslint-disable-next-line @typescript-eslint/require-await override async tick(): Promise { throw new Error('ChildIndexer cannot tick') diff --git a/packages/uif/src/Retries.ts b/packages/uif/src/Retries.ts new file mode 100644 index 00000000..0f2cb1b7 --- /dev/null +++ b/packages/uif/src/Retries.ts @@ -0,0 +1,51 @@ +import assert from 'node:assert' + +export interface RetryStrategy { + /** Returns true if the operation should be retried */ + shouldRetry: () => boolean + /** Increments the number of attempts */ + markAttempt: () => void + /** Returns the timeout in milliseconds */ + timeoutMs: () => number + /** Resets the number of attempts */ + clear: () => void +} + +interface ExponentialBackOffOpts { + initialTimeoutMs: number + maxAttempts: number + maxTimeoutMs?: number +} + +/** + * @param opts.maxAttempts - use Infinity for indefinite retries + * @param opts.initialTimeoutMs - initial timeout by 2**attempts gives the timeout + * @param opts.maxTimeoutMs - maximum timeout between retries (default: Infinity) + */ +function exponentialBackOff(opts: ExponentialBackOffOpts): RetryStrategy { + let attempts = 0 + const maxAttempts = opts.maxAttempts + assert(maxAttempts > 0) + const maxTimeoutMs = opts.maxTimeoutMs ?? Infinity + assert(maxTimeoutMs > 0) + + return { + shouldRetry: () => { + return attempts <= maxAttempts + }, + markAttempt: () => { + attempts++ + }, + timeoutMs: () => { + const timeout = Math.pow(2, attempts - 1) * opts.initialTimeoutMs + return Math.min(timeout, maxTimeoutMs) + }, + clear: () => { + attempts = 0 + }, + } +} + +export const Retries = { + exponentialBackOff, +} diff --git a/packages/uif/src/index.ts b/packages/uif/src/index.ts index 034524f8..cdd00d06 100644 --- a/packages/uif/src/index.ts +++ b/packages/uif/src/index.ts @@ -1,2 +1,3 @@ export * from './BaseIndexer' export * from './Indexer' +export * from './Retries' diff --git a/yarn.lock b/yarn.lock index 6fa7807e..9867c45c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -475,10 +475,10 @@ dependencies: type-detect "4.0.8" -"@sinonjs/fake-timers@^10.2.0": - version "10.2.0" - resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-10.2.0.tgz#b3e322a34c5f26e3184e7f6115695f299c1b1194" - integrity sha512-OPwQlEdg40HAj5KNF8WW6q2KG4Z+cBCZb3m4ninfTZKaBmbIJodviQsDBoYMPHkOyJJMHnOJo5j2+LKDOhOACg== +"@sinonjs/fake-timers@^11.1.0": + version "11.1.0" + resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-11.1.0.tgz#5ad7b44514a61bbd04a3ddec863e21edd6efc2da" + integrity sha512-pUBaWhXoa9N0R/LeYKLqkrN9mqN3jwKBeMfbvlRtHUzLmk55o+0swncIuZBcSH/PpXDttRf/AcPF22pknAzORQ== dependencies: "@sinonjs/commons" "^3.0.0"