Merge branch 'master' into issue-70
authorAlessandro Pio Ardizio <alessandroardizio94@gmail.com>
Mon, 15 Feb 2021 16:30:30 +0000 (17:30 +0100)
committerGitHub <noreply@github.com>
Mon, 15 Feb 2021 16:30:30 +0000 (17:30 +0100)
CHANGELOG.md
src/pools/abstract-pool.ts
src/pools/cluster/dynamic.ts
src/pools/thread/dynamic.ts
tests/pools/cluster/dynamic.test.js
tests/pools/cluster/fixed.test.js
tests/pools/thread/dynamic.test.js
tests/pools/thread/fixed.test.js
tests/worker/cluster/longRunningWorker.js [new file with mode: 0644]
tests/worker/thread/longRunningWorker.js [new file with mode: 0644]

index 6e8e90a03a527a3192c64c8eb8ad6114009b5285..12fd3608b50babfda765bb43aceaad9a311a0b34 100644 (file)
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [2.0.0] - not released yet
 
+### Bug fixes
+ - Now a thread/process is not delete when the task submitted take more time than maxInactiveTime configured( issue #70)
 ### Breaking Changes
 
 We changed some internal structures, but you shouldn't be too affected by them as these are internal changes.
index 2ffd292afab9a0b540826820935b4a0febd34057..879e3e4f78f2b3ab60a5918a865fa7c711e3d882 100644 (file)
@@ -83,7 +83,7 @@ export abstract class AbstractPool<
 
   /**
    * - `key`: The `Worker`
-   * - `value`: Number of tasks that has been assigned to that worker since it started
+   * - `value`: Number of tasks currently in progress on the worker.
    */
   public readonly tasks: Map<Worker, number> = new Map<Worker, number>()
 
@@ -120,7 +120,6 @@ export abstract class AbstractPool<
     if (!this.filePath) {
       throw new Error('Please specify a file with a worker implementation')
     }
-
     this.setupHook()
 
     for (let i = 1; i <= this.numberOfWorkers; i++) {
@@ -199,6 +198,20 @@ export abstract class AbstractPool<
     }
   }
 
