From 2fc5cae38554901a21435ef087036a062c9d1632 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Fri, 14 Apr 2023 23:41:17 +0200 Subject: [PATCH] fix: fix worker choice strategy options handling MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- .eslintrc.js | 3 +- CHANGELOG.md | 1 + .../abstract-worker-choice-strategy.ts | 6 +- .../fair-share-worker-choice-strategy.ts | 14 ++++- .../less-busy-worker-choice-strategy.ts | 14 ++++- .../less-used-worker-choice-strategy.ts | 16 +++++- .../round-robin-worker-choice-strategy.ts | 16 +++++- ...hted-round-robin-worker-choice-strategy.ts | 11 ++-- .../worker-choice-strategy-context.test.js | 57 +++++++++++++++++++ 9 files changed, 123 insertions(+), 15 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index cc27eee7..0a343d6b 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -72,7 +72,8 @@ module.exports = defineConfig({ 'unlink', 'unregister', 'utf8', - 'workerpool' + 'workerpool', + 'wwr' ], skipIfMatch: ['^@.*', '^plugin:.*'] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 5efd1b84..5d599b16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix worker function type definition and validation. +- Fix worker choice strategy options handling. ## [2.4.8] - 2023-04-12 diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts index ce7f9589..77b44f6e 100644 --- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts @@ -38,14 +38,14 @@ export abstract class AbstractWorkerChoiceStrategy< protected readonly pool: IPool, protected readonly opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { - this.checkOptions(this.opts) this.isDynamicPool = this.pool.type === PoolType.DYNAMIC this.choose.bind(this) } - private checkOptions (opts: WorkerChoiceStrategyOptions): void { + protected checkOptions (opts: WorkerChoiceStrategyOptions): void { if (this.requiredStatistics.avgRunTime && opts.medRunTime === true) { - this.requiredStatistics.medRunTime = true + this.requiredStatistics.avgRunTime = false + this.requiredStatistics.medRunTime = opts.medRunTime as boolean } } diff --git a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts index 3f4e9524..81944f7a 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -1,8 +1,11 @@ +import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils' +import type { IPool } from '../pool' import type { IWorker } from '../worker' import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy' import type { IWorkerChoiceStrategy, - RequiredStatistics + RequiredStatistics, + WorkerChoiceStrategyOptions } from './selection-strategies-types' /** @@ -43,6 +46,15 @@ export class FairShareWorkerChoiceStrategy< WorkerVirtualTaskTimestamp > = new Map() + /** @inheritDoc */ + public constructor ( + pool: IPool, + opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS + ) { + super(pool, opts) + this.checkOptions(opts) + } + /** @inheritDoc */ public reset (): boolean { this.workerLastVirtualTaskTimestamp.clear() diff --git a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts index 12e87ff2..708c491f 100644 --- a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts @@ -1,8 +1,11 @@ +import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils' +import type { IPool } from '../pool' import type { IWorker } from '../worker' import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy' import type { IWorkerChoiceStrategy, - RequiredStatistics + RequiredStatistics, + WorkerChoiceStrategyOptions } from './selection-strategies-types' /** @@ -26,6 +29,15 @@ export class LessBusyWorkerChoiceStrategy< medRunTime: false } + /** @inheritDoc */ + public constructor ( + pool: IPool, + opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS + ) { + super(pool, opts) + this.checkOptions(opts) + } + /** @inheritDoc */ public reset (): boolean { return true diff --git a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts index 166de706..1138df95 100644 --- a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts @@ -1,6 +1,11 @@ +import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils' +import type { IPool } from '../pool' import type { IWorker } from '../worker' import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy' -import type { IWorkerChoiceStrategy } from './selection-strategies-types' +import type { + IWorkerChoiceStrategy, + WorkerChoiceStrategyOptions +} from './selection-strategies-types' /** * Selects the less used worker. @@ -16,6 +21,15 @@ export class LessUsedWorkerChoiceStrategy< > extends AbstractWorkerChoiceStrategy implements IWorkerChoiceStrategy { + /** @inheritDoc */ + public constructor ( + pool: IPool, + opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS + ) { + super(pool, opts) + this.checkOptions(opts) + } + /** @inheritDoc */ public reset (): boolean { return true diff --git a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts index 80e958a4..107e9639 100644 --- a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts @@ -1,6 +1,11 @@ +import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils' +import type { IPool } from '../pool' import type { IWorker } from '../worker' import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy' -import type { IWorkerChoiceStrategy } from './selection-strategies-types' +import type { + IWorkerChoiceStrategy, + WorkerChoiceStrategyOptions +} from './selection-strategies-types' /** * Selects the next worker in a round robin fashion. @@ -21,6 +26,15 @@ export class RoundRobinWorkerChoiceStrategy< */ private nextWorkerNodeId: number = 0 + /** @inheritDoc */ + public constructor ( + pool: IPool, + opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS + ) { + super(pool, opts) + this.checkOptions(opts) + } + /** @inheritDoc */ public reset (): boolean { this.nextWorkerNodeId = 0 diff --git a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts index 665022c7..dc2e5df3 100644 --- a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts @@ -7,6 +7,7 @@ import type { WorkerChoiceStrategyOptions } from './selection-strategies-types' import type { IPool } from '../pool' +import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils' /** * Virtual task runtime. @@ -54,17 +55,13 @@ export class WeightedRoundRobinWorkerChoiceStrategy< TaskRunTime >() - /** - * Constructs a worker choice strategy that selects with a weighted round robin scheduling algorithm. - * - * @param pool - The pool instance. - * @param opts - The worker choice strategy options. - */ + /** @inheritDoc */ public constructor ( pool: IPool, - opts?: WorkerChoiceStrategyOptions + opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) + this.checkOptions(opts) this.defaultWorkerWeight = this.computeWorkerWeight() this.initWorkersTaskRunTime() } diff --git a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js index 23d954c9..7e616fc7 100644 --- a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js +++ b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js @@ -336,4 +336,61 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategy ) }) + + it.only('Verify that worker choice strategy options enable median run time pool statistics', () => { + const wwrWorkerChoiceStrategy = WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN + let workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool, + wwrWorkerChoiceStrategy, + { + medRunTime: true + } + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe( + false + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe( + true + ) + workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + dynamicPool, + wwrWorkerChoiceStrategy, + { + medRunTime: true + } + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe( + false + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe( + true + ) + const fsWorkerChoiceStrategy = WorkerChoiceStrategies.FAIR_SHARE + workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool, + fsWorkerChoiceStrategy, + { + medRunTime: true + } + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe( + false + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe( + true + ) + workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + dynamicPool, + fsWorkerChoiceStrategy, + { + medRunTime: true + } + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe( + false + ) + expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe( + true + ) + }) }) -- 2.34.1