Fix strategy handling in pool options (#259)

And increase coverage a bit.

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

diff --git a/README.md b/README.md
index 1ddad503512fa3c8f3e2107006c9ca16cf165128..8f343c399943a803e60fc5c64a26002dfa8a81fd 100644 (file)
--- a/README.md
+++ b/README.md
@@ -205,7 +205,7 @@ We already have a bench folder where you can find some comparisons.
 Before to jump into each poolifier pool type, let highlight that **Node.js comes with a thread pool already**, the libuv thread pool where some particular tasks already run by default.  
 Please take a look at [which tasks run on the libuv thread pool](https://nodejs.org/en/docs/guides/dont-block-the-event-loop/#what-code-runs-on-the-worker-pool).
 
-Now **if your task runs on libuv thread pool**, you can try to:
+**If your task runs on libuv thread pool**, you can try to:
 
 - Tune the libuv thread pool size setting the [UV_THREADPOOL_SIZE](https://nodejs.org/api/cli.html#cli_uv_threadpool_size_size)
 

diff --git a/benchmarks/internal/choose-worker.js b/benchmarks/internal/choose-worker.js
index e4e727a225c781e8aad777f0a3b7d97ddf8f49a3..560a395b69207ec6fede5d6abf39e3a2ad4ba2f3 100644 (file)
--- a/benchmarks/internal/choose-worker.js
+++ b/benchmarks/internal/choose-worker.js
@@ -9,7 +9,7 @@ function generateWorkersArray (numberOfWorkers) {
 
 const workers = generateWorkersArray(60)
 
-let nextWorkerIndex = 0
+let nextWorkerIndex
 
 function chooseWorkerTernaryOffByOne () {
   nextWorkerIndex =
@@ -48,7 +48,7 @@ suite
     nextWorkerIndex = 0
     chooseWorkerTernaryWithNegation()
   })
-  .add('Ternary with PreChoosing', function () {
+  .add('Ternary with pre-choosing', function () {
     nextWorkerIndex = 0
     chooseWorkerTernaryWithPreChoosing()
   })

diff --git a/benchmarks/internal/cluster/dynamic.js b/benchmarks/internal/cluster/dynamic.js
index 6838e07a9cad0e882f62c68d8d890f5ec4a003ea..3c00c50b8c7af84ebfc45f5f6cb4a2c814104c41 100644 (file)
--- a/benchmarks/internal/cluster/dynamic.js
+++ b/benchmarks/internal/cluster/dynamic.js
@@ -5,6 +5,7 @@ const {
 const { runPoolifierTest } = require('../benchmark-utils')
 
 const size = 30
+const numberOfTasks = 1
 
 const dynamicPool = new DynamicClusterPool(
   size / 2,
@@ -20,13 +21,13 @@ const dynamicPoolLessRecentlyUsed = new DynamicClusterPool(
 )
 
 async function dynamicClusterTest (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(dynamicPool, { tasks, workerData })
 }
 
 async function dynamicClusterTestLessRecentlyUsed (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(dynamicPoolLessRecentlyUsed, { tasks, workerData })
 }

diff --git a/benchmarks/internal/cluster/fixed.js b/benchmarks/internal/cluster/fixed.js
index ea61c7fcb398a40326a19f0935b7f13268050cde..fc4cc3ac8dc545ba4b8d530401b19ac396f1f02c 100644 (file)
--- a/benchmarks/internal/cluster/fixed.js
+++ b/benchmarks/internal/cluster/fixed.js
@@ -5,6 +5,7 @@ const {
 const { runPoolifierTest } = require('../benchmark-utils')
 
 const size = 30
+const numberOfTasks = 1
 
 const fixedPool = new FixedClusterPool(
   size,
@@ -18,13 +19,13 @@ const fixedPoolLessRecentlyUsed = new FixedClusterPool(
 )
 
 async function fixedClusterTest (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(fixedPool, { tasks, workerData })
 }
 
 async function fixedClusterTestLessRecentlyUsed (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(fixedPoolLessRecentlyUsed, { tasks, workerData })
 }

diff --git a/benchmarks/internal/thread/dynamic.js b/benchmarks/internal/thread/dynamic.js
index fb74d8985d8d6dd85ba2c88e3f7c327526003c35..290820778e44ec7df7d2b2f2101b9dafeef4cb8c 100644 (file)
--- a/benchmarks/internal/thread/dynamic.js
+++ b/benchmarks/internal/thread/dynamic.js
@@ -5,6 +5,7 @@ const {
 const { runPoolifierTest } = require('../benchmark-utils')
 
 const size = 30
+const numberOfTasks = 1
 
 const dynamicPool = new DynamicThreadPool(
   size / 2,
@@ -20,13 +21,13 @@ const dynamicPoolLessRecentlyUsed = new DynamicThreadPool(
 )
 
 async function dynamicThreadTest (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(dynamicPool, { tasks, workerData })
 }
 
 async function dynamicThreadTestLessRecentlyUsed (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(dynamicPoolLessRecentlyUsed, { tasks, workerData })
 }

diff --git a/benchmarks/internal/thread/fixed.js b/benchmarks/internal/thread/fixed.js
index cd7fdebfdceeb57881be02299447ef3d3533f89d..617aa7e5950952c241c2c98a091e2cf8965553b6 100644 (file)
--- a/benchmarks/internal/thread/fixed.js
+++ b/benchmarks/internal/thread/fixed.js
@@ -5,6 +5,7 @@ const {
 const { runPoolifierTest } = require('../benchmark-utils')
 
 const size = 30
+const numberOfTasks = 1
 
 const fixedPool = new FixedThreadPool(
   size,
@@ -18,13 +19,13 @@ const fixedPoolLessRecentlyUsed = new FixedThreadPool(
 )
 
 async function fixedThreadTest (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(fixedPool, { tasks, workerData })
 }
 
 async function fixedThreadTestLessRecentlyUsed (
-  { tasks, workerData } = { tasks: 1, workerData: { proof: 'ok' } }
+  { tasks, workerData } = { tasks: numberOfTasks, workerData: { proof: 'ok' } }
 ) {
   return runPoolifierTest(fixedPoolLessRecentlyUsed, { tasks, workerData })
 }

diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts
index ff7413dd52f2f24e32b7359ce0f14af6180cca67..54447c82ba2db2e9a88503d6e670f73022a143bc 100644 (file)
--- a/src/pools/abstract-pool.ts
+++ b/src/pools/abstract-pool.ts
@@ -191,7 +191,7 @@ export abstract class AbstractPool<
         })
         return workerCreated
       },
-      opts.workerChoiceStrategy ?? WorkerChoiceStrategies.ROUND_ROBIN
+      this.opts.workerChoiceStrategy
     )
   }
 
@@ -220,6 +220,8 @@ export abstract class AbstractPool<
   }
 
   private checkPoolOptions (opts: PoolOptions<Worker>): void {
+    this.opts.workerChoiceStrategy =
+      opts.workerChoiceStrategy ?? WorkerChoiceStrategies.ROUND_ROBIN
     this.opts.enableEvents = opts.enableEvents ?? true
   }
 

diff --git a/tests/pools/abstract/abstract-pool.test.js b/tests/pools/abstract/abstract-pool.test.js
index 4eccde68f94faa99fab805426679bdc4f916e952..a2170491243313c13c65c36f8222eb5324e19eb9 100644 (file)
--- a/tests/pools/abstract/abstract-pool.test.js
+++ b/tests/pools/abstract/abstract-pool.test.js
@@ -1,5 +1,9 @@
 const expect = require('expect')
-const { FixedClusterPool, FixedThreadPool } = require('../../../lib/index')
+const {
+  FixedClusterPool,
+  FixedThreadPool,
+  WorkerChoiceStrategies
+} = require('../../../lib/index')
 const expectedError = new Error('Worker could not be found in tasks map')
 
 const numberOfWorkers = 1
@@ -102,16 +106,23 @@ describe('Abstract pool test suite', () => {
     )
     expect(pool.opts.enableEvents).toEqual(true)
     expect(pool.emitter).toBeDefined()
+    expect(pool.opts.workerChoiceStrategy).toBe(
+      WorkerChoiceStrategies.ROUND_ROBIN
+    )
     pool.destroy()
     pool = new FixedThreadPool(
       numberOfWorkers,
       './tests/worker-files/thread/testWorker.js',
       {
+        workerChoiceStrategy: WorkerChoiceStrategies.LESS_RECENTLY_USED,
         enableEvents: false
       }
     )
     expect(pool.opts.enableEvents).toEqual(false)
     expect(pool.emitter).toBeUndefined()
+    expect(pool.opts.workerChoiceStrategy).toBe(
+      WorkerChoiceStrategies.LESS_RECENTLY_USED
+    )
     pool.destroy()
   })
 

diff --git a/tests/pools/selection-strategies.test.js b/tests/pools/selection-strategies.test.js
index d631afc6ed30644aa01378bf9534f7cd730ff149..f57eaed2c1caec6d2b1002fb8b26224fd4dde6bb 100644 (file)
--- a/tests/pools/selection-strategies.test.js
+++ b/tests/pools/selection-strategies.test.js
@@ -11,6 +11,37 @@ describe('Selection strategies test suite', () => {
     expect(WorkerChoiceStrategies.LESS_RECENTLY_USED).toBe('LESS_RECENTLY_USED')
   })
 
+  it('Verify ROUND_ROBIN strategy is the default at pool creation', async () => {
+    const min = 0
+    const max = 3
+    const pool = new DynamicThreadPool(
+      min,
+      max,
+      './tests/worker-files/thread/testWorker.js'
+    )
+    expect(pool.opts.workerChoiceStrategy).toBe(
+      WorkerChoiceStrategies.ROUND_ROBIN
+    )
+    // We need to clean up the resources after our test
+    await pool.destroy()
+  })
+
+  it('Verify ROUND_ROBIN strategy can be set after pool creation', async () => {
+    const min = 0
+    const max = 3
+    const pool = new DynamicThreadPool(
+      min,
+      max,
+      './tests/worker-files/thread/testWorker.js'
+    )
+    pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.ROUND_ROBIN)
+    expect(pool.opts.workerChoiceStrategy).toBe(
+      WorkerChoiceStrategies.ROUND_ROBIN
+    )
+    // We need to clean up the resources after our test
+    await pool.destroy()
+  })
+
   it('Verify LESS_RECENTLY_USED strategy is taken at pool creation', async () => {
     const max = 3
     const pool = new FixedThreadPool(
@@ -52,7 +83,6 @@ describe('Selection strategies test suite', () => {
       promises.push(pool.execute({ test: 'test' }))
     }
     await Promise.all(promises)
-
     // We need to clean up the resources after our test
     await pool.destroy()
   })
@@ -72,7 +102,6 @@ describe('Selection strategies test suite', () => {
       promises.push(pool.execute({ test: 'test' }))
     }
     await Promise.all(promises)
-
     // We need to clean up the resources after our test
     await pool.destroy()
   })