Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(HTTPSend): Allow HTTPSend commands to be sent in separate sequential queues (SOFIE-3431) #346

Merged
merged 2 commits into from
Sep 9, 2024
Merged

fix(HTTPSend): Allow HTTPSend commands to be sent in separate sequential queues (SOFIE-3431) #346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84a53cd
feat: allow sequential executionMode to paralelize multiple queues of…
jstarpl Sep 5, 2024
5797d0d
Update packages/timeline-state-resolver/src/service/device.ts
jstarpl Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/timeline-state-resolver-types/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,6 @@ export interface ActionExecutionResult<ResultData = undefined> {

export enum ActionExecutionResultCode {
Error = 'ERROR',
IgnoredNotRelevant = 'IGNORED',
Ok = 'OK',
}
2 changes: 2 additions & 0 deletions packages/timeline-state-resolver/src/integrations/httpSend/__tests__/httpsend.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ describe('HTTP-Send', () => {
content: content,
layer: 'layer0',
},
queueId: undefined,
},
])
await Promise.all(commands.map(async (c) => device.sendCommand(c)))
Expand All @@ -362,6 +363,7 @@ describe('HTTP-Send', () => {
content: content,
layer: 'layer0',
},
queueId: undefined,
},
])
}
Expand Down
7 changes: 5 additions & 2 deletions packages/timeline-state-resolver/src/integrations/httpSend/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export class HTTPSendDevice extends Device<HTTPSendOptions, HttpSendDeviceState,
content: newLayer.content as HTTPSendCommandContent,
layer: layerKey,
},
queueId: (newLayer.content as HTTPSendCommandContent)?.queueId,
})
} else {
// changed?
Expand All @@ -149,6 +150,7 @@ export class HTTPSendDevice extends Device<HTTPSendOptions, HttpSendDeviceState,
content: newLayer.content as HTTPSendCommandContent,
layer: layerKey,
},
queueId: (newLayer.content as HTTPSendCommandContent)?.queueId,
})
}
}
Expand All @@ -162,6 +164,7 @@ export class HTTPSendDevice extends Device<HTTPSendOptions, HttpSendDeviceState,
timelineObjId: oldLayer.id,
context: `removed: ${oldLayer.id}`,
command: { commandName: 'removed', content: oldLayer.content as HTTPSendCommandContent, layer: layerKey },
queueId: (oldLayer.content as HTTPSendCommandContent)?.queueId,
})
}
})
Expand Down Expand Up @@ -194,7 +197,7 @@ export class HTTPSendDevice extends Device<HTTPSendOptions, HttpSendDeviceState,
const trackedHash = this.trackedState.get(command.layer)
if (commandHash !== trackedHash)
return {
result: ActionExecutionResultCode.Error,
result: ActionExecutionResultCode.IgnoredNotRelevant,
} // command is no longer relevant to state
}
if (this._terminated) {
Expand Down Expand Up @@ -254,7 +257,7 @@ export class HTTPSendDevice extends Device<HTTPSendOptions, HttpSendDeviceState,

const response = await httpReq(command.content.url, options)

if (response.statusCode === 200) {
if (response.statusCode >= 200 && response.statusCode < 300) {
this.context.logger.debug(
`HTTPSend: ${command.content.type}: Good statuscode response on url "${command.content.url}": ${response.statusCode} (${context})`
)
Expand Down
9 changes: 4 additions & 5 deletions packages/timeline-state-resolver/src/integrations/quantel/connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,11 +591,10 @@ export class QuantelManager extends EventEmitter {
})
}
public clearAllWaitWithPort(portId: string) {
if (!this._waitWithPorts[portId]) {
Copy link
Member

@Julusian Julusian Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug task already for this SOFIE-3419, as this is not a recent mistake so needs a thorough test

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see any use of clearAllWaitWithPort but it seemed clearly wrong, so I just changed it now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right yeah, this quantel one isn't called anymore due to a mistake in the refactoring. so I guess this can be in whatever state you prefer as it is currently dead code. It is also refactored in r52 too, so expect a minor merge conflict there

_.each(this._waitWithPorts[portId], (fcn) => {
fcn(true)
})
}
if (!this._waitWithPorts[portId]) return
_.each(this._waitWithPorts[portId], (fcn) => {
fcn(true)
})
}
/**
* Returns true if the wait was cleared from someone else
Expand Down
23 changes: 15 additions & 8 deletions packages/timeline-state-resolver/src/service/commandExecutor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as _ from 'underscore'
import { BaseDeviceAPI, CommandWithContext } from './device'
import { Measurement } from './measure'
import { StateHandlerContext } from './stateHandler'
Expand Down Expand Up @@ -50,15 +51,21 @@ export class CommandExecutor<DeviceState, Command extends CommandWithContext> {
): Promise<void> {
const start = Date.now() // note - would be better to use monotonic time here but BigInt's are annoying

for (const command of commands || []) {
const timeToWait = totalTime - (Date.now() - start)
if (timeToWait > 0) await wait(timeToWait)
const commandQueues = _.groupBy(commands || [], (command) => command.queueId ?? '$$default')

measurement?.executeCommand(command)
await this.sendCommand(command).catch((e) => {
this.logger.error('Error while executing command', e)
await Promise.allSettled(
Object.values<Command[]>(commandQueues).map(async (commandsInQueue): Promise<void> => {
for (const command of commandsInQueue) {
const timeToWait = totalTime - (Date.now() - start)
if (timeToWait > 0) await wait(timeToWait)

measurement?.executeCommand(command)
await this.sendCommand(command).catch((e) => {
this.logger.error('Error while executing command', e)
})
measurement?.finishedCommandExecution(command)
}
})
measurement?.finishedCommandExecution(command)
}
)
}
}
2 changes: 2 additions & 0 deletions packages/timeline-state-resolver/src/service/device.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export type CommandWithContext = {
timelineObjId: string
/** this command is to be executed x ms _before_ the scheduled time */
preliminary?: number
/** commands with different queueId's can be executed in paralel in sequential mode */
jstarpl marked this conversation as resolved.
Show resolved Hide resolved
queueId?: string
}

/**
Expand Down
Loading