From 49770d370aac4ac6adf24cf5b7fe1d650dcbf1da Mon Sep 17 00:00:00 2001 From: Johan Nyman Date: Wed, 3 Apr 2024 14:21:36 +0200 Subject: [PATCH 1/2] fix: remove non-existant workers from the tracked availableWorkers & queriedWorkers When workers closed down, their ids where left in the trackedExp.availableWorkers, so the only old, invalid workers where queried, and not the new ones. This caused PM to try to spin up new workers indefinitely. --- .../lib/trackedExpectationAPI.ts | 17 +++++++++++++++++ .../internalManager/lib/trackedWorkerAgents.ts | 14 ++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts b/shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts index 5995b4ff..6cb16ca4 100644 --- a/shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts +++ b/shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts @@ -174,6 +174,23 @@ export class TrackedExpectationAPI { if (trackedExp.session.assignedWorker) throw new Error('Internal error: noWorkerAssigned can only be called when assignedWorker is falsy') + if (trackedExp.availableWorkers.size === 0) { + // Special case: No workers are available at all + // Return to NEW state, so that new workers can be assigned: + + this.updateTrackedExpectationStatus(trackedExp, { + state: ExpectedPackageStatusAPI.WorkStatusState.NEW, + reason: { + user: 'No workers available', + tech: 'availableWorkers=0', + }, + // Don't update the package status, since this means that we don't know anything about the package: + dontUpdatePackage: true, + }) + + return + } + let noAssignedWorkerReason: ExpectedPackageStatusAPI.Reason if (!trackedExp.session.noAssignedWorkerReason) { this.logger.error( diff --git a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts index 6ab38f0e..f4273578 100644 --- a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts +++ b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts @@ -180,6 +180,20 @@ export class TrackedWorkerAgents { return } + // Remove any workers that no longer exist: + { + for (const workerId of trackedExp.availableWorkers.keys()) { + if (!this.get(workerId)) { + trackedExp.availableWorkers.delete(workerId) + } + } + for (const workerId of trackedExp.queriedWorkers.keys()) { + if (!this.get(workerId)) { + trackedExp.queriedWorkers.delete(workerId) + } + } + } + if (!trackedExp.availableWorkers.size) { session.noAssignedWorkerReason = { user: `No workers available`, tech: `No workers available` } } From e9640eb6ebc2e25130ecdd15b4dcbd43dc129d81 Mon Sep 17 00:00:00 2001 From: Johan Nyman Date: Wed, 3 Apr 2024 14:24:22 +0200 Subject: [PATCH 2/2] chore: improve & simplify logging in determineBestWorkerForExpectation() --- .../lib/trackedWorkerAgents.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts index f4273578..3d5cf0c7 100644 --- a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts +++ b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts @@ -90,8 +90,6 @@ export class TrackedWorkerAgents { */ public async determineBestWorkerForExpectation(trackedExp: TrackedExpectation): Promise<{ bestWorker: WorkerAgentAssignment | undefined - countQueried: number - countInfinite: number noCostReason: Reason }> { /** How many requests to send out simultaneously */ @@ -104,10 +102,7 @@ export class TrackedWorkerAgents { const workerIds = Array.from(trackedExp.availableWorkers.keys()) - let noCostReason: Reason = { - user: `${workerIds.length} workers are currently busy`, - tech: `${workerIds.length} busy, ${trackedExp.queriedWorkers.size} queried`, - } + let noCostReason: Reason | undefined = undefined const workerCosts: WorkerAgentAssignment[] = [] @@ -145,6 +140,8 @@ export class TrackedWorkerAgents { tech: `${stringifyError(error, true)}`, } } + } else { + this.logger.error(`Worker "${workerId}" not found in determineBestWorkerForExpectation`) } }) @@ -162,10 +159,20 @@ export class TrackedWorkerAgents { return 0 }) + if (!noCostReason) { + noCostReason = { + user: `${countInfinite} workers are currently busy`, + tech: + `availableWorkers: ${trackedExp.availableWorkers.size}, ` + + `queriedWorkers: ${trackedExp.queriedWorkers.size}, ` + + `countQueried: ${countQueried}, ` + + `countInfinite: ${countInfinite} ` + + `(Worker costs: ${workerCosts.map((c) => `${c.id}: ${c.cost}`).join(', ')}`, + } + } + return { bestWorker: workerCosts[0], - countQueried, - countInfinite, noCostReason, } } @@ -181,6 +188,7 @@ export class TrackedWorkerAgents { } // Remove any workers that no longer exist: + // (Like if a worker has shut down) { for (const workerId of trackedExp.availableWorkers.keys()) { if (!this.get(workerId)) { @@ -200,9 +208,7 @@ export class TrackedWorkerAgents { // Send a number of requests simultaneously: - const { bestWorker, countQueried, countInfinite, noCostReason } = await this.determineBestWorkerForExpectation( - trackedExp - ) + const { bestWorker, noCostReason } = await this.determineBestWorkerForExpectation(trackedExp) if (bestWorker) { session.assignedWorker = bestWorker @@ -210,7 +216,7 @@ export class TrackedWorkerAgents { } else { session.noAssignedWorkerReason = { user: `Waiting for a free worker, ${noCostReason.user}`, - tech: `Waiting for a free worker ${noCostReason.tech} (${trackedExp.availableWorkers.size} busy, ${countQueried} asked, ${countInfinite} infinite cost)`, + tech: `Waiting for a free worker, ${noCostReason.tech}`, } } }