fix: fix dynamic pool busy semantic
authorJérôme Benoit <jerome.benoit@sap.com>
Wed, 5 Apr 2023 08:44:05 +0000 (10:44 +0200)
committerJérôme Benoit <jerome.benoit@sap.com>
Wed, 5 Apr 2023 08:44:05 +0000 (10:44 +0200)
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
src/pools/abstract-pool.ts
src/pools/cluster/dynamic.ts
src/pools/cluster/fixed.ts
src/pools/pool-internal.ts
src/pools/selection-strategies/dynamic-pool-worker-choice-strategy.ts
src/pools/thread/dynamic.ts
src/pools/thread/fixed.ts
src/worker/abstract-worker.ts
tests/pools/cluster/dynamic.test.js
tests/pools/thread/dynamic.test.js

index f904af93183840b89edbf166dd02a61fa7e7ec1d..1e61071c42382e1ccb9d546cb0a035ac6196afe3 100644 (file)
@@ -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
index 8b8c5185772dbea59bc0d22b2e1cb90b9383d507..c4fc788cd8dcdd0528bca2234caead2d9fc70ccd 100644 (file)
@@ -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
+  }
 }
index b8a73e4b0562fff6fbca6f48cab4b582c3c5a562..d644f2298000fe1b6176f335ff228e51483e9144 100644 (file)
@@ -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()
   }
 }
index cf6c6a99b26235a3ee4a28042072d9d3d0d5f057..806107ec74ea29402408abaf3cf95a8f0325581c 100644 (file)
@@ -43,7 +43,7 @@ export interface IPoolInternal<
   Response = unknown
 > extends IPool<Data, Response> {
   /**
-   * Pool workers item array.
+   * Pool worker type items array.
    */
   readonly workers: Array<WorkerType<Worker>>
 
@@ -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.
index c0622613d8701c02360a04c7ab91abe10aff33d3..930675f295dc46ba1961eef6978e1c4a575f205a 100644 (file)
@@ -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()
index 19cb9fbf6b9a21a649c9fe548955e62abc6c750f..3333aa6e0072c852ebc3eb7e857d2c1f0d9a3c89 100644 (file)
@@ -28,7 +28,7 @@ export class DynamicThreadPool<
    */
   public constructor (
     min: number,
-    protected readonly max: number,
+    private readonly max: number,
     filePath: string,
     opts: PoolOptions<ThreadWorkerWithMessageChannel> = {}
   ) {
@@ -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
+  }
 }
index a37ccbf76b5864b8cc4f27b847596d7b9c533d38..2c4f66f0ff4d8be12d192f4028b1b1a17262bbc7 100644 (file)
@@ -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()
   }
 }
index 9c4fd514cc97d46cdfa5eee261993a386719dbe9..bf81a9f2432ff81606f7c10536c6ce338f16f066 100644 (file)
@@ -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 = {
index ddf42501d43ed63a3f84831b3aa2b13ba44fc701..6dd9f506fd78b8e606ed7d3d99906a2cc64465e0 100644 (file)
@@ -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()
index 9036642f1b00a87bd9c8eeebb9a72ffa9abd81a0..d8c68e90c51100886bc8fe83ebfba8ec2ef094f1 100644 (file)
@@ -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()