fix: fix workey weights input validation
authorJérôme Benoit <jerome.benoit@sap.com>
Mon, 8 May 2023 10:49:01 +0000 (12:49 +0200)
committerJérôme Benoit <jerome.benoit@sap.com>
Mon, 8 May 2023 10:49:01 +0000 (12:49 +0200)
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
src/pools/abstract-pool.ts
src/pools/selection-strategies/abstract-worker-choice-strategy.ts
src/pools/selection-strategies/fair-share-worker-choice-strategy.ts
src/pools/selection-strategies/less-busy-worker-choice-strategy.ts
src/pools/selection-strategies/less-used-worker-choice-strategy.ts
src/pools/selection-strategies/round-robin-worker-choice-strategy.ts
src/pools/selection-strategies/weighted-round-robin-worker-choice-strategy.ts
src/queue.ts
tests/pools/abstract/abstract-pool.test.js

index 5114bead6d4670af440b5d66f5af22bfa5aa213f..eab110d8ec4d2200ec78320383874abbf333562a 100644 (file)
@@ -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 (
index bf67b80e84ef257bd8d10ffe86946a53b2433b8a..c3fb28c4f1e162185f65e08029c886e8df826d99 100644 (file)
@@ -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
   }
 
index 3a2e71eea4fcbc3af3c0f35162639747cda171a7..0ae939949b1c115918db6bb1f33dfab2e253191a 100644 (file)
@@ -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 */
index 40a3f649b2116eec3f25b4b07347a8c5134b2e77..f518f9b17f311ad3a8ad262e3b7e5f0be15cc61b 100644 (file)
@@ -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 */
index 295493a494f842da32645f563469e5ba272eea3d..a8daa7b9f43d05e871cf38e0822cb986ba671239 100644 (file)
@@ -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 */
index e99383caf977bec57a28d158b7c147806ed41fcb..be4fe3326b9585c8bd4f7261023cda4fe5af710e 100644 (file)
@@ -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 */
index 5d8bbe2f1e4466bdfa12b0167d79ee53fc94e065..bda206a7d7e1521ac8a6056b0485ad18e7698375 100644 (file)
@@ -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()
   }
 
index ab606547b1f77fb08ab6b203e0053ad182b481e6..7682f65f952136743c1f81724a26e53264ac18ed 100644 (file)
@@ -55,4 +55,12 @@ export class Queue<T> {
     }
     return item
   }
+
+  /**
+   * Peek at the first item.
+   */
+  public peek (): T | undefined {
+    if (this.size <= 0) return undefined
+    return this.items[this.head]
+  }
 }
index 038868c2d1502ce721561b23c469bd845e69a8f1..bf01176de969fe3668ce8ded789673837c4b3fd6 100644 (file)
@@ -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 () => {