From c2ade475e1b3b24aa2a1757b6d97a26063ec708c Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 5 Apr 2023 10:44:05 +0200 Subject: [PATCH] fix: fix dynamic pool busy semantic 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 ++++++++--- src/pools/cluster/dynamic.ts | 9 +++++++-- src/pools/cluster/fixed.ts | 7 ++++++- src/pools/pool-internal.ts | 14 ++++++++------ .../dynamic-pool-worker-choice-strategy.ts | 3 +-- src/pools/thread/dynamic.ts | 9 +++++++-- src/pools/thread/fixed.ts | 7 ++++++- src/worker/abstract-worker.ts | 2 +- tests/pools/cluster/dynamic.test.js | 3 +-- tests/pools/thread/dynamic.test.js | 2 +- 10 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index f904af93..1e61071c 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -143,8 +143,10 @@ export abstract class AbstractPool< /** {@inheritDoc} */ public abstract get type (): PoolType - /** {@inheritDoc} */ - public get numberOfRunningTasks (): number { + /** + * Number of tasks concurrently running. + */ + private get numberOfRunningTasks (): number { return this.promiseResponseMap.size } @@ -177,10 +179,13 @@ export abstract class AbstractPool< ) } + /** {@inheritDoc} */ + public abstract get full (): boolean + /** {@inheritDoc} */ public abstract get busy (): boolean - protected internalGetBusyStatus (): boolean { + protected internalBusy (): boolean { return ( this.numberOfRunningTasks >= this.numberOfWorkers && this.findFreeWorkerKey() === -1 diff --git a/src/pools/cluster/dynamic.ts b/src/pools/cluster/dynamic.ts index 8b8c5185..c4fc788c 100644 --- a/src/pools/cluster/dynamic.ts +++ b/src/pools/cluster/dynamic.ts @@ -27,7 +27,7 @@ export class DynamicClusterPool< */ public constructor ( min: number, - protected readonly max: number, + private readonly max: number, filePath: string, opts: ClusterPoolOptions = {} ) { @@ -40,7 +40,12 @@ export class DynamicClusterPool< } /** {@inheritDoc} */ - public get busy (): boolean { + public get full (): boolean { return this.workers.length === this.max } + + /** {@inheritDoc} */ + public get busy (): boolean { + return this.full && this.findFreeWorkerKey() === -1 + } } diff --git a/src/pools/cluster/fixed.ts b/src/pools/cluster/fixed.ts index b8a73e4b..d644f229 100644 --- a/src/pools/cluster/fixed.ts +++ b/src/pools/cluster/fixed.ts @@ -100,8 +100,13 @@ export class FixedClusterPool< return PoolType.FIXED } + /** {@inheritDoc} */ + public get full (): boolean { + return this.workers.length === this.numberOfWorkers + } + /** {@inheritDoc} */ public get busy (): boolean { - return this.internalGetBusyStatus() + return this.internalBusy() } } diff --git a/src/pools/pool-internal.ts b/src/pools/pool-internal.ts index cf6c6a99..806107ec 100644 --- a/src/pools/pool-internal.ts +++ b/src/pools/pool-internal.ts @@ -43,7 +43,7 @@ export interface IPoolInternal< Response = unknown > extends IPool { /** - * Pool workers item array. + * Pool worker type items array. */ readonly workers: Array> @@ -55,16 +55,18 @@ export interface IPoolInternal< readonly type: PoolType /** - * Whether the pool is busy or not. + * Whether the pool is full or not. * - * The pool busyness boolean status. + * The pool filling boolean status. */ - readonly busy: boolean + readonly full: boolean /** - * Number of tasks currently concurrently running. + * Whether the pool is busy or not. + * + * The pool busyness boolean status. */ - readonly numberOfRunningTasks: number + readonly busy: boolean /** * Finds a free worker key based on the number of tasks the worker has applied. diff --git a/src/pools/selection-strategies/dynamic-pool-worker-choice-strategy.ts b/src/pools/selection-strategies/dynamic-pool-worker-choice-strategy.ts index c0622613..930675f2 100644 --- a/src/pools/selection-strategies/dynamic-pool-worker-choice-strategy.ts +++ b/src/pools/selection-strategies/dynamic-pool-worker-choice-strategy.ts @@ -54,8 +54,7 @@ export class DynamicPoolWorkerChoiceStrategy< if (this.pool.busy) { return this.workerChoiceStrategy.choose() } - const freeWorkerKey = this.pool.findFreeWorkerKey() - if (freeWorkerKey === -1) { + if (!this.pool.full && this.pool.findFreeWorkerKey() === -1) { return this.createWorkerCallback() } return this.workerChoiceStrategy.choose() diff --git a/src/pools/thread/dynamic.ts b/src/pools/thread/dynamic.ts index 19cb9fbf..3333aa6e 100644 --- a/src/pools/thread/dynamic.ts +++ b/src/pools/thread/dynamic.ts @@ -28,7 +28,7 @@ export class DynamicThreadPool< */ public constructor ( min: number, - protected readonly max: number, + private readonly max: number, filePath: string, opts: PoolOptions = {} ) { @@ -41,7 +41,12 @@ export class DynamicThreadPool< } /** {@inheritDoc} */ - public get busy (): boolean { + public get full (): boolean { return this.workers.length === this.max } + + /** {@inheritDoc} */ + public get busy (): boolean { + return this.full && this.findFreeWorkerKey() === -1 + } } diff --git a/src/pools/thread/fixed.ts b/src/pools/thread/fixed.ts index a37ccbf7..2c4f66f0 100644 --- a/src/pools/thread/fixed.ts +++ b/src/pools/thread/fixed.ts @@ -96,8 +96,13 @@ export class FixedThreadPool< return PoolType.FIXED } + /** {@inheritDoc} */ + public get full (): boolean { + return this.workers.length === this.numberOfWorkers + } + /** {@inheritDoc} */ public get busy (): boolean { - return this.internalGetBusyStatus() + return this.internalBusy() } } diff --git a/src/worker/abstract-worker.ts b/src/worker/abstract-worker.ts index 9c4fd514..bf81a9f2 100644 --- a/src/worker/abstract-worker.ts +++ b/src/worker/abstract-worker.ts @@ -44,7 +44,7 @@ export abstract class AbstractWorker< */ public constructor ( type: string, - protected isMain: boolean, + protected readonly isMain: boolean, fn: (data: Data) => Response, protected mainWorker: MainWorker | undefined | null, opts: WorkerOptions = { diff --git a/tests/pools/cluster/dynamic.test.js b/tests/pools/cluster/dynamic.test.js index ddf42501..6dd9f506 100644 --- a/tests/pools/cluster/dynamic.test.js +++ b/tests/pools/cluster/dynamic.test.js @@ -99,7 +99,6 @@ describe('Dynamic cluster pool test suite', () => { } expect(longRunningPool.workers.length).toBe(max) await TestUtils.waitExits(longRunningPool, max - min) - // Here we expect the workers to be at the max size since that the task is still running expect(longRunningPool.workers.length).toBe(min) // We need to clean up the resources after our test await longRunningPool.destroy() @@ -122,7 +121,7 @@ describe('Dynamic cluster pool test suite', () => { } expect(longRunningPool.workers.length).toBe(max) await TestUtils.sleep(1500) - // Here we expect the workers to be at the max size since that the task is still running + // Here we expect the workers to be at the max size since the task is still running expect(longRunningPool.workers.length).toBe(max) // We need to clean up the resources after our test await longRunningPool.destroy() diff --git a/tests/pools/thread/dynamic.test.js b/tests/pools/thread/dynamic.test.js index 9036642f..d8c68e90 100644 --- a/tests/pools/thread/dynamic.test.js +++ b/tests/pools/thread/dynamic.test.js @@ -121,7 +121,7 @@ describe('Dynamic thread pool test suite', () => { } expect(longRunningPool.workers.length).toBe(max) await TestUtils.sleep(1500) - // Here we expect the workers to be at the max size since that the task is still running + // Here we expect the workers to be at the max size since the task is still running expect(longRunningPool.workers.length).toBe(max) // We need to clean up the resources after our test await longRunningPool.destroy() -- 2.34.1