From 51fe3d3cb0858b5e2ad96076b51a6b9daa8f852c Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 5 Apr 2023 21:24:58 +0200 Subject: [PATCH] refactor: move worker choice instance helper into the context class 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 | 34 +++---- .../selection-strategies-utils.ts | 48 ---------- .../worker-choice-strategy-context.ts | 57 +++++++++-- .../selection-strategies-utils.test.js | 94 ------------------- .../worker-choice-strategy-context.test.js | 78 +++++++++++++++ 5 files changed, 142 insertions(+), 169 deletions(-) delete mode 100644 src/pools/selection-strategies/selection-strategies-utils.ts delete mode 100644 tests/pools/selection-strategies/selection-strategies-utils.test.js diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 0da7f86b..7b0e5bc7 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -45,9 +45,9 @@ export abstract class AbstractPool< > = new Map>() /** - * Worker choice strategy instance implementing the worker choice algorithm. + * Worker choice strategy context referencing a worker choice algorithm implementation. * - * Default to a strategy implementing a round robin algorithm. + * Default to a round robin algorithm. */ protected workerChoiceStrategyContext: WorkerChoiceStrategyContext< Worker, @@ -149,7 +149,7 @@ export abstract class AbstractPool< public abstract get type (): PoolType /** - * Number of tasks concurrently running. + * Number of tasks concurrently running in the pool. */ private get numberOfRunningTasks (): number { return this.promiseResponseMap.size @@ -289,21 +289,10 @@ export abstract class AbstractPool< } } - /** - * Removes the given worker from the pool. - * - * @param worker - The worker that will be removed. - */ - protected removeWorker (worker: Worker): void { - const workerKey = this.getWorkerKey(worker) - this.workers.splice(workerKey, 1) - this.workerChoiceStrategyContext.remove(workerKey) - } - /** * Chooses a worker for the next task. * - * The default implementation uses a round robin algorithm to distribute the load. + * The default uses a round robin algorithm to distribute the load. * * @returns [worker key, worker]. */ @@ -440,7 +429,7 @@ export abstract class AbstractPool< } /** - * Pushes the given worker. + * Pushes the given worker in the pool. * * @param worker - The worker. * @param tasksUsage - The worker tasks usage. @@ -453,7 +442,7 @@ export abstract class AbstractPool< } /** - * Sets the given worker. + * Sets the given worker in the pool. * * @param workerKey - The worker key. * @param worker - The worker. @@ -469,4 +458,15 @@ export abstract class AbstractPool< tasksUsage } } + + /** + * Removes the given worker from the pool. + * + * @param worker - The worker that will be removed. + */ + protected removeWorker (worker: Worker): void { + const workerKey = this.getWorkerKey(worker) + this.workers.splice(workerKey, 1) + this.workerChoiceStrategyContext.remove(workerKey) + } } diff --git a/src/pools/selection-strategies/selection-strategies-utils.ts b/src/pools/selection-strategies/selection-strategies-utils.ts deleted file mode 100644 index b0967d09..00000000 --- a/src/pools/selection-strategies/selection-strategies-utils.ts +++ /dev/null @@ -1,48 +0,0 @@ -import type { IPoolInternal } from '../pool-internal' -import type { IPoolWorker } from '../pool-worker' -import { FairShareWorkerChoiceStrategy } from './fair-share-worker-choice-strategy' -import { LessBusyWorkerChoiceStrategy } from './less-busy-worker-choice-strategy' -import { LessUsedWorkerChoiceStrategy } from './less-used-worker-choice-strategy' -import { RoundRobinWorkerChoiceStrategy } from './round-robin-worker-choice-strategy' -import type { - IWorkerChoiceStrategy, - WorkerChoiceStrategy -} from './selection-strategies-types' -import { WorkerChoiceStrategies } from './selection-strategies-types' -import { WeightedRoundRobinWorkerChoiceStrategy } from './weighted-round-robin-worker-choice-strategy' - -/** - * Gets the worker choice strategy instance. - * - * @param pool - The pool instance. - * @param workerChoiceStrategy - The worker choice strategy. - * @returns The worker choice strategy instance. - */ -export function getWorkerChoiceStrategy< - Worker extends IPoolWorker, - Data = unknown, - Response = unknown -> ( - pool: IPoolInternal, - workerChoiceStrategy: WorkerChoiceStrategy = WorkerChoiceStrategies.ROUND_ROBIN -): IWorkerChoiceStrategy { - switch (workerChoiceStrategy) { - case WorkerChoiceStrategies.ROUND_ROBIN: - return new RoundRobinWorkerChoiceStrategy(pool) - case WorkerChoiceStrategies.LESS_USED: - return new LessUsedWorkerChoiceStrategy(pool) - case WorkerChoiceStrategies.LESS_BUSY: - return new LessBusyWorkerChoiceStrategy(pool) - case WorkerChoiceStrategies.FAIR_SHARE: - return new FairShareWorkerChoiceStrategy(pool) - case WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN: - return new WeightedRoundRobinWorkerChoiceStrategy( - pool - ) - default: - throw new Error( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Worker choice strategy '${workerChoiceStrategy}' not found` - ) - } -} diff --git a/src/pools/selection-strategies/worker-choice-strategy-context.ts b/src/pools/selection-strategies/worker-choice-strategy-context.ts index 97d08682..644adfea 100644 --- a/src/pools/selection-strategies/worker-choice-strategy-context.ts +++ b/src/pools/selection-strategies/worker-choice-strategy-context.ts @@ -1,12 +1,16 @@ import type { IPoolInternal } from '../pool-internal' import type { IPoolWorker } from '../pool-worker' +import { FairShareWorkerChoiceStrategy } from './fair-share-worker-choice-strategy' +import { LessBusyWorkerChoiceStrategy } from './less-busy-worker-choice-strategy' +import { LessUsedWorkerChoiceStrategy } from './less-used-worker-choice-strategy' +import { RoundRobinWorkerChoiceStrategy } from './round-robin-worker-choice-strategy' import type { IWorkerChoiceStrategy, RequiredStatistics, WorkerChoiceStrategy } from './selection-strategies-types' import { WorkerChoiceStrategies } from './selection-strategies-types' -import { getWorkerChoiceStrategy } from './selection-strategies-utils' +import { WeightedRoundRobinWorkerChoiceStrategy } from './weighted-round-robin-worker-choice-strategy' /** * The worker choice strategy context. @@ -35,14 +39,14 @@ export class WorkerChoiceStrategyContext< private workerChoiceStrategyType: WorkerChoiceStrategy = WorkerChoiceStrategies.ROUND_ROBIN ) { this.execute.bind(this) - this.workerChoiceStrategy = getWorkerChoiceStrategy( + this.workerChoiceStrategy = this.getWorkerChoiceStrategy( pool, this.workerChoiceStrategyType ) } /** - * Gets the worker choice strategy required statistics. + * Gets the worker choice strategy in the context required statistics. * * @returns The required statistics. */ @@ -63,16 +67,15 @@ export class WorkerChoiceStrategyContext< this.workerChoiceStrategy?.reset() } else { this.workerChoiceStrategyType = workerChoiceStrategy - this.workerChoiceStrategy = getWorkerChoiceStrategy< - Worker, - Data, - Response - >(pool, this.workerChoiceStrategyType) + this.workerChoiceStrategy = this.getWorkerChoiceStrategy( + pool, + this.workerChoiceStrategyType + ) } } /** - * Chooses a worker with the worker choice strategy. + * Executes the worker choice strategy algorithm in the context. * * @returns The key of the chosen one. */ @@ -88,7 +91,7 @@ export class WorkerChoiceStrategyContext< } /** - * Removes a worker in the worker choice strategy internals. + * Removes a worker from the worker choice strategy in the context. * * @param workerKey - The key of the worker to remove. * @returns `true` if the removal is successful, `false` otherwise. @@ -96,4 +99,38 @@ export class WorkerChoiceStrategyContext< public remove (workerKey: number): boolean { return this.workerChoiceStrategy.remove(workerKey) } + + /** + * Gets the worker choice strategy instance. + * + * @param pool - The pool instance. + * @param workerChoiceStrategy - The worker choice strategy. + * @returns The worker choice strategy instance. + */ + private getWorkerChoiceStrategy ( + pool: IPoolInternal, + workerChoiceStrategy: WorkerChoiceStrategy = WorkerChoiceStrategies.ROUND_ROBIN + ): IWorkerChoiceStrategy { + switch (workerChoiceStrategy) { + case WorkerChoiceStrategies.ROUND_ROBIN: + return new RoundRobinWorkerChoiceStrategy(pool) + case WorkerChoiceStrategies.LESS_USED: + return new LessUsedWorkerChoiceStrategy(pool) + case WorkerChoiceStrategies.LESS_BUSY: + return new LessBusyWorkerChoiceStrategy(pool) + case WorkerChoiceStrategies.FAIR_SHARE: + return new FairShareWorkerChoiceStrategy(pool) + case WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN: + return new WeightedRoundRobinWorkerChoiceStrategy< + Worker, + Data, + Response + >(pool) + default: + throw new Error( + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `Worker choice strategy '${workerChoiceStrategy}' not found` + ) + } + } } diff --git a/tests/pools/selection-strategies/selection-strategies-utils.test.js b/tests/pools/selection-strategies/selection-strategies-utils.test.js deleted file mode 100644 index 87150745..00000000 --- a/tests/pools/selection-strategies/selection-strategies-utils.test.js +++ /dev/null @@ -1,94 +0,0 @@ -const { expect } = require('expect') -// const sinon = require('sinon') -const { - getWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/selection-strategies-utils') -const { - FixedThreadPool, - WorkerChoiceStrategies -} = require('../../../lib/index') -const { - RoundRobinWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/round-robin-worker-choice-strategy') -const { - LessUsedWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/less-used-worker-choice-strategy') -const { - LessBusyWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/less-busy-worker-choice-strategy') -const { - FairShareWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/fair-share-worker-choice-strategy') -const { - WeightedRoundRobinWorkerChoiceStrategy -} = require('../../../lib/pools/selection-strategies/weighted-round-robin-worker-choice-strategy') - -describe('Selection strategies utils test suite', () => { - const max = 3 - let pool - - before(() => { - pool = new FixedThreadPool(max, './tests/worker-files/thread/testWorker.js') - }) - - // afterEach(() => { - // sinon.restore() - // }) - - after(async () => { - await pool.destroy() - }) - - it('Verify that getWorkerChoiceStrategy() default return ROUND_ROBIN strategy', () => { - const strategy = getWorkerChoiceStrategy(pool) - expect(strategy).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() can return ROUND_ROBIN strategy', () => { - const strategy = getWorkerChoiceStrategy( - pool, - WorkerChoiceStrategies.ROUND_ROBIN - ) - expect(strategy).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() can return LESS_USED strategy', () => { - const strategy = getWorkerChoiceStrategy( - pool, - WorkerChoiceStrategies.LESS_USED - ) - expect(strategy).toBeInstanceOf(LessUsedWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() can return LESS_BUSY strategy', () => { - const strategy = getWorkerChoiceStrategy( - pool, - WorkerChoiceStrategies.LESS_BUSY - ) - expect(strategy).toBeInstanceOf(LessBusyWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() can return FAIR_SHARE strategy', () => { - const strategy = getWorkerChoiceStrategy( - pool, - WorkerChoiceStrategies.FAIR_SHARE - ) - expect(strategy).toBeInstanceOf(FairShareWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() can return WEIGHTED_ROUND_ROBIN strategy', () => { - const strategy = getWorkerChoiceStrategy( - pool, - WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN - ) - expect(strategy).toBeInstanceOf(WeightedRoundRobinWorkerChoiceStrategy) - }) - - it('Verify that getWorkerChoiceStrategy() throw error on unknown strategy', () => { - expect(() => { - getWorkerChoiceStrategy(pool, 'UNKNOWN_STRATEGY') - }).toThrowError( - new Error("Worker choice strategy 'UNKNOWN_STRATEGY' not found") - ) - }) -}) 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 bcd84208..a93104d2 100644 --- a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js +++ b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js @@ -257,4 +257,82 @@ describe('Worker choice strategy context test suite', () => { WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) }) + + it('Verify that getWorkerChoiceStrategy() default return ROUND_ROBIN strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = + workerChoiceStrategyContext.getWorkerChoiceStrategy(fixedPool) + expect(strategy).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() can return ROUND_ROBIN strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + WorkerChoiceStrategies.ROUND_ROBIN + ) + expect(strategy).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() can return LESS_USED strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + WorkerChoiceStrategies.LESS_USED + ) + expect(strategy).toBeInstanceOf(LessUsedWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() can return LESS_BUSY strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + WorkerChoiceStrategies.LESS_BUSY + ) + expect(strategy).toBeInstanceOf(LessBusyWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() can return FAIR_SHARE strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + WorkerChoiceStrategies.FAIR_SHARE + ) + expect(strategy).toBeInstanceOf(FairShareWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() can return WEIGHTED_ROUND_ROBIN strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + const strategy = workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN + ) + expect(strategy).toBeInstanceOf(WeightedRoundRobinWorkerChoiceStrategy) + }) + + it('Verify that getWorkerChoiceStrategy() throw error on unknown strategy', () => { + const workerChoiceStrategyContext = new WorkerChoiceStrategyContext( + fixedPool + ) + expect(() => { + workerChoiceStrategyContext.getWorkerChoiceStrategy( + fixedPool, + 'UNKNOWN_STRATEGY' + ) + }).toThrowError( + new Error("Worker choice strategy 'UNKNOWN_STRATEGY' not found") + ) + }) }) -- 2.34.1