refactor(core): Simplify marking logic in binary data manager (no-changelog) (#7046)

- For a saved execution, we write to disk binary data and metadata.
These two are only ever deleted via `POST /executions/delete`. No marker
file, so untouched by pruning.
- For an unsaved execution, we write to disk binary data, binary data
metadata, and a marker file at `/meta`. We later delete all three during
pruning.
- The third flow is legacy. Currently, if the execution is unsaved, we
actually store it in the DB while running the workflow and immediately
after the workflow is finished during the `onWorkflowPostExecute()` hook
we delete that execution, so the second flow applies. But formerly, we
did not store unsaved executions in the DB ("ephemeral executions") and
so we needed to write a marker file at `/persistMeta` so that, if the
ephemeral execution crashed after the step where binary data was stored,
we had a way to later delete its associated dangling binary data via a
second pruning cycle, and if the ephemeral execution succeeded, then we
immediately cleaned up the marker file at `/persistMeta` during the
`onWorkflowPostExecute()` hook.

This creation and cleanup at `/persistMeta` is still happening, but this
third flow no longer has a purpose, as we now store unsaved executions
in the DB and delete them immediately after. Hence the third flow can be
removed.

packages/cli/src/InternalHooks.ts

@@ -1,6 +1,5 @@
 import { Service } from 'typedi';
 import { snakeCase } from 'change-case';
-import { BinaryDataManager } from 'n8n-core';
 import type {
 	AuthenticationMethod,
 	ExecutionStatus,
@@ -461,8 +460,6 @@ export class InternalHooks implements IInternalHooksClass {
 				  }),
 		);
 
-		await BinaryDataManager.getInstance().persistBinaryDataForExecutionId(executionId);
-
 		void Promise.all([...promises, this.telemetry.trackWorkflowExecution(properties)]);
 	}
 

packages/cli/src/config/schema.ts

@@ -913,12 +913,6 @@ export const schema = {
 			env: 'N8N_BINARY_DATA_TTL',
 			doc: 'TTL for binary data of unsaved executions in minutes',
 		},
-		persistedBinaryDataTTL: {
-			format: Number,
-			default: 1440,
-			env: 'N8N_PERSISTED_BINARY_DATA_TTL',
-			doc: 'TTL for persisted binary data in minutes (binary data gets deleted if not persisted before TTL expires)',
-		},
 	},
 
 	deployment: {

packages/core/src/BinaryDataManager/FileSystem.ts

@@ -11,7 +11,6 @@ import type { IBinaryDataConfig, IBinaryDataManager } from '../Interfaces';
 import { FileNotFoundError } from '../errors';
 
 const PREFIX_METAFILE = 'binarymeta';
-const PREFIX_PERSISTED_METAFILE = 'persistedmeta';
 
 const executionExtractionRegexp =
 	/^(\w+)(?:[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12})$/;
@@ -21,12 +20,9 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 
 	private binaryDataTTL: number;
 
-	private persistedBinaryDataTTL: number;
-
 	constructor(config: IBinaryDataConfig) {
 		this.storagePath = config.localStoragePath;
 		this.binaryDataTTL = config.binaryDataTTL;
-		this.persistedBinaryDataTTL = config.persistedBinaryDataTTL;
 	}
 
 	async init(startPurger = false): Promise<void> {
@@ -34,18 +30,12 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 			setInterval(async () => {
 				await this.deleteMarkedFiles();
 			}, this.binaryDataTTL * 30000);
-
-			setInterval(async () => {
-				await this.deleteMarkedPersistedFiles();
-			}, this.persistedBinaryDataTTL * 30000);
 		}
 
 		await this.assertFolder(this.storagePath);
 		await this.assertFolder(this.getBinaryDataMetaPath());
-		await this.assertFolder(this.getBinaryDataPersistMetaPath());
 
 		await this.deleteMarkedFiles();
-		await this.deleteMarkedPersistedFiles();
 	}
 
 	async getFileSize(identifier: string): Promise<number> {
@@ -55,7 +45,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 
 	async copyBinaryFile(filePath: string, executionId: string): Promise<string> {
 		const binaryDataId = this.generateFileName(executionId);
-		await this.addBinaryIdToPersistMeta(executionId, binaryDataId);
 		await this.copyFileToLocalStorage(filePath, binaryDataId);
 		return binaryDataId;
 	}
@@ -72,7 +61,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 
 	async storeBinaryData(binaryData: Buffer | Readable, executionId: string): Promise<string> {
 		const binaryDataId = this.generateFileName(executionId);
-		await this.addBinaryIdToPersistMeta(executionId, binaryDataId);
 		await this.saveToLocalStorage(binaryData, binaryDataId);
 		return binaryDataId;
 	}
@@ -105,30 +93,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 		return this.deleteMarkedFilesByMeta(this.getBinaryDataMetaPath(), PREFIX_METAFILE);
 	}
 
