fix: fix worker choice strategy options handling

Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>

diff --git a/.eslintrc.js b/.eslintrc.js
index cc27eee78dd9cc582522dd315e640b7161e2c335..0a343d6b4e4f5899bd8b7d4f3a80aa541d602c0b 100644 (file)
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -72,7 +72,8 @@ module.exports = defineConfig({
           'unlink',
           'unregister',
           'utf8',
-          'workerpool'
+          'workerpool',
+          'wwr'
         ],
         skipIfMatch: ['^@.*', '^plugin:.*']
       }

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5efd1b843646f37ba9a0ded0916782f869ed1149..5d599b1622c13695557c90f33b93b8cf61735793 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fix worker function type definition and validation.
+- Fix worker choice strategy options handling.
 
 ## [2.4.8] - 2023-04-12
 

diff --git a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts
index ce7f9589ab5708975019706047b477f71007731f..77b44f6e344547154a0091ca8f17a82bdf1a7cb2 100644 (file)
--- a/src/pools/selection-strategies/abstract-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/abstract-worker-choice-strategy.ts
@@ -38,14 +38,14 @@ export abstract class AbstractWorkerChoiceStrategy<
     protected readonly pool: IPool<Worker, Data, Response>,
     protected readonly opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
   ) {
-    this.checkOptions(this.opts)
     this.isDynamicPool = this.pool.type === PoolType.DYNAMIC
     this.choose.bind(this)
   }
 
-  private checkOptions (opts: WorkerChoiceStrategyOptions): void {
+  protected checkOptions (opts: WorkerChoiceStrategyOptions): void {
     if (this.requiredStatistics.avgRunTime && opts.medRunTime === true) {
-      this.requiredStatistics.medRunTime = true
+      this.requiredStatistics.avgRunTime = false
+      this.requiredStatistics.medRunTime = opts.medRunTime as boolean
     }
   }
 

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 3f4e95249497897d0f718c4192880fecdc695129..81944f7a6c1d0342889b716a26be5b23cc34034e 100644 (file)
--- a/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/fair-share-worker-choice-strategy.ts
@@ -1,8 +1,11 @@
+import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils'
+import type { IPool } from '../pool'
 import type { IWorker } from '../worker'
 import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy'
 import type {
   IWorkerChoiceStrategy,
-  RequiredStatistics
+  RequiredStatistics,
+  WorkerChoiceStrategyOptions
 } from './selection-strategies-types'
 
 /**
@@ -43,6 +46,15 @@ export class FairShareWorkerChoiceStrategy<
   WorkerVirtualTaskTimestamp
   > = new Map<number, WorkerVirtualTaskTimestamp>()
 
+  /** @inheritDoc */
+  public constructor (
+    pool: IPool<Worker, Data, Response>,
+    opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
+  ) {
+    super(pool, opts)
+    this.checkOptions(opts)
+  }
+
   /** @inheritDoc */
   public reset (): boolean {
     this.workerLastVirtualTaskTimestamp.clear()

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 12e87ff2575649c6c1a9847d8dd24a9ad1e30f9a..708c491f63dea4183232c20f5d1c7373d14738ba 100644 (file)
--- a/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/less-busy-worker-choice-strategy.ts
@@ -1,8 +1,11 @@
+import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils'
+import type { IPool } from '../pool'
 import type { IWorker } from '../worker'
 import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy'
 import type {
   IWorkerChoiceStrategy,
-  RequiredStatistics
+  RequiredStatistics,
+  WorkerChoiceStrategyOptions
 } from './selection-strategies-types'
 
 /**
@@ -26,6 +29,15 @@ export class LessBusyWorkerChoiceStrategy<
     medRunTime: false
   }
 
+  /** @inheritDoc */
+  public constructor (
+    pool: IPool<Worker, Data, Response>,
+    opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
+  ) {
+    super(pool, opts)
+    this.checkOptions(opts)
+  }
+
   /** @inheritDoc */
   public reset (): boolean {
     return 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 166de7069f60f32d1f396576fd8416e9ba87907d..1138df952a4600b31ff4f60a5540b94f035b31b3 100644 (file)
--- a/src/pools/selection-strategies/less-used-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/less-used-worker-choice-strategy.ts
@@ -1,6 +1,11 @@
+import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils'
+import type { IPool } from '../pool'
 import type { IWorker } from '../worker'
 import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy'
-import type { IWorkerChoiceStrategy } from './selection-strategies-types'
+import type {
+  IWorkerChoiceStrategy,
+  WorkerChoiceStrategyOptions
+} from './selection-strategies-types'
 
 /**
  * Selects the less used worker.
@@ -16,6 +21,15 @@ export class LessUsedWorkerChoiceStrategy<
   >
   extends AbstractWorkerChoiceStrategy<Worker, Data, Response>
   implements IWorkerChoiceStrategy {
+  /** @inheritDoc */
+  public constructor (
+    pool: IPool<Worker, Data, Response>,
+    opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
+  ) {
+    super(pool, opts)
+    this.checkOptions(opts)
+  }
+
   /** @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 80e958a406fa2209ce1c20eded5b8e9b79e9ca0b..107e9639b38466e3d832fd217885b21b27206f12 100644 (file)
--- a/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/round-robin-worker-choice-strategy.ts
@@ -1,6 +1,11 @@
+import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils'
+import type { IPool } from '../pool'
 import type { IWorker } from '../worker'
 import { AbstractWorkerChoiceStrategy } from './abstract-worker-choice-strategy'
-import type { IWorkerChoiceStrategy } from './selection-strategies-types'
+import type {
+  IWorkerChoiceStrategy,
+  WorkerChoiceStrategyOptions
+} from './selection-strategies-types'
 
 /**
  * Selects the next worker in a round robin fashion.
@@ -21,6 +26,15 @@ export class RoundRobinWorkerChoiceStrategy<
    */
   private nextWorkerNodeId: number = 0
 
+  /** @inheritDoc */
+  public constructor (
+    pool: IPool<Worker, Data, Response>,
+    opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
+  ) {
+    super(pool, opts)
+    this.checkOptions(opts)
+  }
+
   /** @inheritDoc */
   public reset (): boolean {
     this.nextWorkerNodeId = 0

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 665022c79aebaf4596e306b7975aa7c0c7f3d923..dc2e5df3f170a1c8d65118e705d6b0754cebf56b 100644 (file)
--- a/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts
+++ b/src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts
@@ -7,6 +7,7 @@ import type {
   WorkerChoiceStrategyOptions
 } from './selection-strategies-types'
 import type { IPool } from '../pool'
+import { DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS } from '../../utils'
 
 /**
  * Virtual task runtime.
@@ -54,17 +55,13 @@ export class WeightedRoundRobinWorkerChoiceStrategy<
   TaskRunTime
   >()
 
-  /**
-   * Constructs a worker choice strategy that selects with a weighted round robin scheduling algorithm.
-   *
-   * @param pool - The pool instance.
-   * @param opts - The worker choice strategy options.
-   */
+  /** @inheritDoc */
   public constructor (
     pool: IPool<Worker, Data, Response>,
-    opts?: WorkerChoiceStrategyOptions
+    opts: WorkerChoiceStrategyOptions = DEFAULT_WORKER_CHOICE_STRATEGY_OPTIONS
   ) {
     super(pool, opts)
+    this.checkOptions(opts)
     this.defaultWorkerWeight = this.computeWorkerWeight()
     this.initWorkersTaskRunTime()
   }

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 23d954c9f13159940e542c8774a3d22feefeaaad..7e616fc73008579caff1670d2249e0a8aff8f8d2 100644 (file)
--- a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js
+++ b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js
@@ -336,4 +336,61 @@ describe('Worker choice strategy context test suite', () => {
       workerChoiceStrategy
     )
   })
+
+  it.only('Verify that worker choice strategy options enable median run time pool statistics', () => {
+    const wwrWorkerChoiceStrategy = WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN
+    let workerChoiceStrategyContext = new WorkerChoiceStrategyContext(
+      fixedPool,
+      wwrWorkerChoiceStrategy,
+      {
+        medRunTime: true
+      }
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe(
+      false
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe(
+      true
+    )
+    workerChoiceStrategyContext = new WorkerChoiceStrategyContext(
+      dynamicPool,
+      wwrWorkerChoiceStrategy,
+      {
+        medRunTime: true
+      }
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe(
+      false
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe(
+      true
+    )
+    const fsWorkerChoiceStrategy = WorkerChoiceStrategies.FAIR_SHARE
+    workerChoiceStrategyContext = new WorkerChoiceStrategyContext(
+      fixedPool,
+      fsWorkerChoiceStrategy,
+      {
+        medRunTime: true
+      }
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe(
+      false
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe(
+      true
+    )
+    workerChoiceStrategyContext = new WorkerChoiceStrategyContext(
+      dynamicPool,
+      fsWorkerChoiceStrategy,
+      {
+        medRunTime: true
+      }
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().avgRunTime).toBe(
+      false
+    )
+    expect(workerChoiceStrategyContext.getRequiredStatistics().medRunTime).toBe(
+      true
+    )
+  })
 })