From 9937eec5d81364303b906a84098e6bbf8e4981d2 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Wed, 19 Jul 2023 14:48:50 +0200 Subject: [PATCH 1/7] Implement retry mechanism for HttpExtractor --- .../src/lib/blocks/execution-result.ts | 2 +- .../std/exec/src/http-extractor-executor.ts | 40 ++++++++-- .../std/lang/src/http-extractor-meta-inf.ts | 73 +++++++++++++++++++ 3 files changed, 108 insertions(+), 7 deletions(-) diff --git a/libs/execution/src/lib/blocks/execution-result.ts b/libs/execution/src/lib/blocks/execution-result.ts index 5a4737599..f0f679ec3 100644 --- a/libs/execution/src/lib/blocks/execution-result.ts +++ b/libs/execution/src/lib/blocks/execution-result.ts @@ -5,7 +5,7 @@ import * as E from 'fp-ts/lib/Either'; import { AstNode, DiagnosticInfo } from 'langium'; -interface ExecutionErrorDetails { +export interface ExecutionErrorDetails { message: string; diagnostic: DiagnosticInfo; } diff --git a/libs/extensions/std/exec/src/http-extractor-executor.ts b/libs/extensions/std/exec/src/http-extractor-executor.ts index ba092078f..6927c6fb0 100644 --- a/libs/extensions/std/exec/src/http-extractor-executor.ts +++ b/libs/extensions/std/exec/src/http-extractor-executor.ts @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import { strict as assert } from 'assert'; import * as http from 'http'; import * as https from 'https'; import * as path from 'path'; @@ -12,12 +13,14 @@ import { BinaryFile, BlockExecutorClass, ExecutionContext, + ExecutionErrorDetails, FileExtension, MimeType, None, implementsStatic, } from '@jvalue/jayvee-execution'; import { IOType, PrimitiveValuetypes } from '@jvalue/jayvee-language-server'; +import { AstNode } from 'langium'; import { inferFileExtensionFromContentTypeString, @@ -43,14 +46,39 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< context: ExecutionContext, ): Promise> { const url = context.getPropertyValue('url', PrimitiveValuetypes.Text); - - const file = await this.fetchRawDataAsFile(url, context); - - if (R.isErr(file)) { - return file; + const retries = context.getPropertyValue( + 'retries', + PrimitiveValuetypes.Integer, + ); + const retryBackoff = context.getPropertyValue( + 'retryBackoff', + PrimitiveValuetypes.Integer, + ); + + let failure: ExecutionErrorDetails | undefined; + assert(retries >= 0); // loop executes at least once + for (let attempt = 0; attempt <= retries; ++attempt) { + const isLastAttempt = attempt === retries; + const file = await this.fetchRawDataAsFile(url, context); + + if (R.isOk(file)) { + return R.ok(file.right); + } + + failure = file.left; + + if (!isLastAttempt) { + context.logger.logDebug(failure.message); + context.logger.logDebug( + `Waiting ${retryBackoff}ms before trying again...`, + ); + await new Promise((p) => setTimeout(p, retryBackoff)); + continue; + } } - return R.ok(file.right); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return R.err(failure!); } private fetchRawDataAsFile( diff --git a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts index 8b5b8bf89..fe874c8b8 100644 --- a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts +++ b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts @@ -6,6 +6,7 @@ import { BlockMetaInformation, IOType, PrimitiveValuetypes, + evaluatePropertyValue, } from '@jvalue/jayvee-language-server'; export class HttpExtractorMetaInformation extends BlockMetaInformation { @@ -28,7 +29,79 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { ], }, }, + retries: { + type: PrimitiveValuetypes.Integer, + defaultValue: 0, + docs: { + description: + 'Configures how many retries should be executed after a failure fetching the data.', + examples: [ + { + code: 'retries: 3', + description: + 'Executes up to 3 retries if the original retry fails (so in total max. 4 requests).', + }, + ], + }, + validation: (property, validationContext, evaluationContext) => { + const encodingValue = evaluatePropertyValue( + property, + evaluationContext, + PrimitiveValuetypes.Integer, + ); + if (encodingValue === undefined) { + return; + } + + if (encodingValue < 0) { + validationContext.accept( + 'error', + 'Only not negative integers allowed', + { + node: property, + property: 'value', + }, + ); + } + }, + }, + retryBackoff: { + type: PrimitiveValuetypes.Integer, + defaultValue: 1000, + docs: { + description: + 'Configures the wait time in milliseconds before executing a retry.', + examples: [ + { + code: 'retryBackoff: 5000', + description: 'Waits 5s (5000 ms) before executing a retry.', + }, + ], + }, + validation: (property, validationContext, evaluationContext) => { + const encodingValue = evaluatePropertyValue( + property, + evaluationContext, + PrimitiveValuetypes.Integer, + ); + if (encodingValue === undefined) { + return; + } + + if (encodingValue < 0) { + validationContext.accept( + 'error', + 'Only not negative integers allowed', + { + node: property, + property: 'value', + }, + ); + } + }, + }, }, + // Input type: IOType.NONE, From ccc0ebd09d5ee85be4ba293f903eb3487a2d407e Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 10:56:55 +0200 Subject: [PATCH 2/7] Rename retryBackoff to retryBackoffMilliseconds and introduce min value --- libs/extensions/std/exec/src/http-extractor-executor.ts | 2 +- libs/extensions/std/lang/src/http-extractor-meta-inf.ts | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/libs/extensions/std/exec/src/http-extractor-executor.ts b/libs/extensions/std/exec/src/http-extractor-executor.ts index 6927c6fb0..4a9668e50 100644 --- a/libs/extensions/std/exec/src/http-extractor-executor.ts +++ b/libs/extensions/std/exec/src/http-extractor-executor.ts @@ -51,7 +51,7 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< PrimitiveValuetypes.Integer, ); const retryBackoff = context.getPropertyValue( - 'retryBackoff', + 'retryBackoffMilliseconds', PrimitiveValuetypes.Integer, ); diff --git a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts index fe874c8b8..17633cecc 100644 --- a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts +++ b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts @@ -65,7 +65,7 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { } }, }, - retryBackoff: { + retryBackoffMilliseconds: { type: PrimitiveValuetypes.Integer, defaultValue: 1000, docs: { @@ -79,6 +79,8 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { ], }, validation: (property, validationContext, evaluationContext) => { + const minBockoffValue = 1000; + const encodingValue = evaluatePropertyValue( property, evaluationContext, @@ -88,10 +90,10 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { return; } - if (encodingValue < 0) { + if (encodingValue < minBockoffValue) { validationContext.accept( 'error', - 'Only not negative integers allowed', + `Only integers larger or equal to ${minBockoffValue} are allowed`, { node: property, property: 'value', From d24763044fccdc81ba05cc27efa3cef38e4d0cf6 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 11:21:21 +0200 Subject: [PATCH 3/7] Implement BackoffStrategy --- .../std/exec/src/http-extractor-executor.ts | 15 ++++++- .../exec/src/util/backoff-strategy.spec.ts | 41 +++++++++++++++++++ .../std/exec/src/util/backoff-strategy.ts | 27 ++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 libs/extensions/std/exec/src/util/backoff-strategy.spec.ts create mode 100644 libs/extensions/std/exec/src/util/backoff-strategy.ts diff --git a/libs/extensions/std/exec/src/http-extractor-executor.ts b/libs/extensions/std/exec/src/http-extractor-executor.ts index 4a9668e50..f431871b2 100644 --- a/libs/extensions/std/exec/src/http-extractor-executor.ts +++ b/libs/extensions/std/exec/src/http-extractor-executor.ts @@ -27,6 +27,10 @@ import { inferFileExtensionFromFileExtensionString, inferMimeTypeFromContentTypeString, } from './file-util'; +import { + BackoffStrategy, + LinearBackoffStrategy, +} from './util/backoff-strategy'; type HttpGetFunction = typeof http.get; @@ -54,6 +58,9 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< 'retryBackoffMilliseconds', PrimitiveValuetypes.Integer, ); + const backoffStrategy: BackoffStrategy = new LinearBackoffStrategy( + retryBackoff, + ); let failure: ExecutionErrorDetails | undefined; assert(retries >= 0); // loop executes at least once @@ -69,10 +76,14 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< if (!isLastAttempt) { context.logger.logDebug(failure.message); + + const currentBackoff = backoffStrategy.getBackoffMilliseconds( + attempt + 1, + ); context.logger.logDebug( - `Waiting ${retryBackoff}ms before trying again...`, + `Waiting ${currentBackoff}ms before trying again...`, ); - await new Promise((p) => setTimeout(p, retryBackoff)); + await new Promise((p) => setTimeout(p, currentBackoff)); continue; } } diff --git a/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts b/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts new file mode 100644 index 000000000..a3312f90b --- /dev/null +++ b/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +import { + BackoffStrategy, + ExponentialBackoffStrategy, + LinearBackoffStrategy, +} from './backoff-strategy'; + +describe('BackoffStrategy', () => { + describe('ExponentialBackoffStrategy', () => { + describe('getBackoffMilliseconds', () => { + it('should calculate exponential backoff correctly with 5 retries', () => { + const backoffStrategy: BackoffStrategy = new ExponentialBackoffStrategy( + 2, + ); + + expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(4); + expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(8); + expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(16); + expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(32); + }); + }); + }); + + describe('LinearBackoffStrategy', () => { + describe('getNextBackoffMilliseconds', () => { + it('should calculate exponential backoff correctly with 5 retries', () => { + const backoffStrategy: BackoffStrategy = new LinearBackoffStrategy(2); + + expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(2); + }); + }); + }); +}); diff --git a/libs/extensions/std/exec/src/util/backoff-strategy.ts b/libs/extensions/std/exec/src/util/backoff-strategy.ts new file mode 100644 index 000000000..17e7bd970 --- /dev/null +++ b/libs/extensions/std/exec/src/util/backoff-strategy.ts @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +export interface BackoffStrategy { + /** + * Calculates the next backoff interval in milliseconds. + * @param retry the number of the current retry, starts counting with 1 + */ + getBackoffMilliseconds(retry: number): number; +} + +export class ExponentialBackoffStrategy implements BackoffStrategy { + constructor(private initialBackoffMilliseconds: number) {} + + getBackoffMilliseconds(retry: number): number { + return Math.pow(this.initialBackoffMilliseconds, retry); + } +} + +export class LinearBackoffStrategy implements BackoffStrategy { + constructor(private backoffMilliseconds: number) {} + + getBackoffMilliseconds(): number { + return this.backoffMilliseconds; + } +} From 8a69406b997feee2b53c05c125fa7929c9ef2483 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 11:49:00 +0200 Subject: [PATCH 4/7] Log success only on success not on end --- libs/extensions/std/exec/src/http-extractor-executor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/extensions/std/exec/src/http-extractor-executor.ts b/libs/extensions/std/exec/src/http-extractor-executor.ts index f431871b2..1ebcd1627 100644 --- a/libs/extensions/std/exec/src/http-extractor-executor.ts +++ b/libs/extensions/std/exec/src/http-extractor-executor.ts @@ -69,6 +69,7 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< const file = await this.fetchRawDataAsFile(url, context); if (R.isOk(file)) { + context.logger.logDebug(`Successfully fetched raw data`); return R.ok(file.right); } @@ -130,7 +131,6 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< // When all data is downloaded, create file response.on('end', () => { - context.logger.logDebug(`Successfully fetched raw data`); response.headers; // Infer Mimetype from HTTP-Header, if not inferrable, then default to application/octet-stream From e1e284aa3fc0b64a2155415adc17a45ae5da90a3 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 12:08:25 +0200 Subject: [PATCH 5/7] Add prop retryBackoffStrategy to choose between linear and exponential backoff strategies --- .../std/exec/src/http-extractor-executor.ts | 18 +++++--- .../std/exec/src/util/backoff-strategy.ts | 29 +++++++++++- .../std/lang/src/http-extractor-meta-inf.ts | 45 +++++++++++++++++++ 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/libs/extensions/std/exec/src/http-extractor-executor.ts b/libs/extensions/std/exec/src/http-extractor-executor.ts index 1ebcd1627..b7d1fcc7e 100644 --- a/libs/extensions/std/exec/src/http-extractor-executor.ts +++ b/libs/extensions/std/exec/src/http-extractor-executor.ts @@ -28,8 +28,8 @@ import { inferMimeTypeFromContentTypeString, } from './file-util'; import { - BackoffStrategy, - LinearBackoffStrategy, + createBackoffStrategy, + isBackoffStrategyHandle, } from './util/backoff-strategy'; type HttpGetFunction = typeof http.get; @@ -54,16 +54,22 @@ export class HttpExtractorExecutor extends AbstractBlockExecutor< 'retries', PrimitiveValuetypes.Integer, ); - const retryBackoff = context.getPropertyValue( + assert(retries >= 0); // loop executes at least once + const retryBackoffMilliseconds = context.getPropertyValue( 'retryBackoffMilliseconds', PrimitiveValuetypes.Integer, ); - const backoffStrategy: BackoffStrategy = new LinearBackoffStrategy( - retryBackoff, + const retryBackoffStrategy = context.getPropertyValue( + 'retryBackoffStrategy', + PrimitiveValuetypes.Text, + ); + assert(isBackoffStrategyHandle(retryBackoffStrategy)); + const backoffStrategy = createBackoffStrategy( + retryBackoffStrategy, + retryBackoffMilliseconds, ); let failure: ExecutionErrorDetails | undefined; - assert(retries >= 0); // loop executes at least once for (let attempt = 0; attempt <= retries; ++attempt) { const isLastAttempt = attempt === retries; const file = await this.fetchRawDataAsFile(url, context); diff --git a/libs/extensions/std/exec/src/util/backoff-strategy.ts b/libs/extensions/std/exec/src/util/backoff-strategy.ts index 17e7bd970..0dfcb20dc 100644 --- a/libs/extensions/std/exec/src/util/backoff-strategy.ts +++ b/libs/extensions/std/exec/src/util/backoff-strategy.ts @@ -2,22 +2,47 @@ // // SPDX-License-Identifier: AGPL-3.0-only +export type BackoffStrategyHandle = 'exponential' | 'linear'; +export function isBackoffStrategyHandle( + v: unknown, +): v is BackoffStrategyHandle { + return v === 'exponential' || v === 'linear'; +} + +export function createBackoffStrategy( + handle: BackoffStrategyHandle, + backoffMilliseconds: number, +): BackoffStrategy { + if (handle === 'linear') { + return new LinearBackoffStrategy(backoffMilliseconds); + } + return new ExponentialBackoffStrategy(backoffMilliseconds); +} + export interface BackoffStrategy { /** - * Calculates the next backoff interval in milliseconds. + * Calculates the backoff interval in milliseconds. * @param retry the number of the current retry, starts counting with 1 */ getBackoffMilliseconds(retry: number): number; } +/** + * Strategy for exponential backoffs. + * Uses seconds as unit for calculating the exponent. + */ export class ExponentialBackoffStrategy implements BackoffStrategy { constructor(private initialBackoffMilliseconds: number) {} getBackoffMilliseconds(retry: number): number { - return Math.pow(this.initialBackoffMilliseconds, retry); + const initialBackoffSeconds = this.initialBackoffMilliseconds / 1000; + return Math.pow(initialBackoffSeconds, retry) * 1000; } } +/** + * Strategy for linear backoffs. + */ export class LinearBackoffStrategy implements BackoffStrategy { constructor(private backoffMilliseconds: number) {} diff --git a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts index 17633cecc..bf204e3f3 100644 --- a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts +++ b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts @@ -102,6 +102,51 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { } }, }, + retryBackoffStrategy: { + type: PrimitiveValuetypes.Text, + defaultValue: 'exponential', + docs: { + description: + 'Configures the wait strategy before executing a retry. Can have values "exponential" or "linear".', + examples: [ + { + code: 'retryBackoffStrategy: "linear"', + description: + 'Waits always the same amount of time before executing a retry.', + }, + { + code: 'retryBackoffStrategy: "exponential"', + description: + 'Exponentially increases the wait time before executing a retry.', + }, + ], + }, + validation: (property, validationContext, evaluationContext) => { + const allowedValues = ['exponential', 'linear']; + + const encodingValue = evaluatePropertyValue( + property, + evaluationContext, + PrimitiveValuetypes.Text, + ); + if (encodingValue === undefined) { + return; + } + + if (!allowedValues.includes(encodingValue)) { + validationContext.accept( + 'error', + `Only the following values are allowed: ${allowedValues + .map((v) => `"${v}"`) + .join(', ')}`, + { + node: property, + property: 'value', + }, + ); + } + }, + }, }, // Input type: From 0eb790cc35d1c88820b657d4ac55b6eb3d9a2727 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 12:11:00 +0200 Subject: [PATCH 6/7] Set default of prop retryBackoffMilliseconds to 2000 --- libs/extensions/std/lang/src/http-extractor-meta-inf.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts index bf204e3f3..26dcc7321 100644 --- a/libs/extensions/std/lang/src/http-extractor-meta-inf.ts +++ b/libs/extensions/std/lang/src/http-extractor-meta-inf.ts @@ -67,7 +67,7 @@ export class HttpExtractorMetaInformation extends BlockMetaInformation { }, retryBackoffMilliseconds: { type: PrimitiveValuetypes.Integer, - defaultValue: 1000, + defaultValue: 2000, docs: { description: 'Configures the wait time in milliseconds before executing a retry.', From 4678048bdf7173b30cea9563cca72db8e45bc4b1 Mon Sep 17 00:00:00 2001 From: Georg Schwarz Date: Thu, 20 Jul 2023 13:40:00 +0200 Subject: [PATCH 7/7] Fix tests to use millisecond values in range of allowed spec --- .../exec/src/util/backoff-strategy.spec.ts | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts b/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts index a3312f90b..04d73d917 100644 --- a/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts +++ b/libs/extensions/std/exec/src/util/backoff-strategy.spec.ts @@ -13,14 +13,14 @@ describe('BackoffStrategy', () => { describe('getBackoffMilliseconds', () => { it('should calculate exponential backoff correctly with 5 retries', () => { const backoffStrategy: BackoffStrategy = new ExponentialBackoffStrategy( - 2, + 2000, ); - expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2); - expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(4); - expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(8); - expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(16); - expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(32); + expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2000); + expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(4000); + expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(8000); + expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(16000); + expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(32000); }); }); }); @@ -28,13 +28,15 @@ describe('BackoffStrategy', () => { describe('LinearBackoffStrategy', () => { describe('getNextBackoffMilliseconds', () => { it('should calculate exponential backoff correctly with 5 retries', () => { - const backoffStrategy: BackoffStrategy = new LinearBackoffStrategy(2); + const backoffStrategy: BackoffStrategy = new LinearBackoffStrategy( + 2000, + ); - expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2); - expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(2); - expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(2); - expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(2); - expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(2); + expect(backoffStrategy.getBackoffMilliseconds(1)).toEqual(2000); + expect(backoffStrategy.getBackoffMilliseconds(2)).toEqual(2000); + expect(backoffStrategy.getBackoffMilliseconds(3)).toEqual(2000); + expect(backoffStrategy.getBackoffMilliseconds(4)).toEqual(2000); + expect(backoffStrategy.getBackoffMilliseconds(5)).toEqual(2000); }); }); });