fix(core): Fix pruning of non-finished executions #7333

tomi · 2023-10-03T12:11:08Z

This fixes a bug in the pruning (soft-delete). The pruning was a bit too aggressive, as it also pruned executions that weren't in an end state yet. This only becomes an issue if there are long-running executions (e.g. workflow with Wait node) or the prune parameters are set to keep only a tiny number of executions.

Github issue / Community forum post (link here to close automatically):

This fixes a bug in the pruning (soft-delete). The pruning was a bit too aggressive, as it also pruned executions that weren't in an end state yet. This only becomes an issue if there are long-running executions (e.g. workflow with Wait node) or the prune parameters are set to keep only a tiny number of executions.

CLAassistant · 2023-10-03T12:11:14Z

All committers have signed the CLA.

github-actions · 2023-10-03T12:11:24Z

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

If fixing bug, added test to cover scenario.
If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

tomi · 2023-10-03T12:12:35Z

packages/cli/src/databases/repositories/execution.repository.ts

-		this.logger.debug('Setting soft-deletion interval (pruning) for executions');
+		this.logger.debug(
+			`Setting soft-deletion interval (pruning) for executions with rate ${this.rates.hardDeletion}`,
+		);

 		this.intervals.softDeletion = setInterval(async () => this.prune(), this.rates.hardDeletion);


I noticed we are using hardDeletion rate even for the soft-deletes. Is this intentional or should it be changed?

Good catch, if you can please update.

Fixed. Btw, is it intentional that hard-deletion happens every 15min but soft-deletion only every 1h?

codecov · 2023-10-03T12:37:16Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8f9fe62) 33.29% compared to head (41470df) 33.29%.
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7333   +/-   ##
=======================================
  Coverage   33.29%   33.29%           
=======================================
  Files        3376     3376           
  Lines      205754   205754           
  Branches    22149    22148    -1     
=======================================
  Hits        68499    68499           
  Misses     136141   136141           
  Partials     1114     1114

Files	Coverage Δ
packages/cli/src/config/schema.ts	`81.81% <ø> (ø)`
packages/cli/src/constants.ts	`100.00% <ø> (ø)`
...src/databases/repositories/execution.repository.ts	`42.70% <25.00%> (+2.16%)`	⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

packages/cli/src/databases/repositories/execution.repository.ts

packages/cli/test/integration/repositories/execution.repository.test.ts

tomi · 2023-10-04T09:48:50Z

@ivov thank you for the great PR comments! I've addressed them and this is now ready for re-review

ivov

Looks good! With the new tests, does it make sense to preserve packages/cli/test/unit/repositories/execution.repository.test.ts?

cypress · 2023-10-04T11:14:14Z

1 flaky test on run #2362 ↗︎

0	242	3	0	1

Details:

🌳 ado-896-waiting-execution-data-should-not-be-pruned 🖥️ browsers:node18.12.0-...
Project: n8n	Commit: `41470df3f4`
Status: Passed	Duration: 08:00 💡
Started: Oct 4, 2023 12:44 PM	Ended: Oct 4, 2023 12:52 PM

cypress/e2e/2-credentials.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Credentials > should create a new credential using Add Credential button		`Output` `Screenshots` `Video`

Review all test suite changes for PR #7333 ↗︎

github-actions · 2023-10-04T11:14:19Z

✅ All Cypress E2E specs passed

tomi · 2023-10-04T12:13:38Z

Looks good! With the new tests, does it make sense to preserve packages/cli/test/unit/repositories/execution.repository.test.ts?

Good point. I think those are actually already covered by the new tests. I'll remove them

github-actions · 2023-10-04T12:52:20Z

✅ All Cypress E2E specs passed

This fixes a bug in the pruning (soft-delete). The pruning was a bit too aggressive, as it also pruned executions that weren't in an end state yet. This only becomes an issue if there are long-running executions (e.g. workflow with Wait node) or the prune parameters are set to keep only a tiny number of executions.

## [1.9.1](https://github.com/n8n-io/n8n/compare/n8n@1.9.0...n8n@1.9.1) (2023-10-04) ### Bug Fixes * **core:** Account for itemless case on restoring binary data ID ([#7305](#7305)) ([c479d86](c479d86)) * **core:** Fix pruning of non-finished executions ([#7333](#7333)) ([d8309a1](d8309a1)) * **editor:** Disable email confirmation banner for trialing users ([#7340](#7340)) ([3815327](3815327)) * **editor:** Display value of selected matching column in RMC ([#7298](#7298)) ([9fbd22e](9fbd22e)) * **editor:** Fix canvas endpoint snapping when dragging connection ([#7346](#7346)) ([b4681e7](b4681e7)) * **editor:** Fix RLC not loading when an expression can't resolve ([#7295](#7295)) ([116a6df](116a6df)) * **editor:** Separate cloud endpoint calls ([#7312](#7312)) ([54610b0](54610b0)) * **Jira Software Node:** Get all users in dropdown/RLC ([#7322](#7322)) ([7abf13b](7abf13b)), closes [#2670](#2670) * **Notion Node:** Rename Notion API Key to Internal Integration Token ([#7176](#7176)) ([e606ded](e606ded)) * **Postgres Node:** Node requires comma-separated string even when using a single parameter through an expression ([#7300](#7300)) ([43534ab](43534ab)) * **Set Node:** Do not stringify null and undefined ([#7313](#7313)) ([326abaa](326abaa)) * **Typeform Trigger Node:** Change output format for TypeForm trigger to object instead of array ([#7315](#7315)) ([b1fc981](b1fc981)) Co-authored-by: netroy <netroy@users.noreply.github.com>

