From: Jérôme Benoit <jerome.benoit@sap.com>
Date: Sat, 10 Jun 2023 12:05:25 +0000 (+0200)
Subject: fix: fix average computation
X-Git-Tag: v2.6.1~7
X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=f1c0693012b2ec60fe1a0cbf457015b3f874e96f;p=poolifier.git

fix: fix average computation

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

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e4cd3788..ea36db29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix average statistics computation: ensure failed tasks are not accounted.
+
 ## [2.6.0] - 2023-06-09
 
 ### Added
diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts
index fa084fa1..f64e9ea4 100644
--- a/src/pools/abstract-pool.ts
+++ b/src/pools/abstract-pool.ts
@@ -484,14 +484,21 @@ export abstract class AbstractPool<
   ): void {
     const workerUsage =
       this.workerNodes[this.getWorkerNodeKey(worker)].workerUsage
+    this.updateTaskStatisticsWorkerUsage(workerUsage, message)
+    this.updateRunTimeWorkerUsage(workerUsage, message)
+    this.updateEluWorkerUsage(workerUsage, message)
+  }
+
+  private updateTaskStatisticsWorkerUsage (
+    workerUsage: WorkerUsage,
+    message: MessageValue<Response>
+  ): void {
     const workerTaskStatistics = workerUsage.tasks
     --workerTaskStatistics.executing
     ++workerTaskStatistics.executed
     if (message.taskError != null) {
       ++workerTaskStatistics.failed
     }
-    this.updateRunTimeWorkerUsage(workerUsage, message)
-    this.updateEluWorkerUsage(workerUsage, message)
   }
 
   private updateRunTimeWorkerUsage (
@@ -509,7 +516,8 @@ export abstract class AbstractPool<
         workerUsage.tasks.executed !== 0
       ) {
         workerUsage.runTime.average =
-          workerUsage.runTime.aggregate / workerUsage.tasks.executed
+          workerUsage.runTime.aggregate /
+          (workerUsage.tasks.executed - workerUsage.tasks.failed)
       }
       if (
         this.workerChoiceStrategyContext.getTaskStatisticsRequirements().runTime
@@ -539,7 +547,8 @@ export abstract class AbstractPool<
         workerUsage.tasks.executed !== 0
       ) {
         workerUsage.waitTime.average =
-          workerUsage.waitTime.aggregate / workerUsage.tasks.executed
+          workerUsage.waitTime.aggregate /
+          (workerUsage.tasks.executed - workerUsage.tasks.failed)
       }
       if (
         this.workerChoiceStrategyContext.getTaskStatisticsRequirements()
@@ -577,10 +586,12 @@ export abstract class AbstractPool<
           .average &&
         workerUsage.tasks.executed !== 0
       ) {
+        const executedTasks =
+          workerUsage.tasks.executed - workerUsage.tasks.failed
         workerUsage.elu.idle.average =
-          workerUsage.elu.idle.aggregate / workerUsage.tasks.executed
+          workerUsage.elu.idle.aggregate / executedTasks
         workerUsage.elu.active.average =
-          workerUsage.elu.active.aggregate / workerUsage.tasks.executed
+          workerUsage.elu.active.aggregate / executedTasks
       }
       if (
         this.workerChoiceStrategyContext.getTaskStatisticsRequirements().elu
diff --git a/src/pools/selection-strategies/README.md b/src/pools/selection-strategies/README.md
index 5093beb6..0e49d6df 100644
--- a/src/pools/selection-strategies/README.md
+++ b/src/pools/selection-strategies/README.md
@@ -5,7 +5,7 @@
 ### Fair share
 
 Its goal is to distribute the load evenly across all workers. To achieve this, the strategy keeps track of the average task execution time for each worker and assigns the next task to the worker with the lowest task end prediction time: `task_end_prediction = max(current_time, task_end_prediction) + average_task_execution_time`.  
-By default, the strategy uses the average task execution time for each worker but it can be configured to use the event loop utilization (ELU) active time instead.
+By default, the strategy uses the average task execution time for each worker but it can be configured to use the task event loop utilization (ELU) active time instead.
 
 ### Weighted round robin