From a237fea28e2fb60cba3fdb4264f5f14aaaabd223 Mon Sep 17 00:00:00 2001 From: Johan Nyman Date: Mon, 15 Apr 2024 08:31:12 +0200 Subject: [PATCH 1/3] fix: don't use infinite numbers, because they are not json-serializeable --- .../packages/generic/src/appContainer.ts | 7 ++++--- shared/packages/api/src/methods.ts | 11 ++++++----- shared/packages/api/src/statusReport.ts | 7 ++++--- shared/packages/api/src/worker.ts | 10 +++++++++- .../evaluateExpectationStates/ready.ts | 4 ++-- .../lib/workInProgressTracker.ts | 5 +++-- .../internalManager/lib/trackedWorkerAgents.ts | 8 ++++---- .../src/worker/workers/genericWorker/lib/lib.ts | 12 ++++++++---- shared/packages/worker/src/workerAgent.ts | 16 +++++++++++----- shared/packages/workforce/src/appContainerApi.ts | 5 +++-- shared/packages/workforce/src/workerHandler.ts | 10 ++++++---- 11 files changed, 60 insertions(+), 35 deletions(-) diff --git a/apps/appcontainer-node/packages/generic/src/appContainer.ts b/apps/appcontainer-node/packages/generic/src/appContainer.ts index 752ed44f..243acb70 100644 --- a/apps/appcontainer-node/packages/generic/src/appContainer.ts +++ b/apps/appcontainer-node/packages/generic/src/appContainer.ts @@ -35,6 +35,7 @@ import { LockId, protectString, getLogLevel, + Cost, } from '@sofie-package-manager/api' import { WorkforceAPI } from './workforceApi' @@ -373,7 +374,7 @@ export class AppContainer { async requestAppTypeForExpectation( exp: Expectation.Any - ): Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> { + ): Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> { this.logger.debug(`Got request for resources, for exp "${exp.id}"`) if (this.apps.size >= this.config.appContainer.maxRunningApps) { this.logger.debug(`Is already at our limit, no more resources available`) @@ -420,7 +421,7 @@ export class AppContainer { async requestAppTypeForPackageContainer( packageContainer: PackageContainerExpectation - ): Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> { + ): Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> { this.logger.debug(`Got request for resources, for packageContainer "${packageContainer.id}"`) if (this.apps.size >= this.config.appContainer.maxRunningApps) { this.logger.debug(`Is already at our limit, no more resources available`) @@ -807,7 +808,7 @@ interface AvailableAppInfo { /** Whether the application can be spun up as a critical worker */ canRunInCriticalExpectationsOnlyMode: boolean /** Some kind of value, how much it costs to run it, per minute */ - cost: number + cost: Cost } interface RunningAppInfo { diff --git a/shared/packages/api/src/methods.ts b/shared/packages/api/src/methods.ts index af079c9e..83a77fed 100644 --- a/shared/packages/api/src/methods.ts +++ b/shared/packages/api/src/methods.ts @@ -4,6 +4,7 @@ import { ExpectedPackageStatusAPI } from '@sofie-automation/shared-lib/dist/pack import { Expectation } from './expectationApi' import { PackageContainerExpectation } from './packageContainerApi' import { + Cost, ReturnTypeDisposePackageContainerMonitors, ReturnTypeDoYouSupportExpectation, ReturnTypeDoYouSupportPackageContainer, @@ -160,10 +161,10 @@ export namespace ExpectationManagerWorkerAgent { } export interface ExpectationCost { - /** Cost for working on the Expectation */ - cost: number + /** Cost for working on the Expectation (null means "infinite cost") */ + cost: Cost /** Cost "in queue" until working on the Expectation can start */ - startCost: number + startCost: Cost reason: Reason } export type MessageFromWorker = ( @@ -234,10 +235,10 @@ export namespace WorkForceAppContainer { requestAppTypeForExpectation: ( exp: Expectation.Any - ) => Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> + ) => Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> requestAppTypeForPackageContainer: ( packageContainer: PackageContainerExpectation - ) => Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> + ) => Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> spinUp: (appType: AppType) => Promise spinDown: (appId: AppId, reason: string) => Promise diff --git a/shared/packages/api/src/statusReport.ts b/shared/packages/api/src/statusReport.ts index 216a0de0..17a84ce8 100644 --- a/shared/packages/api/src/statusReport.ts +++ b/shared/packages/api/src/statusReport.ts @@ -9,6 +9,7 @@ import { WorkerAgentId, WorkInProgressLocalId, } from './ids' +import { Cost } from './worker' export interface WorkforceStatusReport { workerAgents: WorkerStatusReport[] @@ -50,7 +51,7 @@ export interface ExpectationManagerStatusReport { id: WorkInProgressId lastUpdated: number workerId: WorkerAgentId - cost: number + cost: Cost label: string progress: number expectationId: ExpectationId @@ -65,8 +66,8 @@ export interface WorkerStatusReport { }[] currentJobs: { - cost: number - startCost: number + cost: Cost + startCost: Cost cancelled: boolean wipId: WorkInProgressLocalId progress: number diff --git a/shared/packages/api/src/worker.ts b/shared/packages/api/src/worker.ts index 948c4c13..31d392ac 100644 --- a/shared/packages/api/src/worker.ts +++ b/shared/packages/api/src/worker.ts @@ -14,7 +14,8 @@ export type ReturnTypeDoYouSupportExpectation = reason: Reason } export type ReturnTypeGetCostFortExpectation = { - cost: number + /** (null means "infinite cost") */ + cost: Cost reason: Reason } export type ReturnTypeIsExpectationReadyToStartWorkingOn = @@ -103,3 +104,10 @@ export type ReturnTypeSetupPackageContainerMonitors = export interface MonitorProperties { label: string } + +/** A numeric value representing the effort needed to work on something (null means "infinitely high cost"). */ +export type Cost = number | null // Note: we're using null to represent infinity because Number.Infinity is not JSON-serializable +/** Converts Cost into a numeric value, that can be used to compare different costs to each other */ +export function valueOfCost(cost: Cost): number { + return cost === null ? Number.POSITIVE_INFINITY : cost +} diff --git a/shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ready.ts b/shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ready.ts index 03cdb593..cd0487a4 100644 --- a/shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ready.ts +++ b/shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ready.ts @@ -1,6 +1,6 @@ // eslint-disable-next-line node/no-extraneous-import import { ExpectedPackageStatusAPI } from '@sofie-automation/shared-lib/dist/package-manager/package' -import { stringifyError } from '@sofie-package-manager/api' +import { valueOfCost, stringifyError } from '@sofie-package-manager/api' import { expLabel } from '../../lib/trackedExpectation' import { assertState, EvaluateContext } from '../lib' @@ -23,7 +23,7 @@ export async function evaluateExpectationStateReady({ if ( trackedExp.session.assignedWorker && // Only allow starting if the job can start in a short while: - trackedExp.session.assignedWorker.cost.startCost > 0 // 2022-03-25: We're setting this to 0 to only allow one job per worker + valueOfCost(trackedExp.session.assignedWorker.cost.startCost) > 0 // 2022-03-25: We're setting this to 0 to only allow one job per worker ) { trackedExp.session.noAssignedWorkerReason = { user: `Workers are busy`, diff --git a/shared/packages/expectationManager/src/expectationTracker/lib/workInProgressTracker.ts b/shared/packages/expectationManager/src/expectationTracker/lib/workInProgressTracker.ts index 597b8b3a..02466d43 100644 --- a/shared/packages/expectationManager/src/expectationTracker/lib/workInProgressTracker.ts +++ b/shared/packages/expectationManager/src/expectationTracker/lib/workInProgressTracker.ts @@ -1,6 +1,7 @@ // eslint-disable-next-line node/no-extraneous-import import { ExpectedPackageStatusAPI } from '@sofie-automation/shared-lib/dist/package-manager/package' import { + Cost, ExpectationManagerWorkerAgent, LoggerInstance, Reason, @@ -161,7 +162,7 @@ export interface WorkInProgress { trackedExp: TrackedExpectation workerId: WorkerAgentId worker: WorkerAgentAPI - cost: number - startCost: number + cost: Cost + startCost: Cost lastUpdated: number } diff --git a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts index 73f64afb..0d6e47a9 100644 --- a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts +++ b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts @@ -1,5 +1,5 @@ import { PromisePool } from '@supercharge/promise-pool' -import { LoggerInstance, Reason, WorkerAgentId, stringifyError } from '@sofie-package-manager/api' +import { LoggerInstance, Reason, WorkerAgentId, valueOfCost, stringifyError } from '@sofie-package-manager/api' import { ExpectationStateHandlerSession, WorkerAgentAssignment } from '../../lib/types' import { WorkerAgentAPI } from '../../workerAgentApi' import { ExpectationTracker } from '../../expectationTracker/expectationTracker' @@ -123,7 +123,7 @@ export class TrackedWorkerAgents { countQueried++ const cost = await workerAgent.api.getCostForExpectation(trackedExp.exp) - if (cost.cost < Number.POSITIVE_INFINITY) { + if (cost.cost !== null) { workerCosts.push({ worker: workerAgent.api, id: workerId, @@ -147,8 +147,8 @@ export class TrackedWorkerAgents { workerCosts.sort((a, b) => { // Lowest cost first: - const aCost: number = a.cost.startCost + a.cost.cost - const bCost: number = b.cost.startCost + b.cost.cost + const aCost: number = valueOfCost(a.cost.startCost) + valueOfCost(a.cost.cost) + const bCost: number = valueOfCost(b.cost.startCost) + valueOfCost(b.cost.cost) if (aCost > bCost) return 1 if (aCost < bCost) return -1 diff --git a/shared/packages/worker/src/worker/workers/genericWorker/lib/lib.ts b/shared/packages/worker/src/worker/workers/genericWorker/lib/lib.ts index efb65fe5..56a6e6cc 100644 --- a/shared/packages/worker/src/worker/workers/genericWorker/lib/lib.ts +++ b/shared/packages/worker/src/worker/workers/genericWorker/lib/lib.ts @@ -8,6 +8,7 @@ import { ReturnTypeGetCostFortExpectation, PackageContainerId, AccessorId, + Cost, } from '@sofie-package-manager/api' import { prioritizeAccessors } from '../../../lib/lib' import { AccessorHandlerResultGeneric } from '../../../accessorHandlers/genericHandle' @@ -161,20 +162,23 @@ export function findBestAccessorOnPackageContainer( } /** Return a standard cost for the various accessorHandler types */ export function getStandardCost(exp: Expectation.Any, worker: BaseWorker): ReturnTypeGetCostFortExpectation { - let sourceCost: number = Number.POSITIVE_INFINITY + // null means that the cost is "infinite" + let sourceCost: Cost if (exp.startRequirement.sources.length > 0) { const source = findBestPackageContainerWithAccessToPackage(worker, exp.startRequirement.sources) - sourceCost = source ? getAccessorCost(source.accessor.type) : Number.POSITIVE_INFINITY + sourceCost = source ? getAccessorCost(source.accessor.type) : null } else { // If there are no sources defined, there is no cost for the source sourceCost = 0 } const target = findBestPackageContainerWithAccessToPackage(worker, exp.endRequirement.targets) - const targetCost = target ? getAccessorCost(target.accessor.type) : Number.POSITIVE_INFINITY + const targetCost: Cost = target ? getAccessorCost(target.accessor.type) : null + + const resultingCost: Cost = sourceCost !== null && targetCost !== null ? 30 * (sourceCost + targetCost) : null return { - cost: 30 * (sourceCost + targetCost), + cost: resultingCost, reason: { user: `Source cost: ${sourceCost}, Target cost: ${targetCost}`, tech: `Source cost: ${sourceCost}, Target cost: ${targetCost}`, diff --git a/shared/packages/worker/src/workerAgent.ts b/shared/packages/worker/src/workerAgent.ts index 806ab353..2ccd51a5 100644 --- a/shared/packages/worker/src/workerAgent.ts +++ b/shared/packages/worker/src/workerAgent.ts @@ -37,6 +37,7 @@ import { isProtectedString, WorkInProgressLocalId, objectEntries, + Cost, } from '@sofie-package-manager/api' import { AppContainerAPI } from './appContainerApi' @@ -379,7 +380,7 @@ export class WorkerAgent { this.spinDownTime = spinDownTime this.setupIntervalCheck() } - private getStartCost(exp: Expectation.Any): { cost: number; jobCount: number } { + private getStartCost(exp: Expectation.Any): { cost: Cost; jobCount: number } { const workerMultiplier: number = this.config.worker.costMultiplier || 1 let systemStartCost = 0 @@ -391,11 +392,16 @@ export class WorkerAgent { } } } + let resultingCost: Cost = systemStartCost + for (const job of this.currentJobs) { + if (resultingCost === null) break + + if (job.cost.cost === null) resultingCost = null + else resultingCost += job.cost.cost * (1 - job.progress) * workerMultiplier + } return { - cost: - (this.currentJobs.reduce((sum, job) => sum + job.cost.cost * (1 - job.progress), 0) + systemStartCost) * - workerMultiplier, + cost: resultingCost, jobCount: this.currentJobs.length, } } @@ -627,7 +633,7 @@ export class WorkerAgent { const startCost = this.getStartCost(exp) return { - cost: costForExpectation.cost * workerMultiplier, + cost: costForExpectation.cost !== null ? costForExpectation.cost * workerMultiplier : null, reason: { user: costForExpectation.reason.user, tech: `Cost: ${costForExpectation.reason.tech}, multiplier: ${workerMultiplier}, jobCount: ${startCost.jobCount}`, diff --git a/shared/packages/workforce/src/appContainerApi.ts b/shared/packages/workforce/src/appContainerApi.ts index a7537c73..f445fe0a 100644 --- a/shared/packages/workforce/src/appContainerApi.ts +++ b/shared/packages/workforce/src/appContainerApi.ts @@ -9,6 +9,7 @@ import { WorkforceId, AppType, AppId, + Cost, } from '@sofie-package-manager/api' /** @@ -40,12 +41,12 @@ export class AppContainerAPI async requestAppTypeForExpectation( exp: Expectation.Any - ): Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> { + ): Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> { return this._sendMessage('requestAppTypeForExpectation', exp) } async requestAppTypeForPackageContainer( packageContainer: PackageContainerExpectation - ): Promise<{ success: true; appType: AppType; cost: number } | { success: false; reason: Reason }> { + ): Promise<{ success: true; appType: AppType; cost: Cost } | { success: false; reason: Reason }> { return this._sendMessage('requestAppTypeForPackageContainer', packageContainer) } async spinUp(appType: AppType): Promise { diff --git a/shared/packages/workforce/src/workerHandler.ts b/shared/packages/workforce/src/workerHandler.ts index ccaab9b8..1fce9898 100644 --- a/shared/packages/workforce/src/workerHandler.ts +++ b/shared/packages/workforce/src/workerHandler.ts @@ -4,6 +4,8 @@ import { PackageContainerExpectation, AppContainerId, AppType, + Cost, + valueOfCost, } from '@sofie-package-manager/api' import { Workforce } from './workforce' @@ -21,12 +23,12 @@ export class WorkerHandler { this.logger.debug(`Workforce: Got request for resources for exp "${exp.id}"`) let errorReason = `No AppContainers registered` - let best: { appContainerId: AppContainerId; appType: AppType; cost: number } | null = null + let best: { appContainerId: AppContainerId; appType: AppType; cost: Cost } | null = null for (const [appContainerId, appContainer] of this.workForce.appContainers.entries()) { this.logger.debug(`Workforce: Asking appContainer "${appContainerId}"`) const proposal = await appContainer.api.requestAppTypeForExpectation(exp) if (proposal.success) { - if (!best || proposal.cost < best.cost) { + if (!best || valueOfCost(proposal.cost) < valueOfCost(best.cost)) { best = { appContainerId: appContainerId, appType: proposal.appType, @@ -57,12 +59,12 @@ export class WorkerHandler { this.logger.debug(`Workforce: Got request for resources for packageContainer "${packageContainer.id}"`) let errorReason = `No AppContainers registered` - let best: { appContainerId: AppContainerId; appType: AppType; cost: number } | null = null + let best: { appContainerId: AppContainerId; appType: AppType; cost: Cost } | null = null for (const [appContainerId, appContainer] of this.workForce.appContainers.entries()) { this.logger.debug(`Workforce: Asking appContainer "${appContainerId}"`) const proposal = await appContainer.api.requestAppTypeForPackageContainer(packageContainer) if (proposal.success) { - if (!best || proposal.cost < best.cost) { + if (!best || valueOfCost(proposal.cost) < valueOfCost(best.cost)) { best = { appContainerId: appContainerId, appType: proposal.appType, From c971418ebd546d9cd8cc77bd20c1a292ca1ba860 Mon Sep 17 00:00:00 2001 From: Johan Nyman Date: Tue, 7 May 2024 11:03:36 +0200 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Mint de Wit --- .../src/internalManager/lib/trackedWorkerAgents.ts | 1 + shared/packages/worker/src/workerAgent.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts index 0d6e47a9..3a5ef65e 100644 --- a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts +++ b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts @@ -124,6 +124,7 @@ export class TrackedWorkerAgents { const cost = await workerAgent.api.getCostForExpectation(trackedExp.exp) if (cost.cost !== null) { + // null means that the cost is "infinite" workerCosts.push({ worker: workerAgent.api, id: workerId, diff --git a/shared/packages/worker/src/workerAgent.ts b/shared/packages/worker/src/workerAgent.ts index 2ccd51a5..faf0d254 100644 --- a/shared/packages/worker/src/workerAgent.ts +++ b/shared/packages/worker/src/workerAgent.ts @@ -394,6 +394,7 @@ export class WorkerAgent { } let resultingCost: Cost = systemStartCost for (const job of this.currentJobs) { + // null means that the cost is "infinite" if (resultingCost === null) break if (job.cost.cost === null) resultingCost = null From cdc72c661ff9959aaaa8852c20efc6116004d9e2 Mon Sep 17 00:00:00 2001 From: Johan Nyman Date: Wed, 8 May 2024 07:46:40 +0200 Subject: [PATCH 3/3] chore: lint --- .../src/internalManager/lib/trackedWorkerAgents.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts index 3a5ef65e..3d91dd72 100644 --- a/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts +++ b/shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts @@ -124,7 +124,7 @@ export class TrackedWorkerAgents { const cost = await workerAgent.api.getCostForExpectation(trackedExp.exp) if (cost.cost !== null) { - // null means that the cost is "infinite" + // null means that the cost is "infinite" workerCosts.push({ worker: workerAgent.api, id: workerId,