From 3300e7bcb155358c2cc1eed6e4ac11581457616f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 5 Apr 2023 17:06:56 +0200 Subject: [PATCH] refactor: apply stricter strategy design pattern requirements on worker choice code MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/pools/abstract-pool.ts | 1 + .../abstract-worker-choice-strategy.ts | 4 ++-- .../fair-share-worker-choice-strategy.ts | 2 +- .../less-busy-worker-choice-strategy.ts | 2 +- .../less-used-worker-choice-strategy.ts | 2 +- .../round-robin-worker-choice-strategy.ts | 2 +- .../selection-strategies-types.ts | 16 +++++++++++++++- .../selection-strategies-utils.ts | 2 +- ...eighted-round-robin-worker-choice-strategy.ts | 2 +- .../worker-choice-strategy-context.ts | 16 ++++++++++------ .../worker-choice-strategy-context.test.js | 10 ++++++++++ 11 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index fa1a8ba1..c1e6ddc0 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -179,6 +179,7 @@ export abstract class AbstractPool< }) } this.workerChoiceStrategyContext.setWorkerChoiceStrategy( + this, workerChoiceStrategy ) } diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts index aa370fe4..44e7e3f8 100644 --- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts @@ -17,7 +17,7 @@ export abstract class AbstractWorkerChoiceStrategy< Worker extends IPoolWorker, Data, Response -> implements IWorkerChoiceStrategy { +> implements IWorkerChoiceStrategy { /** {@inheritDoc} */ public readonly isDynamicPool: boolean /** {@inheritDoc} */ @@ -32,7 +32,7 @@ export abstract class AbstractWorkerChoiceStrategy< * @param pool - The pool instance. */ public constructor ( - protected readonly pool: IPoolInternal + public readonly pool: IPoolInternal ) { this.isDynamicPool = this.pool.type === PoolType.DYNAMIC this.choose.bind(this) 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 7ef28748..1fe68ffa 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -27,7 +27,7 @@ export class FairShareWorkerChoiceStrategy< Response > extends AbstractWorkerChoiceStrategy - implements IWorkerChoiceStrategy { + implements IWorkerChoiceStrategy { /** {@inheritDoc} */ public readonly requiredStatistics: RequiredStatistics = { runTime: true, 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 1d8bbaab..bddae82d 100644 --- a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts @@ -18,7 +18,7 @@ export class LessBusyWorkerChoiceStrategy< Response > extends AbstractWorkerChoiceStrategy - implements IWorkerChoiceStrategy { + implements IWorkerChoiceStrategy { /** {@inheritDoc} */ public readonly requiredStatistics: RequiredStatistics = { runTime: 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 3eeb8329..f0632db8 100644 --- a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts @@ -15,7 +15,7 @@ export class LessUsedWorkerChoiceStrategy< Response > extends AbstractWorkerChoiceStrategy - implements IWorkerChoiceStrategy { + implements IWorkerChoiceStrategy { /** {@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 1499db94..5ee21920 100644 --- a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts @@ -15,7 +15,7 @@ export class RoundRobinWorkerChoiceStrategy< Response > extends AbstractWorkerChoiceStrategy - implements IWorkerChoiceStrategy { + implements IWorkerChoiceStrategy { /** * Id of the next worker. */ diff --git a/src/pools/selection-strategies/selection-strategies-types.ts b/src/pools/selection-strategies/selection-strategies-types.ts index ed6b0103..4fb2f358 100644 --- a/src/pools/selection-strategies/selection-strategies-types.ts +++ b/src/pools/selection-strategies/selection-strategies-types.ts @@ -1,3 +1,6 @@ +import type { IPoolInternal } from '../pool-internal' +import type { IPoolWorker } from '../pool-worker' + /** * Enumeration of worker choice strategies. */ @@ -40,13 +43,24 @@ export interface RequiredStatistics { /** * Worker choice strategy interface. */ -export interface IWorkerChoiceStrategy { +export interface IWorkerChoiceStrategy< + Worker extends IPoolWorker, + Data = unknown, + Response = unknown +> { + /** + * The pool instance. + * @readonly + */ + readonly pool: IPoolInternal /** * Is the pool attached to the strategy dynamic?. + * @readonly */ readonly isDynamicPool: boolean /** * Required pool tasks usage statistics. + * @readonly */ readonly requiredStatistics: RequiredStatistics /** diff --git a/src/pools/selection-strategies/selection-strategies-utils.ts b/src/pools/selection-strategies/selection-strategies-utils.ts index 10c93335..3db64566 100644 --- a/src/pools/selection-strategies/selection-strategies-utils.ts +++ b/src/pools/selection-strategies/selection-strategies-utils.ts @@ -25,7 +25,7 @@ export function getWorkerChoiceStrategy< > ( pool: IPoolInternal, workerChoiceStrategy: WorkerChoiceStrategy = WorkerChoiceStrategies.ROUND_ROBIN -): IWorkerChoiceStrategy { +): IWorkerChoiceStrategy { switch (workerChoiceStrategy) { case WorkerChoiceStrategies.ROUND_ROBIN: return new RoundRobinWorkerChoiceStrategy(pool) 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 3709cce4..a8848f14 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 @@ -29,7 +29,7 @@ export class WeightedRoundRobinWorkerChoiceStrategy< Response > extends AbstractWorkerChoiceStrategy - implements IWorkerChoiceStrategy { + implements IWorkerChoiceStrategy { /** {@inheritDoc} */ public readonly requiredStatistics: RequiredStatistics = { runTime: true, diff --git a/src/pools/selection-strategies/worker-choice-strategy-context.ts b/src/pools/selection-strategies/worker-choice-strategy-context.ts index 8041e2f6..41b515b8 100644 --- a/src/pools/selection-strategies/worker-choice-strategy-context.ts +++ b/src/pools/selection-strategies/worker-choice-strategy-context.ts @@ -20,7 +20,7 @@ export class WorkerChoiceStrategyContext< Data, Response > { - private workerChoiceStrategy!: IWorkerChoiceStrategy + private workerChoiceStrategy: IWorkerChoiceStrategy /** * Worker choice strategy context constructor. @@ -30,12 +30,15 @@ export class WorkerChoiceStrategyContext< * @param workerChoiceStrategy - The worker choice strategy. */ public constructor ( - private readonly pool: IPoolInternal, + pool: IPoolInternal, private readonly createWorkerCallback: () => number, workerChoiceStrategy: WorkerChoiceStrategy = WorkerChoiceStrategies.ROUND_ROBIN ) { this.execute.bind(this) - this.setWorkerChoiceStrategy(workerChoiceStrategy) + this.workerChoiceStrategy = getWorkerChoiceStrategy( + pool, + workerChoiceStrategy + ) } /** @@ -53,11 +56,12 @@ export class WorkerChoiceStrategyContext< * @param workerChoiceStrategy - The worker choice strategy to set. */ public setWorkerChoiceStrategy ( + pool: IPoolInternal, workerChoiceStrategy: WorkerChoiceStrategy ): void { this.workerChoiceStrategy?.reset() this.workerChoiceStrategy = getWorkerChoiceStrategy( - this.pool, + pool, workerChoiceStrategy ) } @@ -70,8 +74,8 @@ export class WorkerChoiceStrategyContext< public execute (): number { if ( this.workerChoiceStrategy.isDynamicPool && - !this.pool.full && - this.pool.findFreeWorkerKey() === -1 + !this.workerChoiceStrategy.pool.full && + this.workerChoiceStrategy.pool.findFreeWorkerKey() === -1 ) { return this.createWorkerCallback() } 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 3f65fe82..d1cad6a9 100644 --- a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js +++ b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js @@ -91,6 +91,7 @@ describe('Worker choice strategy context test suite', () => { fixedPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + fixedPool, WorkerChoiceStrategies.ROUND_ROBIN ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -103,6 +104,7 @@ describe('Worker choice strategy context test suite', () => { dynamicPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + dynamicPool, WorkerChoiceStrategies.ROUND_ROBIN ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -115,6 +117,7 @@ describe('Worker choice strategy context test suite', () => { fixedPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + fixedPool, WorkerChoiceStrategies.LESS_USED ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -127,6 +130,7 @@ describe('Worker choice strategy context test suite', () => { dynamicPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + dynamicPool, WorkerChoiceStrategies.LESS_USED ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -139,6 +143,7 @@ describe('Worker choice strategy context test suite', () => { fixedPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + fixedPool, WorkerChoiceStrategies.LESS_BUSY ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -151,6 +156,7 @@ describe('Worker choice strategy context test suite', () => { dynamicPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + dynamicPool, WorkerChoiceStrategies.LESS_BUSY ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -163,6 +169,7 @@ describe('Worker choice strategy context test suite', () => { fixedPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + fixedPool, WorkerChoiceStrategies.FAIR_SHARE ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -175,6 +182,7 @@ describe('Worker choice strategy context test suite', () => { dynamicPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + dynamicPool, WorkerChoiceStrategies.FAIR_SHARE ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -187,6 +195,7 @@ describe('Worker choice strategy context test suite', () => { fixedPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + fixedPool, WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( @@ -199,6 +208,7 @@ describe('Worker choice strategy context test suite', () => { dynamicPool ) workerChoiceStrategyContext.setWorkerChoiceStrategy( + dynamicPool, WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( -- 2.34.1