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.

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

Merged
merged 68 commits into from Feb 17, 2023

Conversation

flipswitchingmonkey
Copy link
Contributor

@flipswitchingmonkey flipswitchingmonkey commented Jan 9, 2023

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

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jan 9, 2023
@flipswitchingmonkey flipswitchingmonkey changed the title feat(editor): Adds execution events to ExecutionsList and marks unknown as failed feat(editor): Adds execution runData recovery to backend Jan 16, 2023
@@ -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?
Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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.

krynble
krynble previously approved these changes Feb 17, 2023
Copy link
Contributor

@krynble krynble left a 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.

@flipswitchingmonkey flipswitchingmonkey merged commit d143f3f into master Feb 17, 2023
@flipswitchingmonkey flipswitchingmonkey deleted the ENG-13-status-unknown-improvement branch February 17, 2023 09:54
@n8n-assistant n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Feb 17, 2023
@janober
Copy link
Member

janober commented Feb 24, 2023

Got released with n8n@0.217.1

@janober janober removed the Upcoming Release Will be part of the upcoming release label Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants