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

fix: Stop OOM crashes in Execution Data pruning #5095

Merged
merged 2 commits into from
Jan 6, 2023

Conversation

netroy
Copy link
Member

@netroy netroy commented Jan 5, 2023

Currently while pruning execution data, we are loading all the data in memory. For instances where there are thousands of executions, this causes the container to run out of memory. Since ids is all we need, we should only query for ids.

HELP-72

Currently while pruning execution data, we are loading all the data in memory. For instances where there are thousands of executions, this causes the container to run out of memory.
Since ids is all we need, we should only query for ids.
@netroy netroy requested a review from krynble January 5, 2023 18:41
@netroy netroy changed the title fix: Stop OOM crashed in Execution Data pruning fix: Stop OOM crashes in Execution Data pruning Jan 5, 2023
@janober
Copy link
Member

janober commented Jan 5, 2023

Uh wow. Thanks for catching this. But should that not also just happen if we use the not default mode of storing binary data? Because for most people that query should not be required at all.

@netroy
Copy link
Member Author

netroy commented Jan 5, 2023

Uh wow. Thanks for catching this. But should that not also just happen if we use the not default mode of storing binary data? Because for most people that query should not be required at all.

This affects only instances where EXECUTIONS_DATA_PRUNE is set to true. This includes all our cloud customers.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jan 5, 2023
@janober
Copy link
Member

janober commented Jan 5, 2023

Ah yes, only happens if prune is activated.

But I meant the query does currently only have to run if EXECUTIONS_DATA_PRUNE === true && BINARY_MODE === "filesystem".

For everybody that does not use filesystem mode the query runs and then the data does get discarded again unused.

@janober
Copy link
Member

janober commented Jan 5, 2023

Or to say it differently. It currently does that query that the binary files on the filesystem get deleted. But for not filesystem mode there are not files to be deleted as the binary data is saved in the database together with the execution.

…nary data

in default mode the binary data is in the database, and will get pruned along with the executions.
@netroy
Copy link
Member Author

netroy commented Jan 5, 2023

@janober you are right. pushed another commit to skip the query in default mode.

@netroy netroy merged commit c4df204 into master Jan 6, 2023
@netroy netroy deleted the HELP-72-fix-oom-crash-in-ExecutionData-pruning branch January 6, 2023 10:42
@n8n-assistant n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Jan 6, 2023
MiloradFilipovic added a commit that referenced this pull request Jan 9, 2023
* master: (53 commits)
  fix: Remove anonymous ID from tracking calls (#5099)
  refactor(core): Add more overloads for string-type node parameters (no-changelog) (#5101)
  fix(Read Binary File Node): Do not crash the execution when the source file does not exist (#5100)
  fix: Stop OOM crashes in Execution Data pruning (#5095)
  feat(editor): Introduce proxy completions to expressions (#5075)
  fix(Google Sheets Node): Fix for auto-range detection
  📚 Update CHANGELOG.md and main package.json to 0.210.1
  🔖 Release n8n@0.210.1
  ⬆️ Set n8n-editor-ui@0.176.1 and n8n-nodes-base@0.208.1 on n8n
  🔖 Release n8n-editor-ui@0.176.1
  ⬆️ Set n8n-design-system@0.50.1 on n8n-editor-ui
  🔖 Release n8n-design-system@0.50.1
  🔖 Release n8n-nodes-base@0.208.1
  fix: Pass in the correct server reference to external hooks (no-changelog) (#5094)
  fix(Google Sheets Node): Append or Update fails for numeric values
  feat: Add user management invite links without SMTP set up (#5084)
  fix: Remove annonymous ID (no-changelog) (#5093)
  📚 Update CHANGELOG.md and main package.json to 0.210.0
  🔖 Release n8n@0.210.0
  ⬆️ Set n8n-core@0.150.0, n8n-editor-ui@0.176.0, n8n-nodes-base@0.208.0 and n8n-workflow@0.132.0 on n8n
  ...

# Conflicts:
#	packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue
@janober
Copy link
Member

janober commented Jan 9, 2023

Got released with n8n@0.210.2

@janober janober removed the Upcoming Release Will be part of the upcoming release label Jan 9, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants