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

node-persist reading all files to RAM ? #148

Open
Vance-ng-vn opened this issue Jan 15, 2024 · 4 comments
Open

node-persist reading all files to RAM ? #148

Vance-ng-vn opened this issue Jan 15, 2024 · 4 comments
Assignees
Labels

Comments

@Vance-ng-vn
Copy link

Vance-ng-vn commented Jan 15, 2024

My Node.js application reports insufficient memory. The timing seems to coincide with the TTL (Time to Live) of node-persist, and when I inspect the source code, I observe that node-persist appears to be reading all files into the RAM...

        readDirectory: function (dir) {
		return new Promise((resolve, reject) => {
			//check to see if dir is present
			fs.access(dir, (accessErr) => {
				if (!accessErr) {
					//load data
					fs.readdir(dir, async (err, arr) => {
						if (err) {
							return reject(err);
						}
						let data = [];
						try {
							for (let currentFile of arr) {
								if (currentFile[0] !== '.') {
									data.push(await this.readFile(path.join(this.options.dir, currentFile))); //PUSH all file content(text)
								}
							}
						} catch (err) {
							reject(err)
						}
						resolve(data);
					});
				} else {
					reject(new Error(`[node-persist][readDirectory] ${dir} does not exists!`));
				}
			});
		});
	},

	readFile: function (file, options = {}) {
		return new Promise((resolve, reject) => {
			fs.readFile(file, this.options.encoding, (err, text) => {
				if (err) {
					/* Only throw the error if the error is something else other than the file doesn't exist */
					if (err.code === 'ENOENT') {
						this.log(`${file} does not exist, returning undefined value`);
						resolve(options.raw ? '{}' : {});
					} else {
						return reject(err);
					}
				}
				let input = options.raw ? text : this.parse(text);
				if (!options.raw && !isValidStorageFileContent(input)) {
					return this.options.forgiveParseErrors ? resolve(options.raw ? '{}' : {}) : reject(new Error(`[node-persist][readFile] ${file} does not look like a valid storage file!`));
				}
				resolve(input); //Resolve Text
			});
		});
	},
@akhoury
Copy link
Collaborator

akhoury commented Jan 19, 2024

if you use any of these 4 functions, https://github.com/simonlast/node-persist/blob/master/src/local-storage.js#L133-L166 yes, it seems that is in fact reading everything to ram, temporarily, this should be garbage collected since we don't maintain a ref to data. But for most usage, getItem only reads the target file. Now, granted, if you are writing an insane number of files, that could increase the size of the q, can you share how exactly you're using it.

@akhoury
Copy link
Collaborator

akhoury commented Jan 19, 2024

Also, I dont recommend using this library for handling large or high concurrent transactions, maybe something like redis is better for your case. node-persist is designed as a light weight temporary localStorage, with a small limit, inlined with what the browsers currently offer for localStorage, which is 5mb

@Vance-ng-vn
Copy link
Author

if you use any of these 4 functions, https://github.com/simonlast/node-persist/blob/master/src/local-storage.js#L133-L166 yes, it seems that is in fact reading everything to ram, temporarily, this should be garbage collected since we don't maintain a ref to data. But for most usage, getItem only reads the target file. Now, granted, if you are writing an insane number of files, that could increase the size of the q, can you share how exactly you're using it.

I am using it to cache some files in my Node.js app (subtitle files). I need to cache these files for 7 days. However, it seems that node-persist is not suitable for my needs.

Every time it clean up expired files, node-persist loads all of them into RAM. (The total amount of RAM needed remains unchanged compared to caching directly into RAM, even though it's only temporary)

I've switched to using SQLite for temporary storage, and everything is working fine now.

@akhoury
Copy link
Collaborator

akhoury commented Jan 20, 2024

you are right!, cleaning up expired does in fact load everything, that's a bug that i will fix.

@akhoury akhoury self-assigned this Jan 20, 2024
@akhoury akhoury added the bug label Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants