From 49be33feded000ed776ee589274e154fa519b263 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Mon, 8 May 2023 12:49:01 +0200 Subject: [PATCH] fix: fix workey weights input validation 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 | 11 ++++++++++ .../abstract-worker-choice-strategy.ts | 12 ++--------- .../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 +- ...hted-round-robin-worker-choice-strategy.ts | 2 +- src/queue.ts | 8 ++++++++ tests/pools/abstract/abstract-pool.test.js | 20 +++++++++++++++++-- 9 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 5114bead..eab110d8 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -146,6 +146,9 @@ export abstract class AbstractPool< this.opts.workerChoiceStrategyOptions = opts.workerChoiceStrategyOptions ?? DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS + this.checkValidWorkerChoiceStrategyOptions( + this.opts.workerChoiceStrategyOptions + ) this.opts.enableEvents = opts.enableEvents ?? true this.opts.enableTasksQueue = opts.enableTasksQueue ?? false if (this.opts.enableTasksQueue) { @@ -179,6 +182,14 @@ export abstract class AbstractPool< 'Invalid worker choice strategy options: must be a plain object' ) } + if ( + workerChoiceStrategyOptions.weights != null && + Object.keys(workerChoiceStrategyOptions.weights).length !== this.size + ) { + throw new Error( + 'Invalid worker choice strategy options: must have a weight for each worker node' + ) + } } private checkValidTasksQueueOptions ( diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts index bf67b80e..c3fb28c4 100644 --- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts @@ -43,7 +43,7 @@ export abstract class AbstractWorkerChoiceStrategy< this.choose = this.choose.bind(this) } - protected checkOptions (opts: WorkerChoiceStrategyOptions): void { + protected setRequiredStatistics (opts: WorkerChoiceStrategyOptions): void { if (this.requiredStatistics.avgRunTime && opts.medRunTime === true) { this.requiredStatistics.avgRunTime = false this.requiredStatistics.medRunTime = opts.medRunTime as boolean @@ -52,14 +52,6 @@ export abstract class AbstractWorkerChoiceStrategy< this.requiredStatistics.avgRunTime = true this.requiredStatistics.medRunTime = opts.medRunTime as boolean } - if ( - opts.weights != null && - Object.keys(opts.weights).length < this.pool.size - ) { - throw new Error( - 'Worker choice strategy options must have a weight for each worker node.' - ) - } } /** @inheritDoc */ @@ -77,7 +69,7 @@ export abstract class AbstractWorkerChoiceStrategy< /** @inheritDoc */ public setOptions (opts: WorkerChoiceStrategyOptions): void { opts = opts ?? DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS - this.checkOptions(opts) + this.setRequiredStatistics(opts) this.opts = opts } 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 3a2e71ee..0ae93994 100644 --- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts @@ -41,7 +41,7 @@ export class FairShareWorkerChoiceStrategy< opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) - this.checkOptions(this.opts) + this.setRequiredStatistics(this.opts) } /** @inheritDoc */ 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 40a3f649..f518f9b1 100644 --- a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts @@ -35,7 +35,7 @@ export class LessBusyWorkerChoiceStrategy< opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) - this.checkOptions(this.opts) + this.setRequiredStatistics(this.opts) } /** @inheritDoc */ 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 295493a4..a8daa7b9 100644 --- a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts @@ -27,7 +27,7 @@ export class LessUsedWorkerChoiceStrategy< opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) - this.checkOptions(this.opts) + this.setRequiredStatistics(this.opts) } /** @inheritDoc */ 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 e99383ca..be4fe332 100644 --- a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts @@ -32,7 +32,7 @@ export class RoundRobinWorkerChoiceStrategy< opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) - this.checkOptions(this.opts) + this.setRequiredStatistics(this.opts) } /** @inheritDoc */ 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 5d8bbe2f..bda206a7 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 @@ -50,7 +50,7 @@ export class WeightedRoundRobinWorkerChoiceStrategy< opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS ) { super(pool, opts) - this.checkOptions(this.opts) + this.setRequiredStatistics(this.opts) this.defaultWorkerWeight = this.computeDefaultWorkerWeight() } diff --git a/src/queue.ts b/src/queue.ts index ab606547..7682f65f 100644 --- a/src/queue.ts +++ b/src/queue.ts @@ -55,4 +55,12 @@ export class Queue { } return item } + + /** + * Peek at the first item. + */ + public peek (): T | undefined { + if (this.size <= 0) return undefined + return this.items[this.head] + } } diff --git a/tests/pools/abstract/abstract-pool.test.js b/tests/pools/abstract/abstract-pool.test.js index 038868c2..bf01176d 100644 --- a/tests/pools/abstract/abstract-pool.test.js +++ b/tests/pools/abstract/abstract-pool.test.js @@ -103,7 +103,10 @@ describe('Abstract pool test suite', () => { './tests/worker-files/thread/testWorker.js', { workerChoiceStrategy: WorkerChoiceStrategies.LESS_USED, - workerChoiceStrategyOptions: { medRunTime: true }, + workerChoiceStrategyOptions: { + medRunTime: true, + weights: { 0: 300 } + }, enableEvents: false, enableTasksQueue: true, tasksQueueOptions: { concurrency: 2 }, @@ -121,7 +124,8 @@ describe('Abstract pool test suite', () => { WorkerChoiceStrategies.LESS_USED ) expect(pool.opts.workerChoiceStrategyOptions).toStrictEqual({ - medRunTime: true + medRunTime: true, + weights: { 0: 300 } }) expect(pool.opts.messageHandler).toStrictEqual(testHandler) expect(pool.opts.errorHandler).toStrictEqual(testHandler) @@ -152,6 +156,18 @@ describe('Abstract pool test suite', () => { } ) ).toThrowError("Invalid worker choice strategy 'invalidStrategy'") + expect( + () => + new FixedThreadPool( + numberOfWorkers, + './tests/worker-files/thread/testWorker.js', + { + workerChoiceStrategyOptions: { weights: {} } + } + ) + ).toThrowError( + 'Invalid worker choice strategy options: must have a weight for each worker node' + ) }) it('Verify that worker choice strategy options can be set', async () => { -- 2.34.1