From c3b5f73876a16d1a5053fbc231b6d4a4b053a7be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <oss@bajtos.net>
Date: Fri, 18 Oct 2024 14:49:15 +0200
Subject: [PATCH] fix: limit the delay between tasks to 60s
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ensure the checker prints some message at least once per minute,
to prevent Station Core for considering it as stuck.

As a side effect, this also fixes the problem when maxTasksPerRound
is zero, the delay is calculated as Infinity and we call
`setTimeout(Infinity)`. Such call is a no-op in Deno, which causes
the checker to loop very quickly and pollute the logs with too many
"Completed all tasks for the current round." messages.

After this change, the message is print once per minute.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
---
 lib/spark.js  | 29 ++++++++++++++++++++++++++---
 test/spark.js | 32 +++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/lib/spark.js b/lib/spark.js
index 624a4dd..394dcdf 100644
--- a/lib/spark.js
+++ b/lib/spark.js
@@ -211,10 +211,17 @@ export default class Spark {
         this.handleRunError(err)
       }
       const duration = Date.now() - started
-      const baseDelay = APPROX_ROUND_LENGTH_IN_MS / this.#tasker.maxTasksPerRound
-      const delay = baseDelay - duration
+      const delay = calculateDelayBeforeNextTask({
+        roundLengthInMs: APPROX_ROUND_LENGTH_IN_MS,
+        maxTasksPerRound: this.#tasker.maxTasksPerRound,
+        lastTaskDurationInMs: duration
+      })
+
       if (delay > 0) {
-        console.log('Sleeping for %s seconds before starting the next task...', Math.round(delay / 1000))
+        console.log(
+          'Sleeping for %s seconds before starting the next task...',
+          Math.round(delay / 1000)
+        )
         await sleep(delay)
         console.log() // add an empty line to visually delimit logs from different tasks
       }
@@ -231,6 +238,22 @@ export default class Spark {
   }
 }
 
+/**
+ * @param {object} args
+ * @param {number} args.roundLengthInMs
+ * @param {number} args.maxTasksPerRound
+ * @param {number} args.lastTaskDurationInMs
+ */
+export function calculateDelayBeforeNextTask ({
+  roundLengthInMs,
+  maxTasksPerRound,
+  lastTaskDurationInMs
+}) {
+  const baseDelay = roundLengthInMs / maxTasksPerRound
+  const delay = baseDelay - lastTaskDurationInMs
+  return Math.min(delay, 60_000)
+}
+
 export function newStats () {
   return {
     timeout: false,
diff --git a/test/spark.js b/test/spark.js
index f73bc66..4dd4508 100644
--- a/test/spark.js
+++ b/test/spark.js
@@ -1,6 +1,6 @@
 /* global Zinnia */
 
-import Spark, { newStats } from '../lib/spark.js'
+import Spark, { calculateDelayBeforeNextTask, newStats } from '../lib/spark.js'
 import { test } from 'zinnia:test'
 import { assertInstanceOf, assertEquals, assertArrayIncludes } from 'zinnia:assert'
 import { SPARK_VERSION } from '../lib/constants.js'
@@ -271,3 +271,33 @@ test('submitRetrieval', async () => {
     }
   ])
 })
+
+test('calculateDelayBeforeNextTask() returns value based on average task duration', () => {
+  const delay = calculateDelayBeforeNextTask({
+    lastTaskDurationInMs: 3_000,
+
+    // one task every 10 seconds (on average)
+    roundLengthInMs: 60_000,
+    maxTasksPerRound: 6
+  })
+  assertEquals(delay, 7_000)
+})
+
+test('calculateDelayBeforeNextTask() handles zero tasks per round', () => {
+  const delay = calculateDelayBeforeNextTask({
+    maxTasksPerRound: 0,
+    // the values below are not important
+    roundLengthInMs: 12345,
+    lastTaskDurationInMs: 12
+  })
+  assertEquals(delay, 60_000)
+})
+
+test('calculateDelayBeforeNextTask() handles one task per round', () => {
+  const delay = calculateDelayBeforeNextTask({
+    roundLengthInMs: 20 * 60_000,
+    maxTasksPerRound: 1,
+    lastTaskDurationInMs: 1_000
+  })
+  assertEquals(delay, 60_000)
+})