janober · 2023-10-04T16:06:46Z

Got released with n8n@1.9.1

# [1.10.0](https://github.com/n8n-io/n8n/compare/n8n@1.9.0...n8n@1.10.0) (2023-10-05) ### Bug Fixes * **Convert to/from binary data Node:** Rename 'Move Binary Data' to 'Convert to/from binary data' ([#7318](#7318)) ([5e6c1d4](5e6c1d4)) * **core:** Account for itemless case on restoring binary data ID ([#7305](#7305)) ([1691223](1691223)) * **core:** Fix pruning of non-finished executions ([#7333](#7333)) ([1b4848a](1b4848a)) * **editor:** Disable email confirmation banner for trialing users ([#7340](#7340)) ([6d3d178](6d3d178)) * **editor:** Display value of selected matching column in RMC ([#7298](#7298)) ([3aac22b](3aac22b)) * **editor:** Fix canvas endpoint snapping when dragging connection ([#7346](#7346)) ([b59b908](b59b908)) * **editor:** Fix disappearing NDV header in code nodes ([#7290](#7290)) ([7ebf8f3](7ebf8f3)) * **editor:** Fix RLC not loading when an expression can't resolve ([#7295](#7295)) ([ddc26c2](ddc26c2)) * **editor:** Separate cloud endpoint calls ([#7312](#7312)) ([04dfcd7](04dfcd7)) * **Jira Software Node:** Get all users in dropdown/RLC ([#7322](#7322)) ([3704760](3704760)), closes [#2670](#2670) * **Notion Node:** Rename Notion API Key to Internal Integration Token ([#7176](#7176)) ([ec2aa38](ec2aa38)) * **Postgres Node:** Node requires comma-separated string even when using a single parameter through an expression ([#7300](#7300)) ([763d451](763d451)) * **Set Node:** Do not stringify null and undefined ([#7313](#7313)) ([f0a6687](f0a6687)) * **Typeform Trigger Node:** Change output format for TypeForm trigger to object instead of array ([#7315](#7315)) ([b3fc00e](b3fc00e)) ### Features * **core:** Add "Sent by n8n" attribution ([#7183](#7183)) ([8f9fe62](8f9fe62)) * **core:** Add support for building LLM applications ([#7235](#7235)) ([00a4b8b](00a4b8b)), closes [#7246](#7246) [#7137](#7137) * Workflow History pruning and prune time settings ([#7343](#7343)) ([0adc533](0adc533)) Co-authored-by: krynble <krynble@users.noreply.github.com>

tomi requested review from mutdmour and ivov October 3, 2023 12:11

tomi changed the title ~~fix(core): fix pruning of non-finished executions~~ fix(core): Fix pruning of non-finished executions Oct 3, 2023

tomi commented Oct 3, 2023

View reviewed changes

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Oct 3, 2023

ivov reviewed Oct 4, 2023

View reviewed changes

fix(core): use correct interval rate for soft deletion

90ae1fc

tomi mentioned this pull request Oct 4, 2023

update data pruning documentation n8n-io/n8n-docs#1633

Merged

fix(core): address review comments

df07119

tomi requested a review from ivov October 4, 2023 09:43

ivov previously approved these changes Oct 4, 2023

View reviewed changes

test(core): remove duplicated tests

41470df

tomi dismissed ivov’s stale review via 41470df October 4, 2023 12:14

tomi requested a review from ivov October 4, 2023 12:14

netroy approved these changes Oct 4, 2023

View reviewed changes

tomi merged commit 1b4848a into master Oct 4, 2023
54 of 55 checks passed

tomi deleted the ado-896-waiting-execution-data-should-not-be-pruned branch October 4, 2023 13:32

github-actions bot mentioned this pull request Oct 4, 2023

🚀 Release 1.9.1 #7349

Merged

janober added the Released label Oct 4, 2023

github-actions bot mentioned this pull request Oct 4, 2023

🚀 Release 1.10.0 #7350

Merged

Joffcom mentioned this pull request Oct 9, 2023

No longer shows executions #7373

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): Fix pruning of non-finished executions #7333

fix(core): Fix pruning of non-finished executions #7333

tomi commented Oct 3, 2023

CLAassistant commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited by tomi

Loading

tomi Oct 3, 2023 •

edited

Loading

ivov Oct 4, 2023

tomi Oct 4, 2023

codecov bot commented Oct 3, 2023 •

edited

Loading

tomi commented Oct 4, 2023

ivov left a comment

cypress bot commented Oct 4, 2023 •

edited

Loading

github-actions bot commented Oct 4, 2023

tomi commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

janober commented Oct 4, 2023

fix(core): Fix pruning of non-finished executions #7333

fix(core): Fix pruning of non-finished executions #7333

Conversation

tomi commented Oct 3, 2023

CLAassistant commented Oct 3, 2023 • edited Loading

github-actions bot commented Oct 3, 2023 • edited by tomi Loading

tomi Oct 3, 2023 • edited Loading

Choose a reason for hiding this comment

ivov Oct 4, 2023

Choose a reason for hiding this comment

tomi Oct 4, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 3, 2023 • edited Loading

Codecov Report

tomi commented Oct 4, 2023

ivov left a comment

Choose a reason for hiding this comment

cypress bot commented Oct 4, 2023 • edited Loading

1 flaky test on run #2362 ↗︎

Review all test suite changes for PR #7333 ↗︎

github-actions bot commented Oct 4, 2023

tomi commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

janober commented Oct 4, 2023

CLAassistant commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited by tomi

Loading

tomi Oct 3, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading

cypress bot commented Oct 4, 2023 •

edited

Loading