-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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.
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
feat(core): Add execution runData recovery and status field #5112
feat(core): Add execution runData recovery and status field #5112
Conversation
# Conflicts: # packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts # packages/cli/src/eventbus/MessageEventBusWriter/MessageEventBusLogWriter.ts # packages/editor-ui/src/components/ExecutionsList.vue # packages/editor-ui/src/mixins/restApi.ts # packages/editor-ui/src/plugins/i18n/locales/en.json
# Conflicts: # packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts
# Conflicts: # packages/cli/src/Server.ts # packages/cli/src/databases/migrations/mysqldb/index.ts # packages/cli/src/databases/migrations/postgresdb/index.ts # packages/cli/src/databases/migrations/sqlite/index.ts
# Conflicts: # packages/cli/src/WorkflowExecuteAdditionalData.ts # packages/cli/src/WorkflowRunner.ts # packages/cli/src/commands/executeBatch.ts # packages/cli/src/commands/start.ts # packages/cli/src/commands/worker.ts
@@ -59,32 +66,37 @@ export class ActiveExecutions { | |||
const execution = ResponseHelper.flattenExecutionData(fullExecutionData); | |||
|
|||
const executionResult = await Db.collections.Execution.save(execution as IExecutionFlattedDb); | |||
// TODO: what is going on here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably legacy from when we supported MongoDB where IDs where instances of ObjectID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So.... should we keep it in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we could remove this, since this is in the codebase for over 2 years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've ran a few tests and it all seems to be working.
Tested locally with queue mode and without, recovery system seems ok.
Also tested multiple executions in parallel where only one fails and it recovers the correct one.
Got released with |
This PR will add a
status
field to the execution table and sets the status at appropriate points during the executions lifecycle.It also leverages the event logging journal to attempt to recover execution run data in the case of a crashed execution (e.g. through out of memory issues), to pinpoint the node that caused the crash.
Also updates the frontend accordingly to read the status field.
#2195