+  /**
+   * Increase the number of tasks that the given workers has done.
+   *
+   * @param worker Workers whose tasks are increased.
+   */
+  protected decreaseWorkersTasks (worker: Worker): void {
+    const numberOfTasksTheWorkerHas = this.tasks.get(worker)
+    if (numberOfTasksTheWorkerHas !== undefined) {
+      this.tasks.set(worker, numberOfTasksTheWorkerHas - 1)
+    } else {
+      throw Error('Worker could not be found in tasks map')
+    }
+  }
+
   /**
    * Removes the given worker from the pool.
    *
@@ -254,7 +267,7 @@ export abstract class AbstractPool<
       const listener: (message: MessageValue<Response>) => void = message => {
         if (message.id === messageId) {
           this.unregisterWorkerMessageListener(worker, listener)
-          this.increaseWorkersTask(worker)
+          this.decreaseWorkersTasks(worker)
           if (message.error) reject(message.error)
           else resolve(message.data as Response)
         }
index b0bb697372cdc3312d4cf78200ecc7ceca214cac..f6c70126f41599b482c6a653923dad3a66bed964 100644 (file)
@@ -61,7 +61,8 @@ export class DynamicClusterPool<
     // All workers are busy, create a new worker
     const worker = this.createAndSetupWorker()
     this.registerWorkerMessageListener<Data>(worker, message => {
-      if (message.kill) {
+      const tasksInProgress = this.tasks.get(worker)
+      if (message.kill && !tasksInProgress) {
         this.sendToWorker(worker, { kill: 1 })
         void this.destroyWorker(worker)
       }
index fddae4662e8b4a4a2b68baf043015fa498d78b4f..74013f2b70dceaf0e35ece2bbd9c25c5d0168701 100644 (file)
@@ -61,7 +61,10 @@ export class DynamicThreadPool<
     // All workers are busy, create a new worker
     const worker = this.createAndSetupWorker()
     this.registerWorkerMessageListener<Data>(worker, message => {
-      if (message.kill) {
+      const tasksInProgress = this.tasks.get(worker)
+      if (message.kill && !tasksInProgress) {
+        // Kill received from the worker, means that no new tasks are submitted to that worker for a while( > maxInactiveTime)
+        // To handle the case of a long-running task we will check if the there is any active task
         this.sendToWorker(worker, { kill: 1 })
         void this.destroyWorker(worker)
       }
index 6a82b05712a5f5dac09ddedc324f608c78db8b0d..e0a0fde3cc6246bf662cc656851bbf78f32abcde 100644 (file)
@@ -7,8 +7,7 @@ const pool = new DynamicClusterPool(
   max,
   './tests/worker/cluster/testWorker.js',
   {
-    errorHandler: e => console.error(e),
-    onlineHandler: () => console.log('worker is online')
+    errorHandler: e => console.error(e)
   }
 )
 
@@ -51,7 +50,7 @@ describe('Dynamic cluster pool test suite ', () => {
       pool.execute({ test: 'test' })
     }
     expect(pool.workers.length).toBeGreaterThan(min)
-    await new Promise(resolve => setTimeout(resolve, 2000))
+    await new Promise(resolve => setTimeout(resolve, 3000))
     expect(pool.workers.length).toBe(min)
   })
   it('Shutdown test', async () => {
@@ -62,7 +61,7 @@ describe('Dynamic cluster pool test suite ', () => {
       })
     })
     pool.destroy()
-    await new Promise(resolve => setTimeout(resolve, 1000))
+    await new Promise(resolve => setTimeout(resolve, 2000))
     expect(closedWorkers).toBe(min)
   })
 
@@ -87,4 +86,19 @@ describe('Dynamic cluster pool test suite ', () => {
     const res = await pool1.execute({ test: 'test' })
     expect(res).toBeFalsy()
   })
+  it('Verify scale processes up and down is working when long running task is used', async () => {
+    const longRunningPool = new DynamicClusterPool(
+      min,
+      max,
+      './tests/worker/cluster/longRunningWorker.js'
+    )
+    expect(longRunningPool.workers.length).toBe(min)
+    for (let i = 0; i < max * 10; i++) {
+      longRunningPool.execute({ test: 'test' })
+    }
+    expect(longRunningPool.workers.length).toBe(max)
+    await new Promise(resolve => setTimeout(resolve, 3000))
+    // Here we expect the workers to be at the max size since that the task is still running
+    expect(longRunningPool.workers.length).toBe(max)
+  })
 })
index d22f697a5d3893863c5c60798cd8d77a7dd7e451..51501df0350dde87d73be94cc4ddebb7be66cee2 100644 (file)
@@ -6,8 +6,7 @@ const pool = new FixedClusterPool(
   numberOfWorkers,
   './tests/worker/cluster/testWorker.js',
   {
-    errorHandler: e => console.error(e),
-    onlineHandler: () => console.log('worker is online')
+    errorHandler: e => console.error(e)
   }
 )
 const emptyPool = new FixedClusterPool(
@@ -19,8 +18,7 @@ const errorPool = new FixedClusterPool(
   1,
   './tests/worker/cluster/errorWorker.js',
   {
-    errorHandler: e => console.error(e),
-    onlineHandler: () => console.log('worker is online')
+    errorHandler: e => console.error(e)
   }
 )
 
index 98a0dd0f0d180076f4a87a17f7e9bbe1ee050310..2f89b2ffea69b045b576db32287a5acc8bfd2a7b 100644 (file)
@@ -7,8 +7,7 @@ const pool = new DynamicThreadPool(
   max,
   './tests/worker/thread/testWorker.js',
   {
-    errorHandler: e => console.error(e),
-    onlineHandler: () => console.log('worker is online')
+    errorHandler: e => console.error(e)
   }
 )
 
@@ -53,6 +52,7 @@ describe('Dynamic thread pool test suite ', () => {
     await new Promise(resolve => setTimeout(resolve, 1000))
     expect(pool.workers.length).toBe(min)
   })
+
   it('Shutdown test', async () => {
     let closedThreads = 0
     pool.workers.forEach(w => {
@@ -85,4 +85,24 @@ describe('Dynamic thread pool test suite ', () => {
     const res = await pool1.execute({ test: 'test' })
     expect(res).toBeFalsy()
   })
+
+  it('Verify scale thread up and down is working when long running task is used', async () => {
+    const longRunningPool = new DynamicThreadPool(
+      min,
+      max,
+      './tests/worker/thread/longRunningWorker.js',
+      {
+        errorHandler: e => console.error(e),
+        onlineHandler: () => console.log('worker is online')
+      }
+    )
+    expect(longRunningPool.workers.length).toBe(min)
+    for (let i = 0; i < max * 10; i++) {
+      longRunningPool.execute({ test: 'test' })
+    }
+    expect(longRunningPool.workers.length).toBe(max)
+    await new Promise(resolve => setTimeout(resolve, 1000))
+    // Here we expect the workers to be at the max size since that the task is still running
+    expect(longRunningPool.workers.length).toBe(max)
+  })
 })
index a5ec3f0a8a63e720b1c789e9a23249a1ca866a4f..e160f17c72b6d4db2e8111d7fed488f84a666610 100644 (file)
@@ -6,8 +6,7 @@ const pool = new FixedThreadPool(
   numberOfThreads,
   './tests/worker/thread/testWorker.js',
   {
-    errorHandler: e => console.error(e),
-    onlineHandler: () => console.log('worker is online')
+    errorHandler: e => console.error(e)
   }
 )
 const emptyPool = new FixedThreadPool(1, './tests/worker/thread/emptyWorker.js')
diff --git a/tests/worker/cluster/longRunningWorker.js b/tests/worker/cluster/longRunningWorker.js
new file mode 100644 (file)
index 0000000..d751d35
--- /dev/null
@@ -0,0 +1,10 @@
+'use strict'
+const { ClusterWorker } = require('../../../lib/index')
+
+async function sleep (data) {
+  return new Promise((resolve, reject) => {
+    setTimeout(() => resolve(data), 50000)
+  })
+}
+
+module.exports = new ClusterWorker(sleep, { maxInactiveTime: 500, async: true })
diff --git a/tests/worker/thread/longRunningWorker.js b/tests/worker/thread/longRunningWorker.js
new file mode 100644 (file)
index 0000000..8689127
--- /dev/null
@@ -0,0 +1,10 @@
+'use strict'
+const { ThreadWorker } = require('../../../lib/index')
+
+async function sleep (data) {
+  return new Promise((resolve, reject) => {
+    setTimeout(() => resolve(data), 50000)
+  })
+}
+
+module.exports = new ThreadWorker(sleep, { maxInactiveTime: 500, async: true })