From 3baa083708b113152d769123eb87ef8d3c3a7ce7 Mon Sep 17 00:00:00 2001
From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= <jerome.benoit@piment-noir.org>
Date: Thu, 24 Aug 2023 20:00:43 +0200
Subject: [PATCH] fix: fix pool information median computation
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
---
 CHANGELOG.md                     |  1 +
 docs/worker-choice-strategies.md |  2 +-
 src/pools/abstract-pool.ts       | 46 +++++++++++++++++++-------------
 src/pools/pool.ts                |  4 +--
 4 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 811b8397..0e8866ff 100644
--- 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
 
 - Don't account worker usage statistics for tasks that have failed.
+- Fix pool information runtime and wait time median computation.
 
 ### Changed
 
diff --git a/docs/worker-choice-strategies.md b/docs/worker-choice-strategies.md
index 296d8c3b..a2b035f7 100644
--- a/docs/worker-choice-strategies.md
+++ b/docs/worker-choice-strategies.md
@@ -34,4 +34,4 @@ Worker choice strategies enable only the statistics that are needed to choose th
 
 ### Simple moving median
 
-Strategies using the average task execution time for each worker can use the simple moving median instead. Simple moving median is more robust to outliers and can be used to avoid assigning tasks to workers that are currently overloaded. Simple moving median usage introduces a small overhead: measurement history must be kept for each worker and the simple moving median must be recomputed each time a task has finished.
+Strategies using the simple moving average task execution time for each worker can use the simple moving median instead. Simple moving median is more robust to outliers and can be used to avoid assigning tasks to workers that are currently overloaded.
diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts
index a11a5741..8c202696 100644
--- a/src/pools/abstract-pool.ts
+++ b/src/pools/abstract-pool.ts
@@ -427,21 +427,26 @@ export abstract class AbstractPool<
               )
             )
           ),
-          average: round(
-            average(
-              this.workerNodes.reduce<number[]>(
-                (accumulator, workerNode) =>
-                  accumulator.concat(workerNode.usage.runTime.history),
-                []
+          ...(this.workerChoiceStrategyContext.getTaskStatisticsRequirements()
+            .runTime.average && {
+            average: round(
+              average(
+                this.workerNodes.reduce<number[]>(
+                  (accumulator, workerNode) =>
+                    accumulator.concat(workerNode.usage.runTime.history),
+                  []
+                )
               )
             )
-          ),
+          }),
           ...(this.workerChoiceStrategyContext.getTaskStatisticsRequirements()
             .runTime.median && {
             median: round(
               median(
-                this.workerNodes.map(
-                  (workerNode) => workerNode.usage.runTime?.median ?? 0
+                this.workerNodes.reduce<number[]>(
+                  (accumulator, workerNode) =>
+                    accumulator.concat(workerNode.usage.runTime.history),
+                  []
                 )
               )
             )
@@ -465,21 +470,26 @@ export abstract class AbstractPool<
               )
             )
           ),
-          average: round(
-            average(
-              this.workerNodes.reduce<number[]>(
-                (accumulator, workerNode) =>
-                  accumulator.concat(workerNode.usage.waitTime.history),
-                []
+          ...(this.workerChoiceStrategyContext.getTaskStatisticsRequirements()
+            .waitTime.average && {
+            average: round(
+              average(
+                this.workerNodes.reduce<number[]>(
+                  (accumulator, workerNode) =>
+                    accumulator.concat(workerNode.usage.waitTime.history),
+                  []
+                )
               )
             )
-          ),
+          }),
           ...(this.workerChoiceStrategyContext.getTaskStatisticsRequirements()
             .waitTime.median && {
             median: round(
               median(
-                this.workerNodes.map(
-                  (workerNode) => workerNode.usage.waitTime?.median ?? 0
+                this.workerNodes.reduce<number[]>(
+                  (accumulator, workerNode) =>
+                    accumulator.concat(workerNode.usage.waitTime.history),
+                  []
                 )
               )
             )
diff --git a/src/pools/pool.ts b/src/pools/pool.ts
index fb59d37a..7b549809 100644
--- a/src/pools/pool.ts
+++ b/src/pools/pool.ts
@@ -84,13 +84,13 @@ export interface PoolInfo {
   readonly runTime?: {
     readonly minimum: number
     readonly maximum: number
-    readonly average: number
+    readonly average?: number
     readonly median?: number
   }
   readonly waitTime?: {
     readonly minimum: number
     readonly maximum: number
-    readonly average: number
+    readonly average?: number
     readonly median?: number
   }
 }
-- 
2.34.1