-	async deleteMarkedPersistedFiles(): Promise<void> {
-		return this.deleteMarkedFilesByMeta(
-			this.getBinaryDataPersistMetaPath(),
-			PREFIX_PERSISTED_METAFILE,
-		);
-	}
-
-	private async addBinaryIdToPersistMeta(executionId: string, identifier: string): Promise<void> {
-		const currentTime = new Date().getTime();
-		const timeAtNextHour = currentTime + 3600000 - (currentTime % 3600000);
-		const timeoutTime = timeAtNextHour + this.persistedBinaryDataTTL * 60000;
-
-		const filePath = this.resolveStoragePath(
-			'persistMeta',
-			`${PREFIX_PERSISTED_METAFILE}_${executionId}_${timeoutTime}`,
-		);
-
-		try {
-			await fs.access(filePath);
-		} catch {
-			await fs.writeFile(filePath, identifier);
-		}
-	}
-
 	private async deleteMarkedFilesByMeta(metaPath: string, filePrefix: string): Promise<void> {
 		const currentTimeValue = new Date().valueOf();
 		const metaFileNames = await glob(`${filePrefix}_*`, { cwd: metaPath });
@@ -184,19 +148,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 		return this.deleteFromLocalStorage(identifier);
 	}
 
-	async persistBinaryDataForExecutionId(executionId: string): Promise<void> {
-		const metaFiles = await fs.readdir(this.getBinaryDataPersistMetaPath());
-		const promises = metaFiles.reduce<Array<Promise<void>>>((prev, curr) => {
-			if (curr.startsWith(`${PREFIX_PERSISTED_METAFILE}_${executionId}_`)) {
-				prev.push(fs.rm(path.join(this.getBinaryDataPersistMetaPath(), curr)));
-				return prev;
-			}
-
-			return prev;
-		}, []);
-		await Promise.all(promises);
-	}
-
 	private async assertFolder(folder: string): Promise<void> {
 		try {
 			await fs.access(folder);
@@ -213,10 +164,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager {
 		return path.join(this.storagePath, 'meta');
 	}
 
-	private getBinaryDataPersistMetaPath() {
-		return path.join(this.storagePath, 'persistMeta');
-	}
-
 	private async deleteFromLocalStorage(identifier: string) {
 		return fs.rm(this.getBinaryPath(identifier));
 	}

packages/core/src/BinaryDataManager/index.ts

@@ -172,12 +172,6 @@ export class BinaryDataManager {
 		}
 	}
 
-	async persistBinaryDataForExecutionId(executionId: string): Promise<void> {
-		if (this.managers[this.binaryDataMode]) {
-			await this.managers[this.binaryDataMode].persistBinaryDataForExecutionId(executionId);
-		}
-	}
-
 	async deleteBinaryDataByExecutionIds(executionIds: string[]): Promise<void> {
 		if (this.managers[this.binaryDataMode]) {
 			await this.managers[this.binaryDataMode].deleteBinaryDataByExecutionIds(executionIds);

packages/core/src/Interfaces.ts

@@ -39,7 +39,6 @@ export interface IBinaryDataConfig {
 	availableModes: string;
 	localStoragePath: string;
 	binaryDataTTL: number;
-	persistedBinaryDataTTL: number;
 }
 
 export interface IBinaryDataManager {
@@ -57,7 +56,6 @@ export interface IBinaryDataManager {
 	deleteBinaryDataByIdentifier(identifier: string): Promise<void>;
 	duplicateBinaryDataByIdentifier(binaryDataId: string, prefix: string): Promise<string>;
 	deleteBinaryDataByExecutionIds(executionIds: string[]): Promise<string[]>;
-	persistBinaryDataForExecutionId(executionId: string): Promise<void>;
 }
 
 export namespace n8n {

packages/core/test/NodeExecuteFunctions.test.ts

@@ -35,7 +35,6 @@ describe('NodeExecuteFunctions', () => {
 				availableModes: 'default',
 				localStoragePath: temporaryDir,
 				binaryDataTTL: 1,
-				persistedBinaryDataTTL: 1,
 			});
 
 			// Set our binary data buffer
@@ -86,7 +85,6 @@ describe('NodeExecuteFunctions', () => {
 				availableModes: 'filesystem',
 				localStoragePath: temporaryDir,
 				binaryDataTTL: 1,
-				persistedBinaryDataTTL: 1,
 			});
 
 			// Set our binary data buffer

packages/nodes-base/test/nodes/Helpers.ts

@@ -223,7 +223,6 @@ export async function initBinaryDataManager(mode: 'default' | 'filesystem' = 'de
 		availableModes: mode,
 		localStoragePath: temporaryDir,
 		binaryDataTTL: 1,
-		persistedBinaryDataTTL: 1,
 	});
 	return temporaryDir;